← Back to team overview

maria-developers team mailing list archive

Re: 0608c273598: MDEV-15966: Behavior for TRUNCATE versioned table is not documented and not covered by tests

 

On Fri, Dec 28, 2018 at 9:18 AM Sergei Golubchik <serg@xxxxxxxxxxx> 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

Follow ups

References