From c96e06b11bad6f6336a5b7b040487c9192ffd146 Mon Sep 17 00:00:00 2001 From: Brandon Nesterenko Date: Wed, 17 Dec 2025 13:45:25 -0700 Subject: [PATCH 1/3] MDEV-38117 (test): Regression --- ...l_master_slave_mismatched_columns_ring.cnf | 12 ++ ..._master_slave_mismatched_columns_ring.test | 141 ++++++++++++++++++ 2 files changed, 153 insertions(+) create mode 100644 mysql-test/suite/rpl/t/rpl_master_slave_mismatched_columns_ring.cnf create mode 100644 mysql-test/suite/rpl/t/rpl_master_slave_mismatched_columns_ring.test diff --git a/mysql-test/suite/rpl/t/rpl_master_slave_mismatched_columns_ring.cnf b/mysql-test/suite/rpl/t/rpl_master_slave_mismatched_columns_ring.cnf new file mode 100644 index 0000000000000..1ce351ea2dbe9 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_master_slave_mismatched_columns_ring.cnf @@ -0,0 +1,12 @@ +!include ../my.cnf + +[mysqld.1] +gtid_domain_id=0 +server_id=1 +binlog_row_metadata=FULL + +[mysqld.2] +gtid_domain_id=1 +server_id=2 +log_slave_updates +binlog_row_metadata=FULL diff --git a/mysql-test/suite/rpl/t/rpl_master_slave_mismatched_columns_ring.test b/mysql-test/suite/rpl/t/rpl_master_slave_mismatched_columns_ring.test new file mode 100644 index 0000000000000..0b4a466c7194f --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_master_slave_mismatched_columns_ring.test @@ -0,0 +1,141 @@ +# +# Test ensures that, in a ring-replication topology, if a table has different +# column layouts on different servers, replication still works. +# +# References: +# MDEV-36290: ALTER TABLE with multi-master can cause data loss +# MDEV-38117: Replication stops with ERROR when Primary Key is not defined +# in Multi Master +# +--source include/have_binlog_format_row.inc +--source include/have_innodb.inc +--let $rpl_topology= 1->2->1 +--source include/rpl_init.inc + +--echo # +--echo # Initialize test data +--connection server_1 +create table t1 ( id int, a int, b int) engine=innodb; +--source include/rpl_sync.inc + +--echo # Change table layouts between servers +--connection server_1 +set session sql_log_bin=0; +alter table t1 add column m1col int after a; +set session sql_log_bin=1; +--connection server_2 +set session sql_log_bin=0; +alter table t1 add column m2col int after a; +set session sql_log_bin=1; + +--echo # Populate data on different servers +--connection server_1 +insert into t1 values(10,10,10,10); +insert into t1 values(11,11,11,11); +--connection server_2 +insert into t1 values(20,20,20,20); +insert into t1 values(21,21,21,21); +--source include/rpl_sync.inc + +--echo # Server_1 should have full data rows for self-inserted rows, and +--echo # replicated rows should have NULL for its mismatched column.. +--connection server_1 +--let $server_1_rows_with_m1col_values= `select count(*) from t1 where m1col is not null` +--let $server_1_rows_with_m1col_null= `select count(*) from t1 where m1col is null` +if ($server_1_rows_with_m1col_values != 2) +{ + --connection server_1 + select * from t1; + --echo # ..FAIL : Expected 2 rows with m1col values; found $server_1_rows_with_m1col_values + --die +} +if ($server_1_rows_with_m1col_null != 2) +{ + --connection server_1 + select * from t1; + --echo # ..FAIL : Expected 2 rows with m1col NULL; found $server_1_rows_with_m1col_null + --die +} +--echo # ..PASS + +--echo # Server_2 should have full data rows for self-inserted rows, and +--echo # replicated rows should have NULL for its mismatched column.. +--connection server_2 +--let $server_2_rows_with_m2col_values= `select count(*) from t1 where m2col is not null` +--let $server_2_rows_with_m2col_null= `select count(*) from t1 where m2col is null` +if ($server_2_rows_with_m2col_values != 2) +{ + --connection server_2 + select * from t1; + --echo # ..FAIL : Expected 2 rows with m2col values; found $server_2_rows_with_m2col_values + --die +} +if ($server_2_rows_with_m2col_null != 2) +{ + --connection server_2 + select * from t1; + --echo # ..FAIL : Expected 2 rows with m2col NULL; found $server_2_rows_with_m2col_null + --die +} +--echo # ..PASS + + +--echo # +--echo # Test that UPDATE can replicate.. +--connection server_1 +update t1 set a=14 where m1col=10; +--source include/rpl_sync.inc + +--connection server_1 +--let $server_1_a_is_14_count= `select count(*) from t1 where a=14` +if ($server_1_a_is_14_count != 1) +{ + --connection server_1 + select * from t1; + --echo # ..FAIL : Expected 1 row with a=14 on server_1; found $server_1_a_is_14_count + --die +} + +--let $server_2_a_is_14_count= `select count(*) from t1 where a=14` +if ($server_1_a_is_14_count != $server_2_a_is_14_count) +{ + --connection server_2 + select * from t1; + --echo # ..FAIL : server_1 and server_2 should both have a row with a=14 + --die +} +--echo # ..PASS + + +--echo # +--echo # Test that DELETE can replicate +--connection server_1 +delete from t1 where m1col is NULL; +--source include/rpl_sync.inc + +--let $server_1_n_rows= `select count(*) from t1` +if ($server_1_n_rows != 2) +{ + --connection server_1 + select * from t1; + --echo # ..FAIL : Expected 2 rows on server_1; found $server_1_n_rows + --die +} + +--let $server_2_n_rows= `select count(*) from t1` +if ($server_1_n_rows != $server_2_n_rows) +{ + --connection server_2 + select * from t1; + --echo # ..FAIL : server_1 and server_2 should both have 2 rows + --die +} +--echo # ..PASS + + +--echo # +--echo # Cleanup +--connection server_1 +drop table t1; + +--source include/rpl_end.inc From e36155fd1622c3dc61977925c20e0ab6ab79242b Mon Sep 17 00:00:00 2001 From: Brandon Nesterenko Date: Wed, 17 Dec 2025 13:45:05 -0700 Subject: [PATCH 2/3] MDEV-38117: Replication stops with ERROR when Primary Key is not defined in Multi Master When replicating tables with different structures between a master and slave (via binlog_row_metadata=FULL, i.e. MDEV-36290), if the table didn't have any keys, replication could break from a Can't find record in '', Error_code: 1032; error. This was due to the table's internal tracking of provided values (i.e. TABLE::has_value_set) persisting across transactions. I.e., when applying a new row event, the has_value_set bitset would start in a state indicative of the last event. Then, if the new row event required first finding a row, the has_value_set could represent a larger bitset than what was actually unpacked from the master. More specifically, if the last row event inserted something into the table, where slave-only columns would be given default/auto-generated values, these would update the TABLE::has_value_set. However, when the next row event would come in, those auto-generated values from the last transaction would not be present, but the row-search would include them to look-up the row, and not be able to find anything. This is fixed by resetting the table's internal state of provided values (TABLE::has_value_set) before unpacking the row data. Doing so revealed a bug where unpacked fields which explicitly provided NULL values would skip indicating that an explicit value was provided (and thereby couldn't be found by find_row()). To fix this, the slave-side field will always call has_explicit_value() if it is present in the packed data. Test case based on work from: Deepthi Sreenivas Signed-off-by: Brandon Nesterenko Reviewed-by: TODO --- ...aster_slave_mismatched_columns_ring.result | 49 +++++++++++++++++++ sql/rpl_record.cc | 16 +++--- 2 files changed, 59 insertions(+), 6 deletions(-) create mode 100644 mysql-test/suite/rpl/r/rpl_master_slave_mismatched_columns_ring.result diff --git a/mysql-test/suite/rpl/r/rpl_master_slave_mismatched_columns_ring.result b/mysql-test/suite/rpl/r/rpl_master_slave_mismatched_columns_ring.result new file mode 100644 index 0000000000000..ef774f5088b84 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_master_slave_mismatched_columns_ring.result @@ -0,0 +1,49 @@ +include/rpl_init.inc [topology=1->2->1] +# +# Initialize test data +connection server_1; +create table t1 ( id int, a int, b int) engine=innodb; +include/rpl_sync.inc +# Change table layouts between servers +connection server_1; +set session sql_log_bin=0; +alter table t1 add column m1col int after a; +set session sql_log_bin=1; +connection server_2; +set session sql_log_bin=0; +alter table t1 add column m2col int after a; +set session sql_log_bin=1; +# Populate data on different servers +connection server_1; +insert into t1 values(10,10,10,10); +insert into t1 values(11,11,11,11); +connection server_2; +insert into t1 values(20,20,20,20); +insert into t1 values(21,21,21,21); +include/rpl_sync.inc +# Server_1 should have full data rows for self-inserted rows, and +# replicated rows should have NULL for its mismatched column.. +connection server_1; +# ..PASS +# Server_2 should have full data rows for self-inserted rows, and +# replicated rows should have NULL for its mismatched column.. +connection server_2; +# ..PASS +# +# Test that UPDATE can replicate.. +connection server_1; +update t1 set a=14 where m1col=10; +include/rpl_sync.inc +connection server_1; +# ..PASS +# +# Test that DELETE can replicate +connection server_1; +delete from t1 where m1col is NULL; +include/rpl_sync.inc +# ..PASS +# +# Cleanup +connection server_1; +drop table t1; +include/rpl_end.inc diff --git a/sql/rpl_record.cc b/sql/rpl_record.cc index b761968533328..054fb7ecccf31 100644 --- a/sql/rpl_record.cc +++ b/sql/rpl_record.cc @@ -351,6 +351,14 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const master_cols, DBUG_PRINT("rpl_record", ("Table data: tabldef: %p, conv_table: %p", tabledef, conv_table)); + /* + For each field that is unpacked, we mark it as having an explicit value + (via Field::set_has_explicit_value()). So we need to reset the table's + internal tracking of fields with explicit values provided to ensure the + end state is consistent with the fields that are actually unpacked. + */ + table->reset_default_fields(); + /* When unpacking a row for replication (rather than online alter), it needs its own additional checks. The slave must account for its tables having @@ -415,6 +423,7 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const master_cols, DBUG_ASSERT(bitmap_is_set(table->write_set, slave_idx) || bitmap_is_set(table->read_set, slave_idx)); result_field= field= table->field[slave_idx]; + result_field->set_has_explicit_value(); /* Check 3: Skip unpacking if NULL is explicitly provided for the field. @@ -427,17 +436,12 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const master_cols, conv_table_idx++; continue; } + result_field->set_notnull(); /* Phase 2: Unpack the actual value into the slave table with any necessary conversions. - */ - /* Set attributes for the slave-side field */ - result_field->set_has_explicit_value(); - result_field->set_notnull(); - - /* If there is a conversion table, we pick up the field pointer to the conversion table. If the conversion table or the field pointer is NULL, no conversions are necessary. From 9bb985e3be738087a87a9ca334c6fab93a3e3814 Mon Sep 17 00:00:00 2001 From: Brandon Nesterenko Date: Fri, 2 Jan 2026 15:49:13 -0700 Subject: [PATCH 3/3] MDEV-38117: Address review round 1 * Move table->reset_default_fields() up to the Row_event level so it isn't done for each unpacked row * Add test to show that NULL values would result in data divergence between master and slave when using default functions --- ...rpl_default_functions_explicit_null.result | 89 +++++++++++++ .../t/rpl_default_functions_explicit_null.cnf | 12 ++ .../rpl_default_functions_explicit_null.test | 121 ++++++++++++++++++ sql/log_event_server.cc | 9 ++ sql/rpl_record.cc | 8 -- 5 files changed, 231 insertions(+), 8 deletions(-) create mode 100644 mysql-test/suite/rpl/r/rpl_default_functions_explicit_null.result create mode 100644 mysql-test/suite/rpl/t/rpl_default_functions_explicit_null.cnf create mode 100644 mysql-test/suite/rpl/t/rpl_default_functions_explicit_null.test diff --git a/mysql-test/suite/rpl/r/rpl_default_functions_explicit_null.result b/mysql-test/suite/rpl/r/rpl_default_functions_explicit_null.result new file mode 100644 index 0000000000000..9343062f29dd5 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_default_functions_explicit_null.result @@ -0,0 +1,89 @@ +include/master-slave.inc +[connection master] +# +# Initialize test data +connection master; +create table t1 (a int primary key) engine=innodb; +create table t2 (a int primary key, b int default (a+10), c int default (a + 100), d int default (a + 1000)) engine=innodb; +include/rpl_sync.inc +connection master; +use test; +create function f_insert_defaults_first() returns integer +begin +insert into test.t2 (a) values(1); +insert into test.t2 (a, b) values(2, 10000); +insert into test.t2 (a, c) values(3, 10000); +insert into test.t2 (a, d) values(4, 10000); +insert into test.t2 values(5, 10000, NULL, NULL); +insert into test.t2 values(6, NULL, 10000, NULL); +insert into test.t2 values(7, NULL, NULL, 10000); +insert into test.t2 values(8, NULL, NULL, NULL); +return 1; +end// +create function f_insert_nulls_first() returns integer +begin +insert into test.t2 values(9, 20000, NULL, NULL); +insert into test.t2 values(10, NULL, 20000, NULL); +insert into test.t2 values(11, NULL, NULL, 20000); +insert into test.t2 values(12, NULL, NULL, NULL); +insert into test.t2 (a) values(13); +insert into test.t2 (a, b) values(14, 20000); +insert into test.t2 (a, c) values(15, 20000); +insert into test.t2 (a, d) values(16, 20000); +return 2; +end// +create function f_update() returns integer +begin +update test.t2 set b=b+1; +update test.t2 set c=c+1; +update test.t2 set d=d+1; +return 3; +end// +create function f_delete() returns integer +begin +delete from test.t2; +return 4; +end// +# Ensuring INSERT replicates with default functions (defaults first) +connection master; +insert into t1 values(f_insert_defaults_first()); +include/rpl_sync.inc +include/diff_tables.inc [master:t1, slave:t1] +include/diff_tables.inc [master:t2, slave:t2] +# ..PASS +# Ensuring INSERT replicates with default functions (nulls first) +connection master; +insert into t1 values(f_insert_nulls_first()); +include/rpl_sync.inc +include/diff_tables.inc [master:t1, slave:t1] +include/diff_tables.inc [master:t2, slave:t2] +# ..PASS +# Ensuring UPDATE replicates with default functions.. +connection master; +insert into t1 values(f_update()); +include/rpl_sync.inc +include/diff_tables.inc [master:t1, slave:t1] +include/diff_tables.inc [master:t2, slave:t2] +# ..PASS +connection master; +# Ensuring DELETE replicates with default functions.. +connection master; +insert into t1 values(f_delete()); +include/rpl_sync.inc +include/diff_tables.inc [master:t1, slave:t1] +include/diff_tables.inc [master:t2, slave:t2] +# ..PASS +# +# Cleanup +connection master; +drop table t1; +drop table t2; +drop function f_insert_defaults_first; +drop function f_insert_nulls_first; +drop function f_update; +drop function f_delete; +include/save_master_gtid.inc +connection slave; +include/sync_with_master_gtid.inc +include/rpl_end.inc +# End of rpl_default_functions_explicit_null.test diff --git a/mysql-test/suite/rpl/t/rpl_default_functions_explicit_null.cnf b/mysql-test/suite/rpl/t/rpl_default_functions_explicit_null.cnf new file mode 100644 index 0000000000000..b9e1ab901854e --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_default_functions_explicit_null.cnf @@ -0,0 +1,12 @@ +!include ../my.cnf + +[mysqld.1] +log-warnings=9 +binlog_row_metadata=FULL +binlog_row_image=full + + +[mysqld.2] +log-warnings=9 +binlog_row_metadata=FULL +binlog_row_image=full diff --git a/mysql-test/suite/rpl/t/rpl_default_functions_explicit_null.test b/mysql-test/suite/rpl/t/rpl_default_functions_explicit_null.test new file mode 100644 index 0000000000000..a6a10e53b8f96 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_default_functions_explicit_null.test @@ -0,0 +1,121 @@ +# +# Test ensures that tables with default functions replicate correctly when +# NULL values are explicitly provided. Test uses stored procedures to ensure +# multiple rows exist within the Row_log_event with differing NULL value +# placements. +# +# References: +# MDEV-36290: ALTER TABLE with multi-master can cause data loss +# +--source include/have_innodb.inc +--source include/have_binlog_format_row.inc +--source include/master-slave.inc + +--echo # +--echo # Initialize test data +--connection master +create table t1 (a int primary key) engine=innodb; +create table t2 (a int primary key, b int default (a+10), c int default (a + 100), d int default (a + 1000)) engine=innodb; +--source include/rpl_sync.inc + +--connection master +use test; +delimiter //; +create function f_insert_defaults_first() returns integer +begin + insert into test.t2 (a) values(1); + insert into test.t2 (a, b) values(2, 10000); + insert into test.t2 (a, c) values(3, 10000); + insert into test.t2 (a, d) values(4, 10000); + insert into test.t2 values(5, 10000, NULL, NULL); + insert into test.t2 values(6, NULL, 10000, NULL); + insert into test.t2 values(7, NULL, NULL, 10000); + insert into test.t2 values(8, NULL, NULL, NULL); + return 1; +end// + +create function f_insert_nulls_first() returns integer +begin + insert into test.t2 values(9, 20000, NULL, NULL); + insert into test.t2 values(10, NULL, 20000, NULL); + insert into test.t2 values(11, NULL, NULL, 20000); + insert into test.t2 values(12, NULL, NULL, NULL); + insert into test.t2 (a) values(13); + insert into test.t2 (a, b) values(14, 20000); + insert into test.t2 (a, c) values(15, 20000); + insert into test.t2 (a, d) values(16, 20000); + return 2; +end// + +create function f_update() returns integer +begin + update test.t2 set b=b+1; + update test.t2 set c=c+1; + update test.t2 set d=d+1; + return 3; +end// + +create function f_delete() returns integer +begin + delete from test.t2; + return 4; +end// +delimiter ;// + +--echo # Ensuring INSERT replicates with default functions (defaults first) +--connection master +insert into t1 values(f_insert_defaults_first()); +--source include/rpl_sync.inc +--let $diff_tables= master:t1, slave:t1 +--source include/diff_tables.inc +--let $diff_tables= master:t2, slave:t2 +--source include/diff_tables.inc +--echo # ..PASS + +--echo # Ensuring INSERT replicates with default functions (nulls first) +--connection master +insert into t1 values(f_insert_nulls_first()); +--source include/rpl_sync.inc +--let $diff_tables= master:t1, slave:t1 +--source include/diff_tables.inc +--let $diff_tables= master:t2, slave:t2 +--source include/diff_tables.inc +--echo # ..PASS + +--echo # Ensuring UPDATE replicates with default functions.. +--connection master +insert into t1 values(f_update()); +--source include/rpl_sync.inc +--let $diff_tables= master:t1, slave:t1 +--source include/diff_tables.inc +--let $diff_tables= master:t2, slave:t2 +--source include/diff_tables.inc +--echo # ..PASS +--connection master + +--echo # Ensuring DELETE replicates with default functions.. +--connection master +insert into t1 values(f_delete()); +--source include/rpl_sync.inc +--let $diff_tables= master:t1, slave:t1 +--source include/diff_tables.inc +--let $diff_tables= master:t2, slave:t2 +--source include/diff_tables.inc +--echo # ..PASS + +--echo # +--echo # Cleanup +--connection master +drop table t1; +drop table t2; +drop function f_insert_defaults_first; +drop function f_insert_nulls_first; +drop function f_update; +drop function f_delete; +--source include/save_master_gtid.inc + +--connection slave +--source include/sync_with_master_gtid.inc + +--source include/rpl_end.inc +--echo # End of rpl_default_functions_explicit_null.test diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc index af34c2eea671b..a27eb729eb3b1 100644 --- a/sql/log_event_server.cc +++ b/sql/log_event_server.cc @@ -5221,6 +5221,15 @@ int Rows_log_event::do_apply_event(rpl_group_info *rgi) DBUG_ASSERT(rpl_table); DBUG_ASSERT(rpl_table == rgi->get_table_data(table)); + /* + For each field that is unpacked, it will be marked as having an + explicit value (via Field::set_has_explicit_value() in unpack_row()). + So we need to reset the table's internal tracking of fields with + explicit values provided to ensure the end state is consistent with + the fields that are actually unpacked. + */ + table->reset_default_fields(); + bitmap_set_all(table->read_set); bitmap_set_all(table->write_set); table->rpl_write_set= table->write_set; diff --git a/sql/rpl_record.cc b/sql/rpl_record.cc index 054fb7ecccf31..3722921a7835e 100644 --- a/sql/rpl_record.cc +++ b/sql/rpl_record.cc @@ -351,14 +351,6 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const master_cols, DBUG_PRINT("rpl_record", ("Table data: tabldef: %p, conv_table: %p", tabledef, conv_table)); - /* - For each field that is unpacked, we mark it as having an explicit value - (via Field::set_has_explicit_value()). So we need to reset the table's - internal tracking of fields with explicit values provided to ensure the - end state is consistent with the fields that are actually unpacked. - */ - table->reset_default_fields(); - /* When unpacking a row for replication (rather than online alter), it needs its own additional checks. The slave must account for its tables having