diff --git a/mysql-test/std_data/mdev39143_corrupt_diff_vector.000001 b/mysql-test/std_data/mdev39143_corrupt_diff_vector.000001 new file mode 100644 index 00000000000..0b5caf042d9 Binary files /dev/null and b/mysql-test/std_data/mdev39143_corrupt_diff_vector.000001 differ diff --git a/mysql-test/suite/rpl/include/mdev39143_inject_mysql_binlog.inc b/mysql-test/suite/rpl/include/mdev39143_inject_mysql_binlog.inc new file mode 100644 index 00000000000..19e4dbc7180 --- /dev/null +++ b/mysql-test/suite/rpl/include/mdev39143_inject_mysql_binlog.inc @@ -0,0 +1,18 @@ +# MDEV-39143 test helper: substitute a MySQL-recorded binlog fixture for the +# MariaDB master's own master-bin.000001, so the slave can be tested against +# real MySQL-generated row events without a MySQL server being involved in +# the test itself. The server must be down while the file is swapped, and +# is restarted so it reopens the new master-bin.000001 and starts a fresh +# master-bin.000002 of its own. +# +# Caller must --let $mdev39143_binlog_fixture= +# before sourcing this file; it is unset again afterwards. + +--connection master +--let $datadir= `SELECT @@datadir` +--let $rpl_server_number= 1 +--source include/rpl_stop_server.inc +--remove_file $datadir/master-bin.000001 +--copy_file $mdev39143_binlog_fixture $datadir/master-bin.000001 +--source include/rpl_start_server.inc +--let $mdev39143_binlog_fixture= diff --git a/mysql-test/suite/rpl/include/mdev39143_resync_after_mysql_binlog.inc b/mysql-test/suite/rpl/include/mdev39143_resync_after_mysql_binlog.inc new file mode 100644 index 00000000000..5be7e593e18 --- /dev/null +++ b/mysql-test/suite/rpl/include/mdev39143_resync_after_mysql_binlog.inc @@ -0,0 +1,24 @@ +# MDEV-39143 test helper: having fully applied (or halted on) the injected +# MySQL binlog, drop the test database and repoint the slave at +# master-bin.000002 - the empty binlog the master opened when +# mdev39143_inject_mysql_binlog.inc restarted it - so that master and slave +# agree on a position and rpl_end.inc can shut down cleanly. The injected +# binlog's end has no counterpart on the master, so save_master_pos / +# sync_with_master cannot line the two up against it directly. +# +# Caller must already have stopped whichever of the slave's threads are +# still running (that part genuinely differs per test - e.g. a clean +# STOP SLAVE vs. a STOP SLAVE IO_THREAD after the SQL thread halted on an +# expected error - so it is left to the caller rather than folded in here), +# must already be on the slave connection, and must +# --let $mdev39143_cleanup_db= before sourcing this file; +# it is unset again afterwards. + +eval DROP DATABASE $mdev39143_cleanup_db; +CHANGE MASTER TO Master_log_file='master-bin.000002', Master_log_pos=4, Master_use_gtid=No; +START SLAVE; +--connection master +--save_master_pos +--connection slave +--sync_with_master +--let $mdev39143_cleanup_db= diff --git a/mysql-test/suite/rpl/r/rpl_from_mysql80.result b/mysql-test/suite/rpl/r/rpl_from_mysql80.result index 503b0122018..5882f9a8688 100644 --- a/mysql-test/suite/rpl/r/rpl_from_mysql80.result +++ b/mysql-test/suite/rpl/r/rpl_from_mysql80.result @@ -1,7 +1,6 @@ include/master-slave.inc [connection master] connection slave; -INSTALL SONAME 'type_mysql_json'; include/stop_slave.inc connection master; include/rpl_stop_server.inc [server_number=1] @@ -44,7 +43,6 @@ START SLAVE SQL_THREAD; include/wait_for_slave_io_to_start.inc include/wait_for_slave_sql_to_start.inc DROP TABLE t1, t2; -UNINSTALL SONAME 'type_mysql_json'; CALL mtr.add_suppression('TRANSACTION_PAYLOAD_EVENT event. You can avoid this event by specifying'); connection master; include/rpl_end.inc diff --git a/mysql-test/suite/rpl/r/rpl_json_conv_gating.result b/mysql-test/suite/rpl/r/rpl_json_conv_gating.result new file mode 100644 index 00000000000..90ff04d3a21 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_json_conv_gating.result @@ -0,0 +1,33 @@ +include/master-slave.inc +[connection master] +connection slave; +include/stop_slave.inc +connection master; +include/rpl_stop_server.inc [server_number=1] +include/rpl_start_server.inc [server_number=1] +connection slave; +CREATE DATABASE testdb; +CREATE TABLE testdb.tj (id INT PRIMARY KEY, j VARCHAR(10)); +SET GLOBAL slave_type_conversions= 'ALL_NON_LOSSY'; +CHANGE MASTER TO Master_log_file='master-bin.000001', Master_log_pos=799, Master_use_gtid=No; +call mtr.add_suppression("Column .* of table .* cannot be converted"); +call mtr.add_suppression("Slave SQL.*Error executing row event"); +START SLAVE; +include/wait_for_slave_sql_error.inc [errno=1677] +# Slave stopped, Last_SQL_Error matches the expected refusal: +# Column 1 of table 'testdb.tj' cannot be converted from type 'MYSQL_JSON' to type 'varchar(40 octets) character set utf8mb4' +# +# Nothing was applied - the table is still empty, not holding a +# silently truncated value. +SELECT * FROM testdb.tj; +id j +connection slave; +STOP SLAVE IO_THREAD; +include/wait_for_slave_io_to_stop.inc +SET GLOBAL slave_type_conversions= DEFAULT; +DROP DATABASE testdb; +CHANGE MASTER TO Master_log_file='master-bin.000002', Master_log_pos=4, Master_use_gtid=No; +START SLAVE; +connection master; +connection slave; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/r/rpl_json_corrupt_diff_vector.result b/mysql-test/suite/rpl/r/rpl_json_corrupt_diff_vector.result new file mode 100644 index 00000000000..9ae786f742f --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_json_corrupt_diff_vector.result @@ -0,0 +1,30 @@ +include/master-slave.inc +[connection master] +connection slave; +include/stop_slave.inc +connection master; +include/rpl_stop_server.inc [server_number=1] +include/rpl_start_server.inc [server_number=1] +connection slave; +CHANGE MASTER TO Master_log_file='master-bin.000001', Master_log_pos=4, Master_use_gtid=No; +call mtr.add_suppression("the diff vector is truncated or corrupt"); +call mtr.add_suppression("Slave SQL.*Error executing row event"); +START SLAVE; +include/wait_for_slave_sql_error.inc [errno=1105] +# Slave stopped, Last_SQL_Error matches the expected refusal: +# Could not execute MySQL Update_rows_partial event on table jsontest.t1; Could not apply a partial JSON update to column 'j' of table 'jsontest.t1': the diff vector is truncated or corrupt, Error_code: 1105; handler error HA_ERR_CORRUPT_EVENT; the event's master log master-bin.000001, end_log_pos 1630 +# +# The server is still alive and responsive - the corrupt vector did +# not crash it. +SELECT 1; +1 +1 +connection slave; +STOP SLAVE IO_THREAD; +include/wait_for_slave_io_to_stop.inc +DROP DATABASE jsontest; +CHANGE MASTER TO Master_log_file='master-bin.000002', Master_log_pos=4, Master_use_gtid=No; +START SLAVE; +connection master; +connection slave; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/r/rpl_json_missing_plugin.result b/mysql-test/suite/rpl/r/rpl_json_missing_plugin.result new file mode 100644 index 00000000000..d44f4a36eda --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_json_missing_plugin.result @@ -0,0 +1,29 @@ +include/master-slave.inc +[connection master] +connection slave; +include/stop_slave.inc +connection master; +include/rpl_stop_server.inc [server_number=1] +include/rpl_start_server.inc [server_number=1] +connection slave; +CHANGE MASTER TO Master_log_file='master-bin.000001', Master_log_pos=4, Master_use_gtid=No; +call mtr.add_suppression("plugin needed to decode it is not installed"); +call mtr.add_suppression("Slave SQL.*Error executing row event"); +START SLAVE; +include/wait_for_slave_sql_error.inc [errno=4254] +# Slave stopped, Last_SQL_Error names the missing plugin: +# Table structure for binlog event is not compatible with the table definition on this slave: In RBR mode, Slave received a MySQL binary JSON column (type 245) for column Name: testdb.tj.j, but the MYSQL_JSON plugin needed to decode it is not installed. Run INSTALL SONAME 'type_mysql_json' on the slave. +# +# The table exists (DDL applied) but holds no rows - the row event +# that needed the plugin never got that far. +SELECT * FROM testdb.tj; +id j +connection slave; +STOP SLAVE IO_THREAD; +include/wait_for_slave_io_to_stop.inc +DROP DATABASE testdb; +CHANGE MASTER TO Master_log_file='master-bin.000002', Master_log_pos=4, Master_use_gtid=No; +START SLAVE; +connection master; +connection slave; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/r/rpl_json_partial_insert.result b/mysql-test/suite/rpl/r/rpl_json_partial_insert.result index 063892c65d4..b966ce53af6 100644 --- a/mysql-test/suite/rpl/r/rpl_json_partial_insert.result +++ b/mysql-test/suite/rpl/r/rpl_json_partial_insert.result @@ -1,19 +1,18 @@ include/master-slave.inc [connection master] connection slave; -INSTALL SONAME 'type_mysql_json'; include/stop_slave.inc connection master; include/rpl_stop_server.inc [server_number=1] include/rpl_start_server.inc [server_number=1] connection slave; CHANGE MASTER TO Master_log_file='master-bin.000001', Master_log_pos=4, Master_use_gtid=No; -call mtr.add_suppression("MDEV-39143: cannot apply a partial JSON update"); +call mtr.add_suppression("Could not apply a partial JSON update"); call mtr.add_suppression("Slave SQL.*Error executing row event"); START SLAVE; include/wait_for_slave_sql_error.inc [errno=1105] # Slave stopped, Last_SQL_Error matches the expected refusal: -# Could not execute MySQL Update_rows_partial event on table jsontest.t1; MDEV-39143: cannot apply a partial JSON update that inserts at a from-the-end array index: path '$.arr[last-9]' on jsontest.t1.j. MariaDB's JSON_ARRAY_INSERT resolves '[last-N]' differently from MySQL, so the result would silently differ from the source., Error_code: 1105; handler error HA_ERR_GENERIC; the event's master log master-bin.000001, end_log_pos 2453 +# Could not execute MySQL Update_rows_partial event on table jsontest.t1; Could not apply a partial JSON update to column 'j' of table 'jsontest.t1' at path '$.arr[last-9]': cannot apply an INSERT at a from-the-end array index - MariaDB's JSON_ARRAY_INSERT resolves '[last-N]' differently from MySQL, so the result would silently differ from the source, Error_code: 1105; handler error HA_ERR_GENERIC; the event's master log master-bin.000001, end_log_pos 2453 # # Rows applied before the halt: id=1 got its object-member INSERT and # id=2 its array append. id=3 and id=4 keep the inserted values. @@ -32,5 +31,4 @@ CHANGE MASTER TO Master_log_file='master-bin.000002', Master_log_pos=4, Master_u START SLAVE; connection master; connection slave; -UNINSTALL SONAME 'type_mysql_json'; include/rpl_end.inc diff --git a/mysql-test/suite/rpl/r/rpl_json_partial_update.result b/mysql-test/suite/rpl/r/rpl_json_partial_update.result index 5e57609631d..8050e7a9d6d 100644 --- a/mysql-test/suite/rpl/r/rpl_json_partial_update.result +++ b/mysql-test/suite/rpl/r/rpl_json_partial_update.result @@ -1,7 +1,6 @@ include/master-slave.inc [connection master] connection slave; -INSTALL SONAME 'type_mysql_json'; include/stop_slave.inc connection master; include/rpl_stop_server.inc [server_number=1] @@ -23,5 +22,4 @@ CHANGE MASTER TO Master_log_file='master-bin.000002', Master_log_pos=4, Master_u START SLAVE; connection master; connection slave; -UNINSTALL SONAME 'type_mysql_json'; include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_from_mysql80.test b/mysql-test/suite/rpl/t/rpl_from_mysql80.test index fb2ed02d782..9ce68ec2827 100644 --- a/mysql-test/suite/rpl/t/rpl_from_mysql80.test +++ b/mysql-test/suite/rpl/t/rpl_from_mysql80.test @@ -1,5 +1,6 @@ --source include/have_innodb.inc --source include/have_binlog_format_row.inc +--source include/have_type_mysql_json.inc --source include/master-slave.inc # Test some replication events from MySQL 8.0 to MariaDB. @@ -59,19 +60,11 @@ --connection slave -INSTALL SONAME 'type_mysql_json'; --source include/stop_slave.inc ---connection master ---let $datadir= `SELECT @@datadir` ---let $rpl_server_number= 1 ---source include/rpl_stop_server.inc - # Copy in the MySQL 8.0 generated binlog. ---remove_file $datadir/master-bin.000001 ---copy_file $MYSQL_TEST_DIR/std_data/mdev35643_mysql_80_binlog.000001 $datadir/master-bin.000001 - ---source include/rpl_start_server.inc +--let $mdev39143_binlog_fixture= $MYSQL_TEST_DIR/std_data/mdev35643_mysql_80_binlog.000001 +--source suite/rpl/include/mdev39143_inject_mysql_binlog.inc --save_master_pos @@ -129,7 +122,6 @@ START SLAVE SQL_THREAD; --sync_with_master DROP TABLE t1, t2; -UNINSTALL SONAME 'type_mysql_json'; CALL mtr.add_suppression('TRANSACTION_PAYLOAD_EVENT event. You can avoid this event by specifying'); --connection master diff --git a/mysql-test/suite/rpl/t/rpl_json_conv_gating.test b/mysql-test/suite/rpl/t/rpl_json_conv_gating.test new file mode 100644 index 00000000000..e11b76bcd32 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_json_conv_gating.test @@ -0,0 +1,62 @@ +# MDEV-39143: a MySQL binary JSON source replicating into a plain +# VARCHAR/TEXT target (not native JSON) must be classified by the target's +# actual declared capacity, not assumed to always fit - see +# Field_longstr::rpl_conv_type_from()'s mysql_json branch in +# sql/rpl_utility_server.cc. Before that fix, this exact scenario was +# hardcoded as CONV_TYPE_SUBSET_TO_SUPERSET regardless of size, so +# slave_type_conversions=ALL_NON_LOSSY alone was wrongly enough to let a +# too-small target silently truncate the document. With the fix, a +# document that does not fit is CONV_TYPE_SUPERSET_TO_SUBSET, which needs +# ALL_LOSSY - so with only ALL_NON_LOSSY set, replication must refuse the +# table definition rather than apply anything. +# +# Reuses the mdev39143_partial_json.000001 fixture also used by +# rpl_json_partial_update.test (see its header for how the injection +# works), but starts replication at position 799 - right after that +# fixture's own "CREATE TABLE tj (id INT PRIMARY KEY, j JSON)" - so this +# test's own, deliberately too-small VARCHAR(10) definition is what the +# slave actually has, instead of the fixture's JSON column. + +--source include/have_innodb.inc +--source include/have_binlog_format_row.inc +--source include/have_type_mysql_json.inc +--source include/master-slave.inc + +--connection slave +--source include/stop_slave.inc + +--let $mdev39143_binlog_fixture= $MYSQL_TEST_DIR/std_data/mdev39143_partial_json.000001 +--source suite/rpl/include/mdev39143_inject_mysql_binlog.inc + +--connection slave +CREATE DATABASE testdb; +CREATE TABLE testdb.tj (id INT PRIMARY KEY, j VARCHAR(10)); +SET GLOBAL slave_type_conversions= 'ALL_NON_LOSSY'; +# Position 799 is the start of the BEGIN preceding the full-document INSERT +# into tj - i.e. right after the fixture's own CREATE TABLE tj completes. +CHANGE MASTER TO Master_log_file='master-bin.000001', Master_log_pos=799, Master_use_gtid=No; +call mtr.add_suppression("Column .* of table .* cannot be converted"); +call mtr.add_suppression("Slave SQL.*Error executing row event"); +START SLAVE; + +# The slave must refuse the table definition rather than truncate the +# document into the too-small VARCHAR(10). +--let $slave_sql_errno= 1677 +--source include/wait_for_slave_sql_error.inc +--let $err= query_get_value(SHOW SLAVE STATUS, Last_SQL_Error, 1) +--echo # Slave stopped, Last_SQL_Error matches the expected refusal: +--echo # $err + +--echo # +--echo # Nothing was applied - the table is still empty, not holding a +--echo # silently truncated value. +SELECT * FROM testdb.tj; + +# Cleanup. +--connection slave +STOP SLAVE IO_THREAD; +--source include/wait_for_slave_io_to_stop.inc +SET GLOBAL slave_type_conversions= DEFAULT; +--let $mdev39143_cleanup_db= testdb +--source suite/rpl/include/mdev39143_resync_after_mysql_binlog.inc +--source include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_json_corrupt_diff_vector.test b/mysql-test/suite/rpl/t/rpl_json_corrupt_diff_vector.test new file mode 100644 index 00000000000..840df6aef32 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_json_corrupt_diff_vector.test @@ -0,0 +1,57 @@ +# MDEV-39143: a structurally corrupt Json_diff vector in a +# PARTIAL_UPDATE_ROWS_EVENT must stop replication with a clear error, not +# crash the slave - see read_mysql_json_diff_vector()'s NULL-cursor checks +# and apply_partial_json_column()'s corrupt-vector report in +# sql/rpl_record.cc. +# +# The fixture, std_data/mdev39143_corrupt_diff_vector.000001, is a byte-level +# corrupted copy of mdev39143_partial_json_insert.000001 (also used by +# rpl_json_partial_insert.test): the first PARTIAL_UPDATE_ROWS_EVENT's one +# diff - an INSERT of '$.newkey' on id=1 - has its path_length byte (a +# single-byte net_field_length encoding, originally 0x08) overwritten with +# 0xFE, the net_field_length prefix for an 8-byte length value, and that +# event's trailing CRC32 recomputed so the corruption is only in the +# content read_mysql_json_diff_vector() parses, not something the +# replication protocol itself would catch first. safe_net_field_length_ll() +# (sql-common/pack.c) then reads 8 bytes of what is actually path text as a +# bogus 64-bit length, which read_mysql_json_diff_vector()'s bounds check +# must reject as corrupt - this exercises the same "trust nothing +# safe_net_field_length_ll() hands back" defense that also protects against +# it returning a NULL cursor on a plain truncation. + +--source include/have_innodb.inc +--source include/have_binlog_format_row.inc +--source include/have_type_mysql_json.inc +--source include/master-slave.inc + +--connection slave +--source include/stop_slave.inc + +--let $mdev39143_binlog_fixture= $MYSQL_TEST_DIR/std_data/mdev39143_corrupt_diff_vector.000001 +--source suite/rpl/include/mdev39143_inject_mysql_binlog.inc + +--connection slave +CHANGE MASTER TO Master_log_file='master-bin.000001', Master_log_pos=4, Master_use_gtid=No; +call mtr.add_suppression("the diff vector is truncated or corrupt"); +call mtr.add_suppression("Slave SQL.*Error executing row event"); +START SLAVE; + +# The slave must refuse the corrupt vector rather than crash. +--let $slave_sql_errno= 1105 +--source include/wait_for_slave_sql_error.inc +--let $err= query_get_value(SHOW SLAVE STATUS, Last_SQL_Error, 1) +--echo # Slave stopped, Last_SQL_Error matches the expected refusal: +--echo # $err + +--echo # +--echo # The server is still alive and responsive - the corrupt vector did +--echo # not crash it. +SELECT 1; + +# Cleanup. +--connection slave +STOP SLAVE IO_THREAD; +--source include/wait_for_slave_io_to_stop.inc +--let $mdev39143_cleanup_db= jsontest +--source suite/rpl/include/mdev39143_resync_after_mysql_binlog.inc +--source include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_json_missing_plugin.test b/mysql-test/suite/rpl/t/rpl_json_missing_plugin.test new file mode 100644 index 00000000000..48b75f215ec --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_json_missing_plugin.test @@ -0,0 +1,62 @@ +# MDEV-39143: a MySQL binary JSON column replicated to a slave that does not +# have the type_mysql_json plugin loaded must halt with a clear, actionable +# error naming the plugin - see check_wrong_column_usage()'s after-image fix +# and give_compatibility_error()'s SLAVE_FIELD_UNKNOWN_TYPE case in +# sql/rpl_utility_server.cc - rather than silently skip the column or fail +# with a generic "unknown field type" message that gives the DBA nothing to +# act on. +# +# Deliberately does NOT load the plugin on the slave, and reuses the +# mdev39143_partial_json.000001 fixture also used by +# rpl_json_partial_update.test (see its header for how the injection +# works), replicated from the very start this time: the CREATE +# DATABASE/CREATE TABLE DDL needs no plugin and must succeed, so only the +# first row event (the full-document INSERT) should fail. +# +# This cannot --source include/have_type_mysql_json.inc like the other +# MDEV-39143 rpl tests: that file has a companion have_type_mysql_json.opt +# that mtr auto-applies to any test sourcing it (see mtr_cases.pm's +# per-included-file .opt lookup), which loads the plugin at server startup - +# exactly what this test must NOT do. The check below is that file's skip +# condition, inlined. +if (!$TYPE_MYSQL_JSON_SO) { + skip Need MYSQL_JSON plugin; +} + +--source include/have_innodb.inc +--source include/have_binlog_format_row.inc +--source include/master-slave.inc + +--connection slave +--source include/stop_slave.inc + +--let $mdev39143_binlog_fixture= $MYSQL_TEST_DIR/std_data/mdev39143_partial_json.000001 +--source suite/rpl/include/mdev39143_inject_mysql_binlog.inc + +--connection slave +CHANGE MASTER TO Master_log_file='master-bin.000001', Master_log_pos=4, Master_use_gtid=No; +call mtr.add_suppression("plugin needed to decode it is not installed"); +call mtr.add_suppression("Slave SQL.*Error executing row event"); +START SLAVE; + +# The slave must refuse the row event naming the missing plugin, after the +# preceding CREATE DATABASE/CREATE TABLE DDL (which needs no plugin) applies +# cleanly. +--let $slave_sql_errno= 4254 +--source include/wait_for_slave_sql_error.inc +--let $err= query_get_value(SHOW SLAVE STATUS, Last_SQL_Error, 1) +--echo # Slave stopped, Last_SQL_Error names the missing plugin: +--echo # $err + +--echo # +--echo # The table exists (DDL applied) but holds no rows - the row event +--echo # that needed the plugin never got that far. +SELECT * FROM testdb.tj; + +# Cleanup. +--connection slave +STOP SLAVE IO_THREAD; +--source include/wait_for_slave_io_to_stop.inc +--let $mdev39143_cleanup_db= testdb +--source suite/rpl/include/mdev39143_resync_after_mysql_binlog.inc +--source include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_json_partial_insert.test b/mysql-test/suite/rpl/t/rpl_json_partial_insert.test index 86f37344104..e312f1ebbae 100644 --- a/mysql-test/suite/rpl/t/rpl_json_partial_insert.test +++ b/mysql-test/suite/rpl/t/rpl_json_partial_insert.test @@ -14,10 +14,12 @@ # MySQL logs the first as an INSERT diff on an object-member path and the # next two as INSERT diffs on array-cell paths; its own mysqlbinlog renders # them as JSON_INSERT, JSON_ARRAY_INSERT and JSON_ARRAY_INSERT respectively. -# The slave must make the same distinction: id=3 is the discriminating case, -# where the path resolves before the start of the array so the value is -# prepended and the existing elements shift right. JSON_INSERT would leave -# the document untouched there, silently. +# The slave makes the same function choice in apply_partial_json_column() +# (see json_diff_path_is_array_cell()). That choice is not what makes id=3 +# special, though - id=2 and id=3 both correctly resolve to JSON_ARRAY_INSERT. +# What's special about id=3 is its path, '$.arr[last-9]': it is the only one +# here that resolves from the end of the array, which is exactly where +# MariaDB's JSON_ARRAY_INSERT disagrees with MySQL (see below). # # MySQL's own contents after those updates, read from the source table: # id=1 arr [1, 2, 3] newkey "added" @@ -44,23 +46,22 @@ --source include/have_innodb.inc --source include/have_binlog_format_row.inc +--source include/have_type_mysql_json.inc --source include/master-slave.inc +# The slave decodes MySQL binary JSON with the read-only mysql_json plugin, +# loaded on the slave only via rpl_json_partial_insert-slave.opt (it never +# needs to be active on the master, which only ever serves the injected +# fixture's raw bytes). --connection slave -INSTALL SONAME 'type_mysql_json'; --source include/stop_slave.inc ---connection master ---let $datadir= `SELECT @@datadir` ---let $rpl_server_number= 1 ---source include/rpl_stop_server.inc ---remove_file $datadir/master-bin.000001 ---copy_file $MYSQL_TEST_DIR/std_data/mdev39143_partial_json_insert.000001 $datadir/master-bin.000001 ---source include/rpl_start_server.inc +--let $mdev39143_binlog_fixture= $MYSQL_TEST_DIR/std_data/mdev39143_partial_json_insert.000001 +--source suite/rpl/include/mdev39143_inject_mysql_binlog.inc --connection slave CHANGE MASTER TO Master_log_file='master-bin.000001', Master_log_pos=4, Master_use_gtid=No; -call mtr.add_suppression("MDEV-39143: cannot apply a partial JSON update"); +call mtr.add_suppression("Could not apply a partial JSON update"); call mtr.add_suppression("Slave SQL.*Error executing row event"); START SLAVE; @@ -84,12 +85,6 @@ SELECT id, JSON_QUERY(j,'$.arr') AS arr, JSON_VALUE(j,'$.newkey') AS newkey --connection slave STOP SLAVE IO_THREAD; --source include/wait_for_slave_io_to_stop.inc -DROP DATABASE jsontest; -CHANGE MASTER TO Master_log_file='master-bin.000002', Master_log_pos=4, Master_use_gtid=No; -START SLAVE; ---connection master ---save_master_pos ---connection slave ---sync_with_master -UNINSTALL SONAME 'type_mysql_json'; +--let $mdev39143_cleanup_db= jsontest +--source suite/rpl/include/mdev39143_resync_after_mysql_binlog.inc --source include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_json_partial_update.test b/mysql-test/suite/rpl/t/rpl_json_partial_update.test index 9ea9394c5e3..eaca468c6f0 100644 --- a/mysql-test/suite/rpl/t/rpl_json_partial_update.test +++ b/mysql-test/suite/rpl/t/rpl_json_partial_update.test @@ -20,24 +20,18 @@ # format; the injected events are row events regardless of it. --source include/have_innodb.inc --source include/have_binlog_format_row.inc +--source include/have_type_mysql_json.inc --source include/master-slave.inc # The slave decodes MySQL binary JSON with the read-only mysql_json plugin, -# which is not installed by default. Uninstalled again in the teardown. +# loaded on the slave only via rpl_json_partial_update-slave.opt (it never +# needs to be active on the master, which only ever serves the injected +# fixture's raw bytes). --connection slave -INSTALL SONAME 'type_mysql_json'; --source include/stop_slave.inc -# Substitute the recorded MySQL binlog for the master's own. The server must -# be down while the file is swapped, and is restarted so it reopens the new -# master-bin.000001 and starts a fresh master-bin.000002 of its own. ---connection master ---let $datadir= `SELECT @@datadir` ---let $rpl_server_number= 1 ---source include/rpl_stop_server.inc ---remove_file $datadir/master-bin.000001 ---copy_file $MYSQL_TEST_DIR/std_data/mdev39143_partial_json.000001 $datadir/master-bin.000001 ---source include/rpl_start_server.inc +--let $mdev39143_binlog_fixture= $MYSQL_TEST_DIR/std_data/mdev39143_partial_json.000001 +--source suite/rpl/include/mdev39143_inject_mysql_binlog.inc --connection slave CHANGE MASTER TO Master_log_file='master-bin.000001', Master_log_pos=4, Master_use_gtid=No; @@ -54,22 +48,9 @@ SELECT id, JSON_VALUE(j,'$.a') AS a, JSON_VALUE(j,'$.n') AS n, JSON_QUERY(j,'$.arr') AS arr FROM testdb.tj ORDER BY id; # Cleanup. -# The slave is sitting at the end of the injected binlog, which the -# master did not write and knows nothing about, so save_master_pos/ -# sync_with_master cannot line the two up. Repoint the slave at -# master-bin.000002 - the empty binlog the master opened on restart - so that -# the two agree on a position and rpl_end.inc can shut down cleanly. --connection slave STOP SLAVE; --source include/wait_for_slave_to_stop.inc -DROP DATABASE testdb; -CHANGE MASTER TO Master_log_file='master-bin.000002', Master_log_pos=4, Master_use_gtid=No; -START SLAVE; ---connection master ---save_master_pos ---connection slave ---sync_with_master -# Remove the plugin again: it is installed into mysql.plugin, which MTR's -# check-testcase compares against the pre-test server state. -UNINSTALL SONAME 'type_mysql_json'; +--let $mdev39143_cleanup_db= testdb +--source suite/rpl/include/mdev39143_resync_after_mysql_binlog.inc --source include/rpl_end.inc diff --git a/sql/log_event.h b/sql/log_event.h index 1680a69dda7..9515363380b 100644 --- a/sql/log_event.h +++ b/sql/log_event.h @@ -5101,14 +5101,25 @@ protected: // Unpack the current row into m_table->record[0], but with // a different columns bitmap. - int unpack_current_row(rpl_group_info *rgi, MY_BITMAP const *cols) + // + // only_seek is true when there is no matched row to unpack into - the + // caller is only advancing past this row image to skip it (e.g. + // Update_rows_log_event::do_exec_row() after find_row() failed), not + // applying it. Required rather than defaulted: this overload is only + // ever called from that one place, specifically to distinguish its two + // call sites (the real apply, and the find_row()-failure skip), so an + // implicit default would silently reintroduce the ambiguity this + // parameter exists to remove. + int unpack_current_row(rpl_group_info *rgi, MY_BITMAP const *cols, + bool only_seek) { DBUG_ASSERT(m_table); ASSERT_OR_RETURN_ERROR(m_curr_row <= m_rows_end, HA_ERR_CORRUPT_EVENT); return ::unpack_row(rgi, m_table, m_width, m_curr_row, cols, &m_curr_row_end, m_rows_end, - is_partial_json_update() && cols == &m_cols_ai); + is_partial_json_update() && cols == &m_cols_ai, + only_seek); } // Unpack the current row into m_table->record[0] diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc index 3c8d7bea473..8cbc4a1ecbd 100644 --- a/sql/log_event_server.cc +++ b/sql/log_event_server.cc @@ -5421,7 +5421,8 @@ int Rows_log_event::do_apply_event(rpl_group_info *rgi) bitmap_set_bit(table->write_set, table->s->vers.end_fieldno); } - if ((error= rpl_table->check_wrong_column_usage(rgi, &m_cols))) + if ((error= rpl_table->check_wrong_column_usage( + rgi, &m_cols, type == UPDATE_ROWS_EVENT ? &m_cols_ai : NULL))) goto err; table->mark_columns_per_binlog_row_image(); @@ -8690,10 +8691,14 @@ Update_rows_log_event::do_exec_row(rpl_group_info *rgi) { /* We need to read the second image in the event of error to be - able to skip to the next pair of updates + able to skip to the next pair of updates. find_row() failed, so + record[1] does not hold this row - only_seek=true tells + unpack_current_row() (and, for a partial JSON column, + apply_partial_json_column()) to advance past the image without + reading record[1] or applying anything to it. */ if ((m_curr_row= m_curr_row_end)) - unpack_current_row(rgi, &m_cols_ai); + unpack_current_row(rgi, &m_cols_ai, true); return error; } @@ -8716,7 +8721,7 @@ Update_rows_log_event::do_exec_row(rpl_group_info *rgi) m_curr_row= m_curr_row_end; /* this also updates m_curr_row_end */ thd_proc_info(thd, "Update_rows_log_event::unpack_current_row()"); - if (unlikely((error= unpack_current_row(rgi, &m_cols_ai)))) + if (unlikely((error= unpack_current_row(rgi, &m_cols_ai, false)))) goto err; if (m_table->s->long_unique_table) m_table->update_virtual_fields(m_table->file, VCOL_UPDATE_FOR_WRITE); diff --git a/sql/rpl_record.cc b/sql/rpl_record.cc index 85ab4896511..c771ee3f86f 100644 --- a/sql/rpl_record.cc +++ b/sql/rpl_record.cc @@ -243,8 +243,20 @@ struct Unpack_record_state That is, the value should be initially unpacked into a conversion table field, so here we copy and convert the value from the conversion table field to the result field. + + @retval false Success. + @retval true The conversion failed - currently only possible for a MySQL + JSON source, where the conversion field's val_str() + (Field_mysql_json::val_str(), called via Copy_field's + do_save_blob()) decodes corrupt binary JSON: it pushes an + error to the THD but still returns an (empty) string, so + do_copy() "succeeds" and would otherwise store that empty + value with no indication anything was wrong. An error + already on the THD before this call (unrelated to this + conversion) would also be reported here, but no caller + reaches this function with one pending. */ -static void convert_field(Field *result_field, Field *conv_field) +static bool convert_field(Field *result_field, Field *conv_field) { DBUG_ASSERT(result_field); DBUG_ASSERT(conv_field); @@ -265,6 +277,9 @@ static void convert_field(Field *result_field, Field *conv_field) copy.set(result_field, conv_field, TRUE); (*copy.do_copy)(©); + if (conv_field->table->in_use->is_error()) + return true; + #ifndef DBUG_OFF String target_type(type_buf, sizeof(type_buf), system_charset_info); result_field->sql_type(target_type); @@ -274,6 +289,7 @@ static void convert_field(Field *result_field, Field *conv_field) result_field->field_name.str, target_type.c_ptr_safe(), value_string.c_ptr_safe())); #endif + return false; } @@ -369,7 +385,16 @@ static bool read_mysql_json_diff_vector(const uchar **row_image_pos, size_t avail= (size_t) (diffs_end - ptr); diff.path_length= (size_t) safe_net_field_length_ll((uchar**) &ptr, avail); - if ((size_t) (diffs_end - ptr) < diff.path_length) + /* + safe_net_field_length_ll() (sql-common/pack.c) sets its cursor to NULL + on a truncated encoding, rather than leaving it unmoved. Checking the + bounds below without first checking for that would compute + (diffs_end - NULL), a huge bogus value that passes any length check + and leaves ptr poisoned at NULL for every subsequent read in this + vector - including the unconditional `*ptr++` at the top of the next + loop iteration, which would then crash. + */ + if (!ptr || (size_t) (diffs_end - ptr) < diff.path_length) return true; diff.path= (const char*) ptr; ptr+= diff.path_length; @@ -384,7 +409,7 @@ static bool read_mysql_json_diff_vector(const uchar **row_image_pos, avail= (size_t) (diffs_end - ptr); diff.value_length= (size_t) safe_net_field_length_ll((uchar**) &ptr, avail); - if ((size_t) (diffs_end - ptr) < diff.value_length) + if (!ptr || (size_t) (diffs_end - ptr) < diff.value_length) return true; diff.value= ptr; ptr+= diff.value_length; @@ -446,6 +471,114 @@ static bool json_diff_path_is_array_cell(json_path_t *path_obj, } +/** + Check whether a path resolves to an existing value in a JSON document. + + MySQL's own slave applier (sql/item_json_func.cc, apply_json_diff()) + rejects a diff - and MariaDB's apply_json_diffs() (sql/field.cc) stops + replication with ER_COULD_NOT_APPLY_JSON_DIFF for it - whenever the + document does not match what the diff expects: REPLACE and REMOVE + require the target path to already exist; an INSERT on an object member + requires it not to (an INSERT on an array cell has no such requirement - + MySQL only emits one for a cell that does not exist, by construction). + This uses JSON_CONTAINS_PATH(doc, 'one', path) to answer that question, + reusing the same Item-based approach the rest of this file already uses. + + @param thd Used to build and evaluate the Item. + @param doc The current document, as text. + @param path The diff's path text. + @param path_length Its length in bytes. + @param[out] exists Set on success: whether the path resolves. + + @retval false Success; *exists is set. + @retval true The check itself failed (OOM, or the path/document could + not be evaluated) - distinct from the path simply not + existing, which is a normal, successful outcome. +*/ +static bool json_diff_path_exists(THD *thd, const String &doc, + const char *path, size_t path_length, + bool *exists) +{ + List args; + Item *doc_it= new (thd->mem_root) + Item_string(thd, doc.ptr(), (uint) doc.length(), doc.charset()); + Item *mode_it= new (thd->mem_root) + Item_string(thd, STRING_WITH_LEN("one"), &my_charset_utf8mb4_bin); + Item *path_it= new (thd->mem_root) + Item_string(thd, path, (uint) path_length, &my_charset_utf8mb4_bin); + if (!doc_it || !mode_it || !path_it || + args.push_back(doc_it, thd->mem_root) || + args.push_back(mode_it, thd->mem_root) || + args.push_back(path_it, thd->mem_root)) + return true; + + Item *contains= new (thd->mem_root) Item_func_json_contains_path(thd, args); + if (!contains || contains->fix_fields(thd, &contains)) + return true; + + longlong found= contains->val_int(); + if (contains->null_value) + return true; + *exists= found != 0; + return false; +} + + +/** + Report that a partial JSON update could not be applied to a column, and + stop replication. Used both for a structurally-corrupt diff vector (no + path available yet) and for a well-formed diff that is REJECTED - the + slave's document does not match what the diff expects, which MySQL + itself treats as a hard error (ER_COULD_NOT_APPLY_JSON_DIFF), not as a + silent no-op - or that otherwise fails to apply. + + This pushes onto the THD's diagnostics area via my_printf_error(), not + rgi->rli->report(): do_exec_row()'s caller always re-derives Last_SQL_Error + from thd->is_error() once the row event returns an error (see + convert_handler_error() and slave_rows_error_report() in + log_event_server.cc), and unconditionally overwrites whatever + rgi->rli->report() had already recorded. Only a message already sitting in + the diagnostics area when do_exec_row() returns survives that overwrite. + + @param table The slave table, for the error message. + @param result_field The column being updated, for the error message. + @param path The diff's path, or NULL if not applicable (e.g. a + corrupt diff vector, or no base document at all). + @param path_length Its length in bytes; ignored if path is NULL. + @param reason Human-readable reason, appended to the message. +*/ +static void report_pj_apply_error(TABLE *table, Field *result_field, + const char *path, size_t path_length, + const char *reason) +{ + /* + Some callers report after a failed sub-operation (val_str() on corrupt + JSONB, a json_diff_path_exists() evaluation failure, fix_fields()) that + has already pushed its own error onto the diagnostics area. Pushing a + second one would hit set_error_status()'s "no previous error" assertion + (sql_error.cc), so leave the existing, already-specific message alone. + */ + THD *thd= table->in_use; + if (thd->is_error()) + return; + char msg[MYSQL_ERRMSG_SIZE]; + if (path) + my_snprintf(msg, sizeof(msg), + "Could not apply a partial JSON update to column '%s' " + "of table '%s.%s' at path '%.*s': %s", + result_field->field_name.str, table->s->db.str, + table->s->table_name.str, + (int) path_length, path, reason); + else + my_snprintf(msg, sizeof(msg), + "Could not apply a partial JSON update to column '%s' " + "of table '%s.%s': %s", + result_field->field_name.str, table->s->db.str, + table->s->table_name.str, reason); + my_printf_error(ER_UNKNOWN_ERROR, "%s", MYF(0), msg); +} + + /** Apply one partial-JSON column of a MySQL PARTIAL_UPDATE_ROWS_EVENT after-image. @@ -496,30 +629,47 @@ static bool json_diff_path_is_array_cell(json_path_t *path_obj, corrupt. @param row_end One past the last readable byte of the row image, bounding the diff decoding. + @param only_seek + True when there is no matched row to apply to - the + caller (Update_rows_log_event::do_exec_row()) is only + unpacking the after-image to skip past it in the event + stream after find_row() failed, and record[1] does not + hold this row's data. The diff vector is still decoded, + to advance state.pack_ptr correctly, but is then discarded + without being applied or touching result_field. - @retval 0 Success; result_field holds the new document. + @retval 0 Success; result_field holds the new document + (unless only_seek). @retval HA_ERR_CORRUPT_EVENT The diff vector is truncated or malformed. - @retval HA_ERR_GENERIC The before-image was NULL, or building or - evaluating one of the JSON functions failed. - An error has been raised on the THD. - - @note Known gaps, tracked in the MDEV-39143 design document: - - MySQL distinguishes a REJECTED diff (the path does not match, so - the value is left alone and replication continues) from a real - error. Here every failure stops the SQL thread. - - The NULL before-image case raises a placeholder ER_UNKNOWN_ERROR. + Reported via report_pj_apply_error(). + @retval HA_ERR_GENERIC There was no base document to apply to, a + diff was REJECTED (its path does not match + the slave's document - the same condition + MySQL's own applier treats as a hard error, + not a silent no-op, per + sql/item_json_func.cc apply_json_diffs() and + sql/field.cc Field_json::unpack_diff()), or + building/evaluating a JSON function failed. + Reported via report_pj_apply_error(). */ static int apply_partial_json_column(const rpl_group_info *rgi, TABLE *table, Field *result_field, Field *conv_field, Unpack_record_state &state, - const uchar *row_end) + const uchar *row_end, bool only_seek) { Dynamic_array diffs((PSI_memory_key) PSI_NOT_INSTRUMENTED); const uchar *ptr= state.pack_ptr; if (read_mysql_json_diff_vector(&ptr, row_end, diffs)) + { + report_pj_apply_error(table, result_field, NULL, 0, + "the diff vector is truncated or corrupt"); return HA_ERR_CORRUPT_EVENT; + } state.pack_ptr= ptr; + if (only_seek) + return 0; + THD *thd= rgi->thd; DBUG_ASSERT(conv_field); @@ -532,23 +682,36 @@ static int apply_partial_json_column(const rpl_group_info *rgi, TABLE *table, result_field->move_field_offset(-bi_offset); if (base_is_null || !base) { - my_printf_error(ER_UNKNOWN_ERROR, - "MDEV-39143: partial JSON update with no base value (slave out of sync)", - MYF(0)); + report_pj_apply_error(table, result_field, NULL, 0, + "the slave's current value is NULL, so there is " + "no document to apply the update to"); return HA_ERR_GENERIC; } String cur; if (cur.copy(*base)) + { + report_pj_apply_error(table, result_field, NULL, 0, + "out of memory"); return HA_ERR_GENERIC; + } /* - Build and evaluate the JSON functions on a private free_list, then release - them (and their internal buffers) here with free_items(), instead of - letting them accumulate until end of statement. + Build and evaluate the JSON functions on a private free_list and a + private MEM_ROOT, then release both here instead of letting them + accumulate until end of statement. free_items() alone is not enough: + it only runs destructors, it does not reclaim the memory the Items and + their buffers were allocated from, so if they were left on + thd->mem_root (the statement-level arena, never shrunk mid-statement) + a multi-row UPDATE touching many JSON rows would grow it for the whole + statement's duration. free_root() below reclaims it per row instead. */ Item *const save_free_list= thd->free_list; thd->free_list= NULL; + MEM_ROOT json_apply_root; + init_alloc_root(PSI_NOT_INSTRUMENTED, &json_apply_root, 1024, 0, MYF(0)); + MEM_ROOT *const save_mem_root= thd->mem_root; + thd->mem_root= &json_apply_root; int rc= 0; /* Reused by every INSERT diff of this column to classify its path. */ @@ -570,7 +733,48 @@ static int apply_partial_json_column(const rpl_group_info *rgi, TABLE *table, if (!doc_it || !path_it || args.push_back(doc_it, thd->mem_root) || args.push_back(path_it, thd->mem_root)) - { rc= HA_ERR_GENERIC; break; } + { + report_pj_apply_error(table, result_field, diff.path, + diff.path_length, "out of memory"); + rc= HA_ERR_GENERIC; break; + } + + /* + A diff is REJECTED - and, per MySQL's own applier + (sql/item_json_func.cc apply_json_diffs(), sql/field.cc + Field_json::unpack_diff()), that is a hard error, not a silent + no-op - when the slave's document does not match what the diff + expects: REPLACE and REMOVE require the target path to already + exist; an INSERT on an object member requires it not to. An INSERT + on an array cell has no such requirement: MySQL only emits one for a + cell that does not exist, by construction. + */ + bool target_exists; + if (diff.op != MYSQL_JSON_DIFF_INSERT) + { + bool exists_check_failed= + json_diff_path_exists(thd, cur, diff.path, diff.path_length, + &target_exists); + if (exists_check_failed) + { + report_pj_apply_error(table, result_field, diff.path, + diff.path_length, + "could not check whether the path exists " + "in the current document"); + rc= HA_ERR_GENERIC; break; + } + if (!target_exists) + { + report_pj_apply_error(table, result_field, diff.path, + diff.path_length, + diff.op == MYSQL_JSON_DIFF_REMOVE ? + "REJECTED - the path to remove does not " + "exist in the slave's document" : + "REJECTED - the path to replace does not " + "exist in the slave's document"); + rc= HA_ERR_GENERIC; break; + } + } Item *func; if (diff.op == MYSQL_JSON_DIFF_REMOVE) @@ -592,8 +796,22 @@ static int apply_partial_json_column(const rpl_group_info *rgi, TABLE *table, ((Field_blob *) conv_field)->set_ptr(diff.value_length, const_cast(diff.value)); String vbuf, *vtext= conv_field->val_str(&vbuf); - if (!vtext) - { rc= HA_ERR_GENERIC; break; } + if (!vtext || thd->is_error()) + { + /* + Field_mysql_json::val_str() (plugin/type_mysql_json/type.cc) never + actually returns NULL: on corrupt JSONB it pushes ER_UNKNOWN_ERROR + to the THD and returns an empty string instead, which would + otherwise sail through JSON_COMPACT() as a seemingly-valid empty + value and get committed - so the THD error state, not the return + value, is what must be checked here. + */ + report_pj_apply_error(table, result_field, diff.path, + diff.path_length, + "the diff's value is corrupt or unsupported " + "MySQL binary JSON"); + rc= HA_ERR_GENERIC; break; + } /* Copy the decoded value onto the statement mem_root (val_str's buffer is transient) and wrap it in JSON_COMPACT, so JSON_INSERT/REPLACE inserts @@ -607,7 +825,11 @@ static int apply_partial_json_column(const rpl_group_info *rgi, TABLE *table, Item_func_json_format(thd, v_it, Item_func_json_format::COMPACT) : NULL; if (!val_json || args.push_back(val_json, thd->mem_root)) - { rc= HA_ERR_GENERIC; break; } + { + report_pj_apply_error(table, result_field, diff.path, + diff.path_length, "out of memory"); + rc= HA_ERR_GENERIC; break; + } if (diff.op == MYSQL_JSON_DIFF_INSERT) { /* @@ -619,13 +841,32 @@ static int apply_partial_json_column(const rpl_group_info *rgi, TABLE *table, diff.path_length, &is_array_cell, &is_from_end)) { - my_printf_error(ER_UNKNOWN_ERROR, - "MDEV-39143: unparsable JSON path in a partial " - "JSON update: '%.*s'", MYF(0), - (int) diff.path_length, diff.path); + report_pj_apply_error(table, result_field, diff.path, + diff.path_length, "unparsable JSON path"); rc= HA_ERR_GENERIC; break; } + if (!is_array_cell) + { + bool member_exists; + if (json_diff_path_exists(thd, cur, diff.path, diff.path_length, + &member_exists)) + { + report_pj_apply_error(table, result_field, diff.path, + diff.path_length, + "could not check whether the path exists " + "in the current document"); + rc= HA_ERR_GENERIC; break; + } + if (member_exists) + { + report_pj_apply_error(table, result_field, diff.path, + diff.path_length, + "REJECTED - the object member to insert " + "already exists in the slave's document"); + rc= HA_ERR_GENERIC; break; + } + } /* Refuse rather than diverge silently. MySQL emits an array-cell INSERT only for a cell that does not exist, which for a @@ -640,16 +881,13 @@ static int apply_partial_json_column(const rpl_group_info *rgi, TABLE *table, */ if (is_array_cell && is_from_end) { - my_printf_error(ER_UNKNOWN_ERROR, - "MDEV-39143: cannot apply a partial JSON update " - "that inserts at a from-the-end array index: " - "path '%.*s' on %s.%s.%s. MariaDB's " - "JSON_ARRAY_INSERT resolves '[last-N]' " - "differently from MySQL, so the result would " - "silently differ from the source.", MYF(0), - (int) diff.path_length, diff.path, - table->s->db.str, table->s->table_name.str, - result_field->field_name.str); + report_pj_apply_error(table, result_field, diff.path, + diff.path_length, + "cannot apply an INSERT at a from-the-end " + "array index - MariaDB's JSON_ARRAY_INSERT " + "resolves '[last-N]' differently from " + "MySQL, so the result would silently " + "differ from the source"); rc= HA_ERR_GENERIC; break; } @@ -663,20 +901,36 @@ static int apply_partial_json_column(const rpl_group_info *rgi, TABLE *table, thd, args); } if (!func || func->fix_fields(thd, &func)) - { rc= HA_ERR_GENERIC; break; } + { + report_pj_apply_error(table, result_field, diff.path, + diff.path_length, + "internal error building the JSON function"); + rc= HA_ERR_GENERIC; break; + } String rbuf, *res= func->val_str(&rbuf); if (!res || func->null_value || cur.copy(*res)) - { rc= HA_ERR_GENERIC; break; } + { + report_pj_apply_error(table, result_field, diff.path, + diff.path_length, + "the JSON function produced no result"); + rc= HA_ERR_GENERIC; break; + } } thd->free_items(); thd->free_list= save_free_list; + thd->mem_root= save_mem_root; + free_root(&json_apply_root, MYF(0)); if (rc) return rc; result_field->set_notnull(); if (result_field->store(cur.ptr(), cur.length(), cur.charset())) + { + report_pj_apply_error(table, result_field, NULL, 0, + "could not store the resulting document"); return HA_ERR_GENERIC; + } return 0; } @@ -724,6 +978,17 @@ static int apply_partial_json_column(const rpl_group_info *rgi, TABLE *table, diff vector instead of a full value. Must be false for a before-image, which never has that header and never carries diffs, and for every other event type. + @param only_seek + True when there is no matched row to unpack into - the + caller is only advancing past this row image in the event + stream (e.g. Update_rows_log_event::do_exec_row() after + find_row() failed), not applying it. Only meaningful + together with is_partial_json_after_image: a partial JSON + column's diff vector is still decoded, to correctly + advance past it, but is discarded rather than applied - + see apply_partial_json_column(). Ignored otherwise, since + ordinary field unpacking never reads record[1], so it is + harmless regardless of whether a real row was matched. @retval 0 No error @@ -736,7 +1001,8 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const master_cols, uchar const *const row_data, MY_BITMAP const *cols, uchar const **const current_row_end, uchar const *const row_end, - bool is_partial_json_after_image) + bool is_partial_json_after_image, + bool only_seek) { int error; bool null_value; @@ -764,10 +1030,46 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const master_cols, size_t avail= (size_t) (row_end - rec_ptr); ulonglong value_options= safe_net_field_length_ll((uchar**) &rec_ptr, avail); + /* + safe_net_field_length_ll() sets its cursor to NULL on a truncated + encoding (e.g. the event ends right where the after-image header + should start) instead of leaving it unmoved - see the identical + hazard documented in read_mysql_json_diff_vector(). MySQL also + treats any value_options bit beyond PARTIAL_JSON_UPDATES (bit 0) as a + corrupt event, since none is defined; a value greater than 1 means + either corruption or a future MySQL version using bits this code does + not understand, so it must not be read as if bit 0 were the only one. + */ + if (!rec_ptr || value_options > PARTIAL_JSON_UPDATES) + { + /* + my_printf_error(), not rgi->rli->report(): see the comment on + report_pj_apply_error() above for why only a message already in the + diagnostics area when do_exec_row() returns survives to become + Last_SQL_Error. + */ + char msg[MYSQL_ERRMSG_SIZE]; + my_snprintf(msg, sizeof(msg), + "Could not read the partial JSON update header for " + "table '%s.%s'", table->s->db.str, table->s->table_name.str); + my_printf_error(ER_UNKNOWN_ERROR, "%s", MYF(0), msg); + DBUG_RETURN(HA_ERR_CORRUPT_EVENT); + } if (value_options & PARTIAL_JSON_UPDATES) { pj_partial_bits= rec_ptr; - rec_ptr+= (pj_tabledef->json_column_count() + 7) / 8; + size_t partial_bits_len= (pj_tabledef->json_column_count() + 7) / 8; + if (partial_bits_len > (size_t) (row_end - rec_ptr)) + { + char msg[MYSQL_ERRMSG_SIZE]; + my_snprintf(msg, sizeof(msg), + "Could not read the partial JSON update bitmap " + "for table '%s.%s'", table->s->db.str, + table->s->table_name.str); + my_printf_error(ER_UNKNOWN_ERROR, "%s", MYF(0), msg); + DBUG_RETURN(HA_ERR_CORRUPT_EVENT); + } + rec_ptr+= partial_bits_len; } } @@ -823,8 +1125,7 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const master_cols, if (pj_partial_bits && tabledef->binlog_type(master_idx) == MYSQL_TYPE_JSON_MYSQL) { - is_partial_col= (pj_partial_bits[pj_json_col / 8] & - (1 << (pj_json_col % 8))) != 0; + is_partial_col= rpl_bitmap_is_set(pj_partial_bits, pj_json_col); pj_json_col++; } /* @@ -901,7 +1202,8 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const master_cols, if (is_partial_col) { int pj_err= apply_partial_json_column(rgi, table, result_field, - conv_field, st, row_end); + conv_field, st, row_end, + only_seek); if (pj_err) DBUG_RETURN(pj_err); } @@ -927,8 +1229,22 @@ int unpack_row(const rpl_group_info *rgi, TABLE *table, uint const master_cols, If copy_fields is set, it means we are doing an online alter table, and will use copy_fields set up in copy_data_between_tables */ - if (conv_field) - convert_field(result_field, conv_field); + if (conv_field && convert_field(result_field, conv_field)) + { + /* + convert_field() only returns true once thd->is_error() is already + true (it checks that itself, after Copy_field has run) - that + existing, more specific message (e.g. the corrupt-JSONB error from + Field_mysql_json::val_str()) is what must survive to become + Last_SQL_Error, so it is left alone rather than pushing a second + one: do_exec_row()'s caller re-derives Last_SQL_Error from + thd->is_error() once the row event returns an error (see + convert_handler_error() in log_event_server.cc), and a second + my_error() call here would hit set_error_status()'s "no previous + error" assertion (sql_error.cc). + */ + DBUG_RETURN(HA_ERR_GENERIC); + } } } } diff --git a/sql/rpl_record.h b/sql/rpl_record.h index 92b3d7b41a8..15d5452ac24 100644 --- a/sql/rpl_record.h +++ b/sql/rpl_record.h @@ -34,7 +34,8 @@ int unpack_row(const rpl_group_info *rgi, uchar const *const row_data, MY_BITMAP const *cols, uchar const **const curr_row_end, uchar const *const row_end, - bool is_partial_json_after_image= false); + bool is_partial_json_after_image= false, + bool only_seek= false); // Fill table's record[0] with default values. int prepare_record(TABLE *const table); diff --git a/sql/rpl_utility.h b/sql/rpl_utility.h index 781c3870506..69277c4ced1 100644 --- a/sql/rpl_utility.h +++ b/sql/rpl_utility.h @@ -44,6 +44,16 @@ */ #define MYSQL_TYPE_JSON_MYSQL 245 +/* + Name of the data type plugin (plugin/type_mysql_json) that decodes + MYSQL_TYPE_JSON_MYSQL columns, as registered with + MariaDB_DATA_TYPE_INTERFACE_VERSION. Shared so the three lookup sites + (table.cc's .frm unpacking, and table_def::field_type_handler()'s two + uses in rpl_utility_server.cc) don't each spell it out separately. +*/ +static const LEX_CSTRING MYSQL_JSON_PLUGIN_NAME= + {STRING_WITH_LEN("MYSQL_JSON")}; + class Relay_log_info; class Log_event; class Rows_log_event; @@ -320,7 +330,8 @@ struct RPL_TABLE_LIST : public TABLE_LIST const Copy_field *online_alter_copy_fields_end); void create_column_mapping(rpl_group_info *rgi); bool give_compatibility_error(rpl_group_info *rgi, uint col); - bool check_wrong_column_usage(rpl_group_info *rgi, MY_BITMAP *m_cols); + bool check_wrong_column_usage(rpl_group_info *rgi, MY_BITMAP *m_cols, + MY_BITMAP *m_cols_ai= NULL); }; diff --git a/sql/rpl_utility_server.cc b/sql/rpl_utility_server.cc index 096cf2498b3..f8d52aafa9e 100644 --- a/sql/rpl_utility_server.cc +++ b/sql/rpl_utility_server.cc @@ -536,17 +536,35 @@ Field_longstr::rpl_conv_type_from(const Conv_source &source, /* A MySQL binary JSON source (mysql_json plugin), replicated from a MySQL master. The conversion-table field decodes the binary JSON into text. - - target native JSON: same logical type, only the storage format differs, - so this is lossless -> CONV_TYPE_VARIANT (allowed without - slave_type_conversions). - - target plain string/blob: a genuine cross-type conversion -> - CONV_TYPE_SUBSET_TO_SUPERSET (requires slave_type_conversions). + - target native JSON: same logical type, only the storage format + differs -> CONV_TYPE_VARIANT (allowed without slave_type_conversions, + i.e. treated as policy-safe by default). This is not quite lossless: + a MySQL JSONB OPAQUE value tagged as a temporal or DECIMAL type is + decoded to a plain quoted JSON string (see + plugin/type_mysql_json/mysql_json.cc print_mysql_datetime_value() + and the JSONB_TYPE_OPAQUE case), so the document's content survives + but that original type tag does not; MariaDB's native JSON has no + typed-scalar concept to preserve it in either representation, so + treating this as policy-safe is still the right default, just not + for the "nothing is lost" reason the old wording implied. + - target plain string/blob: a genuine cross-type conversion. Classify + it by actual declared capacity, the same way the parallel + blob/string-source branch below does, rather than assume it always + fits: Type_handler_mysql_json derives from Type_handler_blob and + inherits its max_display_length_for_field(), which reads the pack + length MYSQL_JSON stores in the same metadata slot an ordinary blob + would (see Type_handler_mysql_json::make_conversion_table_field()), + so compare_lengths() works on it unchanged. */ { - const LEX_CSTRING mysql_json{STRING_WITH_LEN("MYSQL_JSON")}; - if (source.type_handler()->name().eq(mysql_json)) - return Type_handler_json_common::is_json_type_handler(type_handler()) ? - CONV_TYPE_VARIANT : CONV_TYPE_SUBSET_TO_SUPERSET; + if (source.type_handler()->name().eq(MYSQL_JSON_PLUGIN_NAME)) + { + if (Type_handler_json_common::is_json_type_handler(type_handler())) + return CONV_TYPE_VARIANT; + return compare_lengths(source.type_handler(), + max_display_length_for_field(source), + type_handler(), max_display_length()); + } } /** @todo @@ -920,8 +938,7 @@ const Type_handler *table_def::field_type_handler(uint col) const */ if (typecode == MYSQL_TYPE_JSON_MYSQL) { - const LEX_CSTRING mysql_json{STRING_WITH_LEN("MYSQL_JSON")}; - return Type_handler::handler_by_name(current_thd, mysql_json); + return Type_handler::handler_by_name(current_thd, MYSQL_JSON_PLUGIN_NAME); } return Type_handler::get_handler_by_real_type(typecode); } @@ -1056,17 +1073,27 @@ table_def::compatible_with(THD *thd, rpl_group_info *rgi, /* - Check if there are any not supported columns are used + Check if there are any not supported columns are used. + + m_cols_ai, when given, is an Update_rows_log_event's after-image bitmap. + Under binlog_row_image=MINIMAL, a changed column can appear only in the + after-image and not in the before-image m_cols; without checking it here + too, an incompatible column visible only there (e.g. a MySQL binary JSON + column with the decoding plugin not installed) was never reported and was + just silently excluded from the write set instead, leaving its slave-side + value stale with no error raised. */ bool RPL_TABLE_LIST::check_wrong_column_usage(rpl_group_info *rgi, - MY_BITMAP *m_cols) + MY_BITMAP *m_cols, + MY_BITMAP *m_cols_ai) { DBUG_ENTER("RPL_TABLE_LIST::check_wrong_column_usage"); bool has_err= false; for (uint col= 0 ; col < m_tabledef.size() ; col++) { - if (!bitmap_is_set(m_cols, col)) + if (!bitmap_is_set(m_cols, col) && + !(m_cols_ai && bitmap_is_set(m_cols_ai, col))) continue; if (m_tabledef.master_to_slave_error[col]) has_err|= give_compatibility_error(rgi, col); diff --git a/sql/slave.cc b/sql/slave.cc index e7c664c01f0..d8507fc232e 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -6521,6 +6521,15 @@ dbug_gtid_accept: case WRITE_ROWS_EVENT: case UPDATE_ROWS_EVENT: case DELETE_ROWS_EVENT: + /* + MySQL 8.0+ partial JSON update (MDEV-39143); same row-event header shape + as UPDATE_ROWS_EVENT, including STMT_END_F, so it is tracked the same + way. Omitted here, a statement ending in this event type instead of a + plain UPDATE_ROWS_EVENT would leave stmt_end_seen false, causing a + false-positive "did not receive an expected Rows-log end-of-statement" + error on the next tracked statement. + */ + case PARTIAL_UPDATE_ROWS_EVENT: { is_rows_event= true; mi->rows_event_tracker.update(mi->master_log_name, diff --git a/sql/sql_type.cc b/sql/sql_type.cc index c90a3b64e93..ec7e50a3bcd 100644 --- a/sql/sql_type.cc +++ b/sql/sql_type.cc @@ -234,10 +234,20 @@ Type_handler::handler_by_name(THD *thd, const LEX_CSTRING &name) if ((plugin= my_plugin_lock_by_name(thd, &name, MariaDB_DATA_TYPE_PLUGIN))) { /* - Data type plugins do not maintain ref_count yet. - For now we have only mandatory built-in plugins - and dynamic plugins for test purposes. - It should be safe to unlock the plugin immediately. + Data type plugins do not maintain ref_count yet, so the Type_handler + below is returned with no lock protecting it: a concurrent + UNINSTALL SONAME of this plugin can unload it while the returned + pointer is still being used - e.g. cached in a Field, Column_definition + or table_def and dereferenced well after this call returns, including + by the mysql_json replication path (MDEV-39143) - which is a real, + currently-unguarded race. In practice this hasn't yet been a problem + because the data type plugins in actual use are either mandatory + built-ins (never unloaded) or loaded for test purposes under + controlled conditions. A real fix needs the returned handler to carry + a held plugin_ref for as long as it is used, the way + TABLE_SHARE::db_plugin pins a storage engine plugin for a table's + lifetime; that is a larger change than this comment's history + suggests, not a settled safety property. */ const Type_handler *ph= reinterpret_cast (plugin_decl(plugin)->info)->type_handler; diff --git a/sql/table.cc b/sql/table.cc index 8e07fe11313..34289555f21 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -2594,8 +2594,8 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, share->mysql_version >= 50700) { share->incompatible_version|= HA_CREATE_USED_ENGINE; - const LEX_CSTRING mysql_json{STRING_WITH_LEN("MYSQL_JSON")}; - handler= Type_handler::handler_by_name_or_error(thd, mysql_json); + handler= Type_handler::handler_by_name_or_error( + thd, MYSQL_JSON_PLUGIN_NAME); } if (!handler)