← Back to team overview

maria-developers team mailing list archive

Re: 394bb653b0b: MDEV-26695: Number of an invalid row is not calculated for table value constructor

 

Hi, Rucha!

please, see below

On Jan 11, Rucha Deodhar wrote:
> revision-id: 394bb653b0b (mariadb-10.6.1-140-g394bb653b0b)
> parent(s): 3c857a07d92
> author: Rucha Deodhar
> committer: Rucha Deodhar
> timestamp: 2021-10-20 01:30:04 +0530
> message:
> 
> MDEV-26695: Number of an invalid row is not calculated for table value
> constructor
> 
> Analysis: counter does not increment while sending rows for table value
> constructor and so row_number assumes the default value (0 in this case).
> Fix: Increment the counter to avoid counter using default value.
> 
> diff --git a/sql/sql_tvc.cc b/sql/sql_tvc.cc
> index 49f319b3856..a191ec9c328 100644
> --- a/sql/sql_tvc.cc
> +++ b/sql/sql_tvc.cc
> @@ -422,7 +422,9 @@ bool table_value_constr::exec(SELECT_LEX *sl)
>    DBUG_ENTER("table_value_constr::exec");
>    List_iterator_fast<List_item> li(lists_of_values);
>    List_item *elem;
> +  THD *cur_thd= sl->parent_lex->thd;
>    ha_rows send_records= 0;
> +  int rc;
>    
>    if (select_options & SELECT_DESCRIBE)
>      DBUG_RETURN(false);
> @@ -438,16 +440,21 @@ bool table_value_constr::exec(SELECT_LEX *sl)
>  
>    while ((elem= li++))
>    {
> +    cur_thd->get_stmt_da()->inc_current_row_for_warning();
>      if (send_records >= sl->master_unit()->lim.get_select_limit())
> -      break;
> +      goto reset_counter_and_exit;

Why goto, break worked here just fine, didn't it?

Also, note that if you jump from here out of the loop, you'll do
if (rc>0) where rc wasn't initialized yet.

> -    int rc=
> -      result->send_data_with_check(*elem, sl->master_unit(), send_records);
> +    rc= result->send_data_with_check(*elem, sl->master_unit(), send_records);
>      if (!rc)
>        send_records++;
>      else if (rc > 0)
> -      DBUG_RETURN(true);
> +      goto reset_counter_and_exit;

break here too?

>    }
>  
> +reset_counter_and_exit:
> +  cur_thd->get_stmt_da()->reset_current_row_for_warning(0);

why do you need to reset here?

> +  if (rc>0)
> +    DBUG_RETURN(true);
> +
>    if (result->send_eof())
>      DBUG_RETURN(true);

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