December 2024 - commits - lists.mariadb.org

[PATCH] MDEV-35233: RBR does not work with CSV tables
by Kristian Nielsen 04 Dec '24

04 Dec '24

Handle null bits for record comparison in row events the same way as in handler::calculate_checksum(), forcing bits that can be undefined to 1. These bits are the trailing unused bits, as well as the first bit for tables not using HA_OPTION_PACK_RECORD. The csv storage engine leaves these bits at 0, while the row-based replication has them set to 1, which otherwise cause can't find record error. Signed-off-by: Kristian Nielsen <knielsen(a)knielsen-hq.org> --- mysql-test/suite/rpl/r/rpl_csv.result | 13 +++++++ mysql-test/suite/rpl/t/rpl_csv.test | 16 ++++++++ sql/log_event_server.cc | 54 ++++++++++++++++++++++----- 3 files changed, 73 insertions(+), 10 deletions(-) create mode 100644 mysql-test/suite/rpl/r/rpl_csv.result create mode 100644 mysql-test/suite/rpl/t/rpl_csv.test diff --git a/mysql-test/suite/rpl/r/rpl_csv.result b/mysql-test/suite/rpl/r/rpl_csv.result new file mode 100644 index 00000000000..39b78186df3 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_csv.result @@ -0,0 +1,13 @@ +include/master-slave.inc +[connection master] +*** MDEV-35233: RBR does not work with CSV tables +CREATE TABLE t (a INT NOT NULL) ENGINE=CSV; +INSERT INTO t VALUES (1),(2); +DELETE FROM t where a=1; +connection slave; +SELECT * FROM t ORDER BY a; +a +2 +connection master; +DROP TABLE t; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_csv.test b/mysql-test/suite/rpl/t/rpl_csv.test new file mode 100644 index 00000000000..82f745d549e --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_csv.test @@ -0,0 +1,16 @@ +--source include/have_csv.inc +--source include/have_binlog_format_row.inc +--source include/master-slave.inc + +--echo *** MDEV-35233: RBR does not work with CSV tables +CREATE TABLE t (a INT NOT NULL) ENGINE=CSV; +INSERT INTO t VALUES (1),(2); +DELETE FROM t where a=1; + +--sync_slave_with_master +SELECT * FROM t ORDER BY a; + +# Cleanup +--connection master +DROP TABLE t; +--source include/rpl_end.inc diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc index 8b976ba5e14..d6617e8fb68 100644 --- a/sql/log_event_server.cc +++ b/sql/log_event_server.cc @@ -6646,6 +6646,46 @@ last_uniq_key(TABLE *table, uint keyno) return 1; } + +/* + We need to set the null bytes to ensure that the filler bit are + all set when returning. There are storage engines that just set + the necessary bits on the bytes and don't set the filler bits + correctly. +*/ +static void +normalize_null_bits(TABLE *table) +{ + if (table->s->null_bytes > 0) + { + DBUG_ASSERT(table->s->last_null_bit_pos < 8); + /* + Normalize any unused null bits. + + We need to set the highest (8 - last_null_bit_pos) bits to 1, except that + if last_null_bit_pos is 0 then there are no unused bits and we should set + no bits to 1. + + When N = last_null_bit_pos != 0, we can get a mask for this with + + 0xff << N = (0xff << 1) << (N-1) = 0xfe << (N-1) = 0xfe << ((N-1) & 7) + + And we can get a mask=0 for the case N = last_null_bit_pos = 0 with + + 0xfe << 7 = 0xfe << ((N-1) & 7) + + Thus we can set the desired bits in all cases by OR-ing with + (0xfe << ((N-1) & 7)), avoiding a conditional jump. + */ + table->record[0][table->s->null_bytes - 1]|= + (uchar)(0xfe << ((table->s->last_null_bit_pos - 1) & 7)); + /* Normalize the delete marker bit, if any. */ + table->record[0][0]|= + !(table->s->db_create_options & HA_OPTION_PACK_RECORD); + } +} + + /** Check if an error is a duplicate key error. @@ -7089,6 +7129,8 @@ static bool record_compare(TABLE *table, bool vers_from_plain= false) bool result= false; bool all_values_set= bitmap_is_set_all(&table->has_value_set); + normalize_null_bits(table); + /** Compare full record only if: - all fields were given values @@ -7552,6 +7594,8 @@ int Rows_log_event::find_row(rpl_group_info *rgi) // We can't use position() - try other methods. + normalize_null_bits(table); + /* Save copy of the record in table->record[1]. It might be needed later if linear search is used to find exact match. @@ -7588,16 +7632,6 @@ int Rows_log_event::find_row(rpl_group_info *rgi) DBUG_DUMP("key data", m_key, m_key_info->key_length); #endif - /* - We need to set the null bytes to ensure that the filler bit are - all set when returning. There are storage engines that just set - the necessary bits on the bytes and don't set the filler bits - correctly. - */ - if (table->s->null_bytes > 0) - table->record[0][table->s->null_bytes - 1]|= - 256U - (1U << table->s->last_null_bit_pos); - const enum ha_rkey_function find_flag= m_usable_key_parts == m_key_info->user_defined_key_parts ? HA_READ_KEY_EXACT : HA_READ_KEY_OR_NEXT; -- 2.39.5

1 0

[PATCH] MDEV-35233: RBR does not work with CSV tables
by Kristian Nielsen 03 Dec '24

03 Dec '24

Handle null bits for record comparison in row events the same way as in handler::calculate_checksum(), forcing bits that can be undefined to 1. These bits are the trailing unused bits, as well as the first bit for tables not using HA_OPTION_PACK_RECORD. The csv storage engine leaves these bits at 0, while the row-based replication has them set to 1, which otherwise cause can't find record error. Signed-off-by: Kristian Nielsen <knielsen(a)knielsen-hq.org> --- mysql-test/suite/rpl/r/rpl_csv.result | 13 +++++++++++ mysql-test/suite/rpl/t/rpl_csv.test | 16 ++++++++++++++ sql/log_event_server.cc | 32 ++++++++++++++++++--------- 3 files changed, 51 insertions(+), 10 deletions(-) create mode 100644 mysql-test/suite/rpl/r/rpl_csv.result create mode 100644 mysql-test/suite/rpl/t/rpl_csv.test diff --git a/mysql-test/suite/rpl/r/rpl_csv.result b/mysql-test/suite/rpl/r/rpl_csv.result new file mode 100644 index 00000000000..39b78186df3 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_csv.result @@ -0,0 +1,13 @@ +include/master-slave.inc +[connection master] +*** MDEV-35233: RBR does not work with CSV tables +CREATE TABLE t (a INT NOT NULL) ENGINE=CSV; +INSERT INTO t VALUES (1),(2); +DELETE FROM t where a=1; +connection slave; +SELECT * FROM t ORDER BY a; +a +2 +connection master; +DROP TABLE t; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_csv.test b/mysql-test/suite/rpl/t/rpl_csv.test new file mode 100644 index 00000000000..82f745d549e --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_csv.test @@ -0,0 +1,16 @@ +--source include/have_csv.inc +--source include/have_binlog_format_row.inc +--source include/master-slave.inc + +--echo *** MDEV-35233: RBR does not work with CSV tables +CREATE TABLE t (a INT NOT NULL) ENGINE=CSV; +INSERT INTO t VALUES (1),(2); +DELETE FROM t where a=1; + +--sync_slave_with_master +SELECT * FROM t ORDER BY a; + +# Cleanup +--connection master +DROP TABLE t; +--source include/rpl_end.inc diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc index 8b976ba5e14..d5773687e82 100644 --- a/sql/log_event_server.cc +++ b/sql/log_event_server.cc @@ -7089,6 +7089,14 @@ static bool record_compare(TABLE *table, bool vers_from_plain= false) bool result= false; bool all_values_set= bitmap_is_set_all(&table->has_value_set); + if (table->s->null_bytes > 0) + { + table->record[0][table->s->null_bytes - 1]|= + 256U - (1U << table->s->last_null_bit_pos); + if (!(table->s->db_create_options & HA_OPTION_PACK_RECORD)) + table->record[0][0] |= 1; + } + /** Compare full record only if: - all fields were given values @@ -7552,6 +7560,20 @@ int Rows_log_event::find_row(rpl_group_info *rgi) // We can't use position() - try other methods. + /* + We need to set the null bytes to ensure that the filler bit are + all set when returning. There are storage engines that just set + the necessary bits on the bytes and don't set the filler bits + correctly. + */ + if (table->s->null_bytes > 0) + { + table->record[0][table->s->null_bytes - 1]|= + 256U - (1U << table->s->last_null_bit_pos); + if (!(table->s->db_create_options & HA_OPTION_PACK_RECORD)) + table->record[0][0] |= 1; + } + /* Save copy of the record in table->record[1]. It might be needed later if linear search is used to find exact match. @@ -7588,16 +7610,6 @@ int Rows_log_event::find_row(rpl_group_info *rgi) DBUG_DUMP("key data", m_key, m_key_info->key_length); #endif - /* - We need to set the null bytes to ensure that the filler bit are - all set when returning. There are storage engines that just set - the necessary bits on the bytes and don't set the filler bits - correctly. - */ - if (table->s->null_bytes > 0) - table->record[0][table->s->null_bytes - 1]|= - 256U - (1U << table->s->last_null_bit_pos); - const enum ha_rkey_function find_flag= m_usable_key_parts == m_key_info->user_defined_key_parts ? HA_READ_KEY_EXACT : HA_READ_KEY_OR_NEXT; -- 2.39.5

1 0

[PATCH] MDEV-24959: ER_BINLOG_ROW_LOGGING_FAILED (1534: Writing one row to the row-based binary log failed)
by Kristian Nielsen 03 Dec '24

03 Dec '24

The Write_rows_log_event originally allocated the m_rows_buf up-front, and thus is_valid() checks that the buffer is allocated correctly. But at some point this was changed to allocate the buffer lazily on demand. This means that a a valid event can now have m_rows_buf==NULL. The is_valid() code was not changed, and thus is_valid() could return false on a valid event. This caused a bug for REPLACE INTO t() VALUES(), () which generates a write_rows event with no after image; then the m_rows_buf was never allocated and is_valid() incorrectly returned false, causing an error in some other parts of the code. Also fix a couple of missing special cases in the code for mysqlbinlog to correctly decode (in comments) row events with missing after image. Signed-off-by: Kristian Nielsen <knielsen(a)knielsen-hq.org> --- mysql-test/main/mysqlbinlog.result | 18 ++++++++++++ mysql-test/main/mysqlbinlog.test | 45 ++++++++++++++++++++++++++++++ sql/log_event.h | 2 +- sql/log_event_client.cc | 21 ++++++++++++-- 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/mysql-test/main/mysqlbinlog.result b/mysql-test/main/mysqlbinlog.result index 4e0f570c899..5bde2b05a98 100644 --- a/mysql-test/main/mysqlbinlog.result +++ b/mysql-test/main/mysqlbinlog.result @@ -1314,3 +1314,21 @@ a b 2 2023-07-22 00:36:20.567890 DROP TABLE t; SET time_zone= default; +# +# MDEV-24959: ER_BINLOG_ROW_LOGGING_FAILED (1534: Writing one row to the row-based binary log failed) +# +SET SESSION binlog_format= ROW; +SET SESSION binlog_row_image= MINIMAL; +RESET MASTER; +CREATE TABLE t1 (a INT NOT NULL DEFAULT 0 PRIMARY KEY); +REPLACE INTO t1 () VALUES (),(); +DROP TABLE t1; +FLUSH BINARY LOGS; +SET SESSION binlog_format= STATEMENT; +SET SESSION binlog_row_image= default; +FOUND 1 /Number of rows: 2/ in mdev24959_1.txt +FOUND 1 /DROP TABLE/ in mdev24959_1.txt +FOUND 1 /Number of rows: 2/ in mdev24959_2.txt +FOUND 1 /DROP TABLE/ in mdev24959_2.txt +FOUND 1 /INSERT INTO .* VALUES/ in mdev24959_2.txt +FOUND 1 /SET /[*] no columns [*]// in mdev24959_2.txt diff --git a/mysql-test/main/mysqlbinlog.test b/mysql-test/main/mysqlbinlog.test index 0e5fd9efb70..ceb6ff2eee6 100644 --- a/mysql-test/main/mysqlbinlog.test +++ b/mysql-test/main/mysqlbinlog.test @@ -671,3 +671,48 @@ SELECT * FROM t; SELECT * FROM t; DROP TABLE t; SET time_zone= default; + + +--echo # +--echo # MDEV-24959: ER_BINLOG_ROW_LOGGING_FAILED (1534: Writing one row to the row-based binary log failed) +--echo # + +SET SESSION binlog_format= ROW; +SET SESSION binlog_row_image= MINIMAL; + +RESET MASTER; +CREATE TABLE t1 (a INT NOT NULL DEFAULT 0 PRIMARY KEY); +REPLACE INTO t1 () VALUES (),(); +DROP TABLE t1; +FLUSH BINARY LOGS; +SET SESSION binlog_format= STATEMENT; +SET SESSION binlog_row_image= default; + +--exec $MYSQL_BINLOG --base64-output=decode-rows $MYSQLD_DATADIR/master-bin.000001 > $MYSQLTEST_VARDIR/tmp/mdev24959_1.txt +--exec $MYSQL_BINLOG --base64-output=decode-rows --verbose $MYSQLD_DATADIR/master-bin.000001 > $MYSQLTEST_VARDIR/tmp/mdev24959_2.txt + +--let SEARCH_FILE= $MYSQLTEST_VARDIR/tmp/mdev24959_1.txt +--let SEARCH_ABORT= NOT FOUND +--let SEARCH_PATTERN= Number of rows: 2 +--source include/search_pattern_in_file.inc + +# There was a bug that mysqlbinlog would get an error while decoding the +# update rows event with no after image and abort the dump; test that now +# the dump is complete and includes the final DROP TABLE. +--let SEARCH_PATTERN= DROP TABLE +--source include/search_pattern_in_file.inc + +--let SEARCH_FILE= $MYSQLTEST_VARDIR/tmp/mdev24959_2.txt +--let SEARCH_PATTERN= Number of rows: 2 +--source include/search_pattern_in_file.inc + +--let SEARCH_PATTERN= DROP TABLE +--source include/search_pattern_in_file.inc + +--let SEARCH_PATTERN= INSERT INTO .* VALUES +--source include/search_pattern_in_file.inc +--let SEARCH_PATTERN= SET /[*] no columns [*]/ +--source include/search_pattern_in_file.inc + +--remove_file $MYSQLTEST_VARDIR/tmp/mdev24959_1.txt +--remove_file $MYSQLTEST_VARDIR/tmp/mdev24959_2.txt diff --git a/sql/log_event.h b/sql/log_event.h index a869c6e04f1..0a1d6502932 100644 --- a/sql/log_event.h +++ b/sql/log_event.h @@ -5086,7 +5086,7 @@ class Rows_log_event : public Log_event */ bool is_valid() const override { - return m_rows_buf && m_cols.bitmap; + return m_cols.bitmap; } uint m_row_count; /* The number of rows added to the event */ diff --git a/sql/log_event_client.cc b/sql/log_event_client.cc index 720cc5ab611..a04776e6404 100644 --- a/sql/log_event_client.cc +++ b/sql/log_event_client.cc @@ -1384,6 +1384,13 @@ void Rows_log_event::count_row_events(PRINT_EVENT_INFO *print_event_info) switch (general_type_code) { case WRITE_ROWS_EVENT: + /* + A write rows event containing no after image (can happen for REPLACE + INTO t() VALUES ()), count this correctly as 1 row and no 0. + */ + if (unlikely(m_rows_buf == m_rows_end)) + print_event_info->row_events++; + /* Fall through. */ case DELETE_ROWS_EVENT: row_events= 1; break; @@ -1509,6 +1516,7 @@ bool Rows_log_event::print_verbose(IO_CACHE *file, /* If the write rows event contained no values for the AI */ if (((general_type_code == WRITE_ROWS_EVENT) && (m_rows_buf==m_rows_end))) { + print_event_info->row_events++; if (my_b_printf(file, "### INSERT INTO %`s.%`s VALUES ()\n", map->get_db_name(), map->get_table_name())) goto err; @@ -1542,9 +1550,16 @@ bool Rows_log_event::print_verbose(IO_CACHE *file, /* Print the second image (for UPDATE only) */ if (sql_clause2) { - if (!(length= print_verbose_one_row(file, td, print_event_info, - &m_cols_ai, value, - (const uchar*) sql_clause2))) + /* If the update rows event contained no values for the AI */ + if (unlikely(bitmap_is_clear_all(&m_cols_ai))) + { + length= (bitmap_bits_set(&m_cols_ai) + 7) / 8; + if (my_b_printf(file, "### SET /* no columns */\n")) + goto err; + } + else if (!(length= print_verbose_one_row(file, td, print_event_info, + &m_cols_ai, value, + (const uchar*) sql_clause2))) goto err; value+= length; } -- 2.39.5

1 0

[PATCH] MDEV-21858: START/STOP ALL SLAVES does not return access errors
by Kristian Nielsen 02 Dec '24

02 Dec '24

Check the user privileges and fail the command, even if there are no slaves that need starting respectively stopping. Signed-off-by: Kristian Nielsen <knielsen(a)knielsen-hq.org> --- mysql-test/suite/multi_source/simple.result | 28 +++++++++++++++ mysql-test/suite/multi_source/simple.test | 38 +++++++++++++++++++++ sql/rpl_mi.cc | 7 ++++ 3 files changed, 73 insertions(+) diff --git a/mysql-test/suite/multi_source/simple.result b/mysql-test/suite/multi_source/simple.result index f9f43d44ca7..da80b89c2ef 100644 --- a/mysql-test/suite/multi_source/simple.result +++ b/mysql-test/suite/multi_source/simple.result @@ -520,6 +520,34 @@ Slave_received_heartbeats 0 Slave_heartbeat_period 60.000 Gtid_Slave_Pos stop all slaves; +# +# MDEV-21858: START/STOP ALL SLAVES does not return access errors +# +connection slave; +SET SESSION sql_log_bin=0; +CREATE USER 'unpriv'@'127.0.0.1'; +GRANT USAGE ON *.* TO 'unpriv'@'127.0.0.1'; +connect con1,127.0.0.1,unpriv,,,$SERVER_MYPORT_3; +STOP SLAVE 'slave2'; +ERROR 42000: Access denied; you need (at least one of) the REPLICATION SLAVE ADMIN privilege(s) for this operation +START SLAVE 'slave2'; +ERROR 42000: Access denied; you need (at least one of) the REPLICATION SLAVE ADMIN privilege(s) for this operation +STOP ALL SLAVES; +ERROR 42000: Access denied; you need (at least one of) the REPLICATION SLAVE ADMIN privilege(s) for this operation +connection slave; +START SLAVE 'slave2'; +set default_master_connection = 'slave2'; +include/wait_for_slave_to_start.inc +connection con1; +START ALL SLAVES; +ERROR 42000: Access denied; you need (at least one of) the REPLICATION SLAVE ADMIN privilege(s) for this operation +disconnect con1; +connection slave; +STOP SLAVE 'slave2'; +set default_master_connection = 'slave2'; +include/wait_for_slave_to_stop.inc +DROP USER 'unpriv'@'127.0.0.1'; +SET SESSION sql_log_bin=1; include/reset_master_slave.inc disconnect slave; connection master1; diff --git a/mysql-test/suite/multi_source/simple.test b/mysql-test/suite/multi_source/simple.test index ac5e84586ae..5894a205480 100644 --- a/mysql-test/suite/multi_source/simple.test +++ b/mysql-test/suite/multi_source/simple.test @@ -84,6 +84,44 @@ query_vertical show all slaves status; # Ensure that start all slaves doesn't do anything as all slaves are stopped stop all slaves; +--echo # +--echo # MDEV-21858: START/STOP ALL SLAVES does not return access errors +--echo # +--connection slave +SET SESSION sql_log_bin=0; +CREATE USER 'unpriv'@'127.0.0.1'; +GRANT USAGE ON *.* TO 'unpriv'@'127.0.0.1'; + +connect (con1,127.0.0.1,unpriv,,,$SERVER_MYPORT_3); + +--error ER_SPECIFIC_ACCESS_DENIED_ERROR +STOP SLAVE 'slave2'; +--error ER_SPECIFIC_ACCESS_DENIED_ERROR +START SLAVE 'slave2'; + +# Test that STOP/START ALL SLAVES checks privileges, even if there are no +# slaves that need stopping or starting. +--error ER_SPECIFIC_ACCESS_DENIED_ERROR +STOP ALL SLAVES; + +--connection slave +START SLAVE 'slave2'; +set default_master_connection = 'slave2'; +--source include/wait_for_slave_to_start.inc + +--connection con1 +--error ER_SPECIFIC_ACCESS_DENIED_ERROR +START ALL SLAVES; +--disconnect con1 + +--connection slave +STOP SLAVE 'slave2'; +set default_master_connection = 'slave2'; +--source include/wait_for_slave_to_stop.inc + +DROP USER 'unpriv'@'127.0.0.1'; +SET SESSION sql_log_bin=1; + # # clean up # diff --git a/sql/rpl_mi.cc b/sql/rpl_mi.cc index b7b018b8f63..f101e1fcf04 100644 --- a/sql/rpl_mi.cc +++ b/sql/rpl_mi.cc @@ -21,6 +21,7 @@ #include "slave.h" #include "strfunc.h" #include "sql_repl.h" +#include "sql_acl.h" #ifdef HAVE_REPLICATION @@ -1641,6 +1642,9 @@ bool Master_info_index::start_all_slaves(THD *thd) DBUG_ENTER("start_all_slaves"); mysql_mutex_assert_owner(&LOCK_active_mi); + if (check_global_access(thd, PRIV_STMT_START_SLAVE)) + DBUG_RETURN(-1); + for (uint i= 0; i< master_info_hash.records; i++) { Master_info *mi; @@ -1719,6 +1723,9 @@ bool Master_info_index::stop_all_slaves(THD *thd) mysql_mutex_assert_owner(&LOCK_active_mi); DBUG_ASSERT(thd); + if (check_global_access(thd, PRIV_STMT_STOP_SLAVE)) + DBUG_RETURN(-1); + for (uint i= 0; i< master_info_hash.records; i++) { Master_info *mi; -- 2.39.5

1 0