maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Rucha!

On Oct 17, Rucha Deodhar wrote:
> revision-id: a895fce3277 (mariadb-10.6.1-138-ga895fce3277)
> parent(s): c27f04ede5a
> author: Rucha Deodhar
> committer: Rucha Deodhar
> timestamp: 2021-10-17 17:59:11 +0530
> message:
> 
> MDEV-26842: MDEV-26842: ROW_NUMBER is not set and differs from the message
> upon WARN_DATA_TRUNCATED produced by inplace ALTER
> 
> Analysis: When row number is passed as parameter to set_warning() it is only
> used for error/warning text but m_current_row_for_warning is not updated.
> Hence default value of m_current_row_for_warning is assumed.
> Fix: update m_current_row_for_warning when error/warning occurs.
> 
> diff --git a/sql/field.cc b/sql/field.cc
> index a5cd595f05b..048798ea509 100644
> --- a/sql/field.cc
> +++ b/sql/field.cc
> @@ -11086,6 +11086,10 @@ Field::set_warning(Sql_condition::enum_warning_level level, uint code,
>      will have table == NULL.
>    */
>    THD *thd= get_thd();
> +    ulong row_count= current_row ? current_row
> +                   : thd->get_stmt_da()->current_row_for_warning();
> +
> +  thd->get_stmt_da()->reset_current_row_for_warning(row_count);

Isn't it a bit confusing, why do you set current_row_for_warning to its
own actual value? May be it'd be clearer to

   if (current_row)
     thd->get_stmt_da()->reset_current_row_for_warning(current_row);

And also, please, mention either here or in the commit comment that it's
for INPLACE alter, where the server cannot know what row has generated
the warning, so the value of current_row is supplied by the engine.

>    if (thd->count_cuted_fields > CHECK_FIELD_EXPRESSION)
>    {
>      thd->cuted_fields+= cut_increment;
> 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx