maria-developers team mailing list archive

[Michael Widenius <michael.widenius@xxxxxxxxx>]

Hi!

On Thu, Apr 16, 2020 at 1:17 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Michael!
>
> On Apr 13, Michael Widenius wrote:
> > revision-id: bc5c062b1d1 (mariadb-10.5.2-124-gbc5c062b1d1)
> > parent(s): fb29c886701
> > author: Michael Widenius <monty@xxxxxxxxxxx>
> > committer: Michael Widenius <monty@xxxxxxxxxxx>
> > timestamp: 2020-04-09 01:37:02 +0300
> > message:
> >
> > Don't try to open temporary tables if there are no temporary tables

> Why?

Because it's quite slow to loop over all tables to check if could be a
temporary tables
in the default case where we don't have any temporary tables at all.
This will improve performance for almost any table open.

I have added this to the commit message

> > diff --git a/sql/sql_base.cc b/sql/sql_base.cc
> > index 4d7a7606136..be19a2e1d82 100644
> > --- a/sql/sql_base.cc
> > +++ b/sql/sql_base.cc
> > @@ -3704,9 +3704,9 @@ open_and_process_table(THD *thd, TABLE_LIST *tables, uint *counter, uint flags,
> >          The problem is that since those attributes are not set in merge
> >          children, another round of PREPARE will not help.
> >      */
> > -    error= thd->open_temporary_table(tables);
> > -
> > -    if (!error && !tables->table)
> > +    if (!thd->has_temporary_tables() ||
> > +        (!(error= thd->open_temporary_table(tables)) &&
> > +         !tables->table))
>
> please don't write conditions like that. keep it as before, two
> statements:

Why?  It's perfectly clear and not worse than

  if (!thd->has_temporary_tables())
    if (!(error= thd->open_temporary_table(tables)) &&  !tables->table))


>    if (thd->has_temporary_tables())
>      error= thd->open_temporary_table(tables);
>    if (!error && !tables->table)

Sorry, the above is worse and more error prone as you error may no be
set when you test it.

> > --- a/sql/temporary_tables.cc
> > +++ b/sql/temporary_tables.cc
> > @@ -338,9 +338,9 @@ bool THD::open_temporary_table(TABLE_LIST *tl)
> >      have invalid db or table name.
> >      Instead THD::open_tables() should be used.
> >    */
> > -  DBUG_ASSERT(!tl->derived && !tl->schema_table);
> > +  DBUG_ASSERT(!tl->derived && !tl->schema_table && has_temporary_tables());
>
> please, never do asserts with &&-ed conditions, this should be
>
>      DBUG_ASSERT(!tl->derived);
>      DBUG_ASSERT(!tl->schema_table);
>      DBUG_ASSERT(has_temporary_tables());

ok, I can change that.

Regards,
Monty