On Fri, Dec 28, 2018 at 9:18 AM Sergei Golubchik wrote:

Hi, Nikita!

On Dec 28, Nikita Malyavin wrote:

...

Hi, Sergei!

In short, I am on the side of standard conformance.

Our TRUNCATE is not exactly a standard TRUNCATE. it's defined as DROP+REPLACE. But generally I agree, standard conformance.

The fun thing is that even handler::truncate() is defined as delete_all_rows() 🙂 Actually even ha_innodb does something same, and frm file is never deleted/touched. So physically it's always deletion of rows data, not DROP + CREATE Maybe I'm missing something, am I?

Next two statements are generally out of scope of this bug, but: I

...

like to see extra2 reading in separate function, and i believe that `TABLE_SHARE::init_from_binary_frm_image` needs more decomposiiton.

May be... not sure...

...

Further:

...

I don't quite like that a small and simple function dd_frm_type(), which used to just read few first bytes of the frm file, gets more and more complex, growing into a complete frm-open method.

So, perhaps, I'd consider changing TRUNCATE to open the table properly without all that just-read-few-bytes shortcuts.

Wasn't the there some performance issue as the reason to introduce such read-few-bytes thing? If it's not the case, then I'm okay with that, but feels like TRUNCATE code written in manner to make IO as fast as possible there.

I don't know if there was... but now it's certainly used in some cases to avoid full table open. Mainly in ha_table_exists(), which is used in many places in the server. And there we don't care whether a table is versioned or not.

TRUNCATE uses it too, but it's not a performance critical statement, so I think it's ok to do a full table open.

...

...

Or, may be, simply document that TRUNCATE works on versioning tables just as DROP+CREATE does. There is no logical reason why it shouldn't - if one can do SHOW CREATE TABLE and CREATE OR REPLACE, then TRUNCATE won't add any new functionality on top of that. That's the easiest and most logical "fix" to this bug. Agree?

Consider we'll have VTMD someday at last. Then the TRUNCATE behavior will differ for different `vers_alter_history` modes. It is better to extend the command (or create new one) to make something related to versioning: either truncate only actual data, or only history (we already have delete history for this one), or both.

Okay, then just do a full table open.

Updated the code.

I'd try to remove HTON_CAN_RECREATE flag completely and implement recreate logic inside handler::delete_all_rows()

Yes, good idea, I'm thinking the same. So am I supposed to file MDEV for it then?

-- Yours truly, Nikita Malyavin

On Fri, Mar 1, 2019 at 1:24 AM Sergei Golubchik wrote:

Hi, Nikita!

On Feb 28, Nikita Malyavin wrote:

...

On Fri, Dec 28, 2018 at 9:18 AM Sergei Golubchik wrote:

...

On Dec 28, Nikita Malyavin wrote:

...

Hi, Sergei!

In short, I am on the side of standard conformance.

Our TRUNCATE is not exactly a standard TRUNCATE. it's defined as DROP+REPLACE. But generally I agree, standard conformance.

The fun thing is that even handler::truncate() is defined as delete_all_rows() 🙂 Actually even ha_innodb does something same, and frm file is never deleted/touched. So physically it's always deletion of rows data, not DROP + CREATE Maybe I'm missing something, am I?

you are :)

If the handlerton has a flag HTON_CAN_RECREATE, then Sql_cmd_truncate_table::lock_table() will set hton_can_recreate and Sql_cmd_truncate_table::truncate_table() will use dd_recreate_table(), not handler::delete_all_rows()

I know, but it doesn't affect frm.

...

I'd try to remove HTON_CAN_RECREATE flag completely and implement

...

recreate logic inside handler::delete_all_rows()

Yes, good idea, I'm thinking the same. So am I supposed to file MDEV for it then?

Yes, please

MDEV-18773

-- Yours truly, Nikita Malyavin