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
>
> 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..."
> +
> 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.
2. why MYSQL_VERSION_ID<110201 ?
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.
> +#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
