Hi,
On Mon, Jun 23, 2025 at 7:30 PM Sergei Golubchik via developers <
developers@lists.mariadb.org> wrote:
> Hi, Aleksey,
>
> On Jun 23, Aleksey Midenkov wrote:
> > commit 0ddb9744cea
> > Author: Aleksey Midenkov <mide...@mariadb.com>
> > Date: Sun May 25 23:23:29 2025 +0300
> >
> > MDEV-33957 UPDATE fails on replica replicating blob virtual column in
> > NOBLOB mode when replica logging is off
>
> please, put the whole commit subject in one line. various tools
> assume the subject is one line only and don't handle line wrapping
>
Done.
>
> >
> > The sequence that causes the issue:
> >
> > 1. file->row_logging is false because slave-bin was not open;
> > 2. TABLE::mark_columns_per_binlog_row_image() didn't mark column for
> > read because file->row_logginbg is false. This was implemented in
> > e53ad95b733 (MDEV-6877);
> > 3. TABLE::update_virtual_fields() didn't update virtual field value
> > because column is not marked for read;
> > 4. calc_row_difference() sees o_len as UNIV_SQL_NULL, but new row
> > value is "1". The virtual column is added to update vector;
> > 5. row_upd() tries to update secondary index, but row_upd_sec_step()
> > doesn't see old value in the index.
> >
> > The patch does mark_virtual_column_with_deps() in case of rgi_slave
> in
> > mark_columns_per_binlog_row_image() so that non-stored virtual
> columns
> > are marked for update in slave thread.
> >
> > diff --git a/sql/ha_partition.h b/sql/ha_partition.h
> > index 01d15ca70ac..d926bd03137 100644
> > --- a/sql/ha_partition.h
> > +++ b/sql/ha_partition.h
> > @@ -576,6 +576,17 @@ class ha_partition final :public handler
> > {
> > m_file[part_id]->update_create_info(create_info);
> > }
> > +
> > + void column_bitmaps_signal() override
> > + {
> > + for (uint i= bitmap_get_first_set(&m_opened_partitions);
> > + i < m_tot_parts;
> > + i= bitmap_get_next_set(&m_opened_partitions, i))
> > + {
> > + m_file[i]->column_bitmaps_signal();
> > + }
> > + }
>
> That's a good one. Although it's not really related to the bug in question.
> Please at least mention this in the commit comment, like "also fixed..."
>
Done.
>
> > +
> > private:
> > int copy_partitions(ulonglong * const copied, ulonglong * const
> deleted);
> > void cleanup_new_partition(uint part_count);
> > diff --git a/sql/table.cc b/sql/table.cc
> > index 3a51228d1f3..b48aaadb6e1 100644
> > --- a/sql/table.cc
> > +++ b/sql/table.cc
> > @@ -8019,6 +8024,12 @@ void TABLE::mark_columns_per_binlog_row_image()
> > }
> > file->column_bitmaps_signal();
> > }
> > +#if MYSQL_VERSION_ID<110201
> > + else if (thd->rgi_slave)
> > + {
> > + file->column_bitmaps_signal();
> > + }
>
> 1. This doesn't match the commit comment, please update the latter to
> describe the actual code change.
>
No, mark_virtual_column_with_deps() is what the patch does (via
column_bitmaps_signal()). Updated the comment to improve clarity.
>
> 2. why MYSQL_VERSION_ID<110201 ?
>
Because 11.2 has another fix 9545eb969 which does not fit 11.1 (as it
depends on Online Alter).
>
> 3. you can also move `rpl_write_set= write_set` from the beginning
> of the function here, inside the else branch. This makes the code
> clearer: rpl_write_set was changed, so we have to signal it.
>
I should remove MYSQL_VERSION_ID and rgi_slave then. That really looks more
clear.
New version:
1ffbee16e86 (HEAD -> bb-10.11-midenok3, mariadb/bb-10.11-midenok3)
MDEV-33957 UPDATE fails on replica replicating blob virtual column in
NOBLOB mode when replica logging is off
> > +#endif
> >
> > DBUG_VOID_RETURN;
> > }
>
> Regards,
> Sergei
> Chief Architect, MariaDB Server
> and secur...@mariadb.org
> _______________________________________________
> developers mailing list -- developers@lists.mariadb.org
> To unsubscribe send an email to developers-le...@lists.mariadb.org
>
--
@midenok
_______________________________________________
developers mailing list -- developers@lists.mariadb.org
To unsubscribe send an email to developers-le...@lists.mariadb.org
