maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Oleksandr!

On Oct 15, Oleksandr Byelkin wrote:
> revision-id: baaaa971662 (mariadb-10.2.40-61-gbaaaa971662)
> parent(s): dce490e9d4d
> author: Oleksandr Byelkin
> committer: Oleksandr Byelkin
> timestamp: 2021-09-19 19:23:49 +0200
> message:
> 
> MDEV-22883: Assertion `!parse_error || lex.sphead == 0' failed in Table_triggers_list::check_n_load | SIGSEGV in strxmov
> 
> Abort execution of SHOW in case of fatal error.

Does this patch fixes all test cases from the bug report?
There are quite a few.

> diff --git a/sql/sql_show.cc b/sql/sql_show.cc
> index 721bb053343..105b2719882 100644
> --- a/sql/sql_show.cc
> +++ b/sql/sql_show.cc
> @@ -4849,7 +4849,8 @@ static int fill_schema_table_from_frm(THD *thd, TABLE *table,
>    */
>    DBUG_ASSERT(thd->open_tables == NULL);
>    thd->mdl_context.rollback_to_savepoint(open_tables_state_backup->mdl_system_tables_svp);
> -  thd->clear_error();
> +  if (!thd->is_fatal_error)
> +    thd->clear_error();

Hmm. THD::clear_error() starts with a comment:

  /**  
    Clear the current error, if any.
    We do not clear is_fatal_error or is_fatal_sub_stmt_error since we
    assume this is never called if the fatal error is set.

Unfortunately, it doesn't have an assert to go with this, otherwise all
these bugs would've probably failed identically and early. Please add

  DBUG_ASSERT(!is_fatal_error);
  DBUG_ASSERT(!is_fatal_sub_stmt_error);

or something like that to THD::clear_error()

>    return res;
>  }
>  
> @@ -5064,7 +5065,7 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond)
>                  continue;
>              }
>  
> -            if (thd->killed == ABORT_QUERY)
> +            if (thd->killed == ABORT_QUERY || thd->is_fatal_error)
>              {
>                error= 0;
>                goto err;

that's kind of weird. Why does it continue executing if the statement is
killed? Perhaps it'd better be

   if (thd->killed || thd->is_fatal_error)
   {
     if (thd->killed == ABORT_QUERY)
       error= 0;
     goto err;

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