maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Rucha,

The fix is ok to push. But as a general comment, better try to define
variables in the smallest scope where they're used. In particular here
it could've been

         {
           MY_BITMAP *old_map= dbug_tmp_use_all_columns(table, &table->write_set);
-          (void) item->save_in_field(key_part->field, 1);
+          int res= item->save_in_field(key_part->field, 1);

because you don't need `res` outside of that scope. Makes the code
easier to read.

Of course, there can be exceptions. For example, if the variable is an
object and its initialization is very expensive.

But whether you move `int res` declaration inside or not - it's ok to
push, no need to ask for another review.

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

On Apr 25, Rucha Deodhar wrote:
> revision-id: 5f8ffc7271a (mariadb-10.2.40-251-g5f8ffc7271a)
> parent(s): 2cbf92522b5
> author: Rucha Deodhar
> committer: Rucha Deodhar
> timestamp: 2022-01-27 01:18:19 +0530
> message:
> 
> MDEV-20207: Assertion `! is_set()' failed in Diagnostics_area::set_eof_status
> upon HANDLER READ
> 
> Analysis: The error state is not stored while checking condition and key name.
> Fix: Return true while checking condition and key name if error is reported
> because geometry object can't be created from the data in the index value for
> HANDLER READ.
> 
> diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc
> index ec3756eceba..7c002a68d9e 100644
> --- a/sql/sql_handler.cc
> +++ b/sql/sql_handler.cc
> @@ -554,6 +554,7 @@ mysql_ha_fix_cond_and_key(SQL_HANDLER *handler,
>  {
>    THD *thd= handler->thd;
>    TABLE *table= handler->table;
> +  int res= 0;
>    if (cond)
>    {
>      /* This can only be true for temp tables */
> @@ -613,8 +614,10 @@ mysql_ha_fix_cond_and_key(SQL_HANDLER *handler,
>          if (!in_prepare)
>          {
>            MY_BITMAP *old_map= dbug_tmp_use_all_columns(table, &table->write_set);
> -          (void) item->save_in_field(key_part->field, 1);
> +          res= item->save_in_field(key_part->field, 1);
>            dbug_tmp_restore_column_map(&table->write_set, old_map);
> +          if (res)
> +            return 1;
>          }
>          key_len+= key_part->store_length;
>          keypart_map= (keypart_map << 1) | 1;