maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Rucha!

Note test failures in buildbot, don't forget to update all result files
before pushing.

On Dec 28, Rucha Deodhar wrote:
> revision-id: 4774601a3cd (mariadb-10.2.40-192-g4774601a3cd)
> parent(s): 42fea34d4a9
> author: Rucha Deodhar
> committer: Rucha Deodhar
> timestamp: 2021-12-27 16:48:04 +0530
> message:
> 
> MDEV-26698: Incorrect row number upon INSERT .. SELECT from the same
> table: rows are counted twice
> 
> Analysis: When the table we are trying to insert into and the SELECT table
> are same for INSERT ... SELECT, rows from the SELECT table are copied into
> internal temporary table and then to the INSERT table. We only want to
> count the rows when we start inserting into the table.
> Fix: Reset the counter to 1 before starting to copy from internal temporary
> table to select table and then increment the counter.
> 
> diff --git a/sql/sql_select.cc b/sql/sql_select.cc
> index a331f4f3dbc..7a50f3d74ac 100644
> --- a/sql/sql_select.cc
> +++ b/sql/sql_select.cc
> @@ -3832,6 +3832,8 @@ mysql_select(THD *thd,
>      }
>    }
>  
> +  thd->get_stmt_da()->reset_current_row_for_warning();

What is this for? It wasn't needed before.

>    if ((err= join->optimize()))
>    {
>      goto err;					// 1
> @@ -19124,10 +19126,10 @@ evaluate_join_record(JOIN *join, JOIN_TAB *join_tab,
>  
>      if (found)
>      {
> +      unlock_row= false;
>        enum enum_nested_loop_state rc;
>        /* A match from join_tab is found for the current partial join. */
>        rc= (*join_tab->next_select)(join, join_tab+1, 0);
> -      join->thd->get_stmt_da()->inc_current_row_for_warning();

By moving this down you skip few returns, incl NESTED_LOOP_OK
and NESTED_LOOP_NO_MORE_ROWS. Are you sure it's fine?
Won't the counter be off, if the query execution continues after, say,
NESTED_LOOP_OK?

>        if (rc != NESTED_LOOP_OK && rc != NESTED_LOOP_NO_MORE_ROWS)
>          DBUG_RETURN(rc);
>        if (return_tab < join->return_tab)
> @@ -26945,6 +26943,12 @@ AGGR_OP::end_send()
>    table->reginfo.lock_type= TL_UNLOCK;
>  
>    bool in_first_read= true;
> +
> +  /*
> +     Reset the counter before copying rows from internal temporary table to
> +     INSERT table.

this code is also used for group by. Please, add a test for

  insert ... select ... group by

Why did put your fix here in a common code path in select, and not in,
say, select_insert::send_data() ?

> +  */
> +  join_tab->join->thd->get_stmt_da()->reset_current_row_for_warning();
>    while (rc == NESTED_LOOP_OK)
>    {
>      int error;
> @@ -26968,6 +26972,8 @@ AGGR_OP::end_send()
>      else
>      {
>        rc= evaluate_join_record(join, join_tab, 0);
> +      /* Increment the counter after copying rows. */
> +      join_tab->join->thd->get_stmt_da()->inc_current_row_for_warning();
>      }
>    }

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