maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Sachin!

ok to push with one change, see below.

On Jun 03, Sachin wrote:
> revision-id: 0552ed86782 (mariadb-10.4.11-223-g0552ed86782)
> parent(s): 61b2cd38d48
> author: Sachin <sachin.setiya@xxxxxxxxxxx>
> committer: Sachin <sachin.setiya@xxxxxxxxxxx>
> timestamp: 2020-06-02 08:24:02 +0530
> message:
> 
> MDEV-21804 Assertion `marked_for_read()' failed upon INSERT into table with long unique blob under binlog_row_image=NOBLOB
> 
> Problem:- Calling mark_columns_per_binlog_row_image() earlier may change the
> result of mark_virtual_columns_for_write() , Since it can set the bitmap on
> for virtual column, and henceforth  mark_virtual_column_deps(field) will
> never be called in mark_virtual_column_with_deps.
> 
> This bug is not specific for long unique, It also fails for this case
>    create table t2(id int primary key, a blob, b varchar(20) as (LEFT(a,2)));
> 
> diff --git a/sql/table.cc b/sql/table.cc
> index bc8ddbc0f9d..8edbbe8d599 100644
> --- a/sql/table.cc
> +++ b/sql/table.cc
> @@ -7040,7 +7040,6 @@ void TABLE::mark_columns_needed_for_update()
>    DBUG_ENTER("TABLE::mark_columns_needed_for_update");
>    bool need_signal= false;
>  
> -  mark_columns_per_binlog_row_image();
>  
>    if (triggers)
>      triggers->mark_fields_used(TRG_EVENT_UPDATE);
> @@ -7048,6 +7047,7 @@ void TABLE::mark_columns_needed_for_update()
>      mark_default_fields_for_write(FALSE);
>    if (vfield)
>      need_signal|= mark_virtual_columns_for_write(FALSE);
> +  mark_columns_per_binlog_row_image();

I think that's too early. In that if() below you can see

>    if (file->ha_table_flags() & HA_REQUIRES_KEY_COLUMNS_FOR_DELETE)
>    {
>      KEY *end= key_info + s->keys;

  ...
          if (bitmap_fast_test_and_set(read_set, idx))
            continue;
          if (field[idx]->vcol_info)
            mark_virtual_col(field[idx]);

and mark_columns_per_binlog_row_image() might break that.

Better to put mark_columns_per_binlog_row_image() at the very end, just
before signalling.

> @@ -7132,7 +7132,6 @@ void TABLE::mark_columns_needed_for_update()
>  void TABLE::mark_columns_needed_for_insert()
>  {
>    DBUG_ENTER("mark_columns_needed_for_insert");
> -  mark_columns_per_binlog_row_image();
>  
>    if (triggers)
>    {
> @@ -7152,6 +7151,7 @@ void TABLE::mark_columns_needed_for_insert()
>    /* Mark virtual columns for insert */
>    if (vfield)
>      mark_virtual_columns_for_write(TRUE);
> +  mark_columns_per_binlog_row_image();

this looks ok.

>    if (check_constraints)
>      mark_check_constraint_columns_for_read();
>    DBUG_VOID_RETURN;

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