database/mariadb
1
0
Fork 0
mirror of https://github.com/MariaDB/server.git synced 2026-10-23 18:49:29 +03:00
Code Activity

MDEV-39143 follow-up: fix review findings in MySQL JSON replication

Browse Source 
Addresses issues found in review of 16ce07df15 (Replicate MySQL binary
JSON to a MariaDB slave), in both the new code and two pieces of
pre-existing core replication code it interacts with. Issue numbers
below refer to the review's own numbering (I001-I039).

Crashes and silent corruption:
 - A truncated or malformed Json_diff vector in a
   PARTIAL_UPDATE_ROWS_EVENT could NULL-deref the slave:
   safe_net_field_length_ll() nulls its cursor on a truncated
   encoding, and nothing checked for that. (I001, I002)
 - The same unchecked-cursor bug existed in the after-image header
   parser, which additionally had no bound on partial_bits
   consumption and did not reject out-of-range value_options. (I005)
 - A diff that MySQL's own applier would REJECT (its path doesn't
   match the slave's document) was silently applied as a no-op
   instead of halting replication - the opposite of MySQL's own
   behavior (ER_COULD_NOT_APPLY_JSON_DIFF). (I004, I025)
 - Corrupt MySQL binary JSON in a diff value was committed as a JSON
   null instead of being detected: Field_mysql_json::val_str() pushes
   an error but returns a non-null empty string on failure, so the
   previous `if (!vtext)` check could never fire. (I003)
 - The same silent-corruption class existed on the full-document
   conversion path for a native-JSON target. (I038)
 - find_row() failing to locate the matched row still read the base
   document from stale record[1] data, and did not record the
   correct stream position on error, risking a parse desync under
   slave_exec_mode=IDEMPOTENT. (I020, I033)

Error reporting:
 - Most failure paths in apply_partial_json_column() returned a bare
   HA_ERR_GENERIC with nothing reported to the DBA, contradicting the
   function's own documented contract. (I024, I026, I027, I031)
 - Fixed messages use my_printf_error() rather than
   rgi->rli->report(): do_exec_row()'s caller unconditionally
   re-derives Last_SQL_Error from thd->is_error() once a row event
   returns an error, overwriting anything rgi->rli->report() had
   already recorded with a generic "Got error NNN from storage
   engine" wrapper - only a message already in the diagnostics area
   survives.
 - check_wrong_column_usage() only checked an UPDATE event's
   before-image bitmap, so an incompatible column visible only in
   the after-image (binlog_row_image=MINIMAL) was silently dropped
   from the write set instead of being reported. (I006)
 - slave.cc's IO-thread statement-end tracking was missing
   PARTIAL_UPDATE_ROWS_EVENT, which could produce a false-positive
   "did not receive an expected Rows-log end-of-statement" error.
   (I008)
 - Documented, but did not fix (needs generic plugin refcounting,
   out of scope here): Type_handler::handler_by_name() releases its
   plugin lock before the caller is done using the returned handler,
   a pre-existing race in the data-type plugin framework. (I039)

Design/consistency:
 - A MySQL JSON source converting to a plain string/blob target was
   always classified as safely widening regardless of actual size;
   it is now classified by the target's real declared capacity like
   any other blob/string conversion, so a document that doesn't fit
   requires slave_type_conversions=ALL_LOSSY instead of silently
   truncating under ALL_NON_LOSSY alone. (I007)
 - apply_partial_json_column() now builds its per-diff JSON function
   Items on a private MEM_ROOT freed per row/column, instead of
   thd->mem_root (the statement-level arena), avoiding unbounded
   growth on a large multi-row UPDATE. (I028)
 - An open-coded bitmap test duplicating the existing
   rpl_bitmap_is_set() helper was replaced with a call to it. (I032)
 - Corrected a comment overstating MySQL JSON -> native JSON
   conversion as lossless; a JSONB OPAQUE temporal/decimal value
   loses its original type tag even though its content survives.
   (I036)
 - Deduplicated the "MYSQL_JSON" plugin-name literal into one
   constant. (I037)

Tests:
 - Switched the existing tests from unconditional INSTALL/UNINSTALL
   SONAME to the repo's have_type_mysql_json.inc convention. (I013)
 - Corrected a test comment that misattributed why the from-the-end
   array-index diff is refused. (I014)
 - Extracted the tests' duplicated binlog-fixture-injection and
   post-replication resync logic into shared include files. (I015)
 - Added tests for the missing-plugin, conversion-length-gating, and
   corrupt-diff-vector error paths above. (I016)

Assisted-by: Claude AI
This commit is contained in:
Pekka Lampio
2026-10-16 15:38:55 +03:00
parent 16ce07df15
commit 88f91a4bf8
24 changed files with 809 additions and 142 deletions
Download Patch File Download Diff File Expand all files Collapse all files

BIN
mysql-test/std_data/mdev39143_corrupt_diff_vector.000001 Normal file
View File

Binary file not shown.

18
mysql-test/suite/rpl/include/mdev39143_inject_mysql_binlog.inc Normal file
View File

@@ -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= <path to the .000001 file>
# 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=

24
mysql-test/suite/rpl/include/mdev39143_resync_after_mysql_binlog.inc Normal file
View File

@@ -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= <database to drop> 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=

2
mysql-test/suite/rpl/r/rpl_from_mysql80.result
View File

@@ -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

33
mysql-test/suite/rpl/r/rpl_json_conv_gating.result Normal file
View File

@@ -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

30
mysql-test/suite/rpl/r/rpl_json_corrupt_diff_vector.result Normal file
View File

@@ -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

29
mysql-test/suite/rpl/r/rpl_json_missing_plugin.result Normal file
View File

@@ -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

6
mysql-test/suite/rpl/r/rpl_json_partial_insert.result
View File

@@ -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

2
mysql-test/suite/rpl/r/rpl_json_partial_update.result
View File

@@ -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

14
mysql-test/suite/rpl/t/rpl_from_mysql80.test
View File

@@ -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

62
mysql-test/suite/rpl/t/rpl_json_conv_gating.test Normal file
View File

@@ -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

57
mysql-test/suite/rpl/t/rpl_json_corrupt_diff_vector.test Normal file
View File

@@ -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

62
mysql-test/suite/rpl/t/rpl_json_missing_plugin.test Normal file
View File

@@ -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

37
mysql-test/suite/rpl/t/rpl_json_partial_insert.test
View File

@@ -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

35
mysql-test/suite/rpl/t/rpl_json_partial_update.test
View File

@@ -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

15
sql/log_event.h
View File

@@ -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]

13
sql/log_event_server.cc
View File

@@ -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);

410
sql/rpl_record.cc
View File

@@ -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)(&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<Item> 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<Mysql_json_diff> 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<uchar *>(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);
}
}
}
}

3
sql/rpl_record.h
View File

@@ -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);

13
sql/rpl_utility.h
View File

@@ -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);
};

55
sql/rpl_utility_server.cc
View File

@@ -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);

9
sql/slave.cc
View File

@@ -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,

18
sql/sql_type.cc
View File

@@ -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<st_mariadb_data_type*>
(plugin_decl(plugin)->info)->type_handler;

4
sql/table.cc
View File

@@ -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)