Re: 0ddb9744cea: MDEV-33957 UPDATE fails on replica replicating blob virtual column in NOBLOB mode when replica logging is off
Hi, Aleksey, On Jun 23, Aleksey Midenkov wrote:
commit 0ddb9744cea Author: Aleksey Midenkov <midenok@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 security@mariadb.org
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 <midenok@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 security@mariadb.org _______________________________________________ developers mailing list -- developers@lists.mariadb.org To unsubscribe send an email to developers-leave@lists.mariadb.org
-- @midenok
participants (2)
-
Aleksey Midenkov -
Sergei Golubchik