maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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?

> 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:

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

>        error= open_table(thd, tables, ot_ctx);
>  
>      thd->pop_internal_handler();
> @@ -3723,9 +3723,9 @@ open_and_process_table(THD *thd, TABLE_LIST *tables, uint *counter, uint flags,
>      Repair_mrg_table_error_handler repair_mrg_table_handler;
>      thd->push_internal_handler(&repair_mrg_table_handler);
>  
> -    error= thd->open_temporary_table(tables);
> -
> -    if (!error && !tables->table)
> +    if (!thd->has_temporary_tables() ||
> +        (!(error= thd->open_temporary_table(tables)) &&
> +         !tables->table))
>        error= open_table(thd, tables, ot_ctx);
>  
>      thd->pop_internal_handler();
> @@ -3740,7 +3740,8 @@ open_and_process_table(THD *thd, TABLE_LIST *tables, uint *counter, uint flags,
>          still might need to look for a temporary table if this table
>          list element corresponds to underlying table of a merge table.
>        */
> -      error= thd->open_temporary_table(tables);
> +      if (thd->has_temporary_tables())
> +        error= thd->open_temporary_table(tables);
>      }
>  
>      if (!error && !tables->table)
> diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc
> index 407072a7b49..553460729a1 100644
> --- 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());

> -  if (tl->open_type == OT_BASE_ONLY || !has_temporary_tables())
> +  if (tl->open_type == OT_BASE_ONLY)
>    {
>      DBUG_PRINT("info", ("skip_temporary is set or no temporary tables"));
>      DBUG_RETURN(false);

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx