Skip to content

Commit 0457aaa

Browse files
tonychen2001tonychen2001
committed
MDEV-39223 Reversed executable comments incorrectly written in binlog
When a reversed executable comment (/*!!version */ or /*M!!version */) is not executed on the master (server version >= specified version), the binlog writer only neutralized the second '!' to a space, leaving `/*! command */`, an executable comment the slave would then try to execute. Fix (non-executed path): also replace the first '!' with a space so the SQL becomes /* (a plain comment). The restore path for unclosed comments also restores both '!' characters. Additionally, when a reversed executable comment IS executed on the source (server version < specified version), the /*!!version syntax was preserved verbatim in the binlog. A replica with a higher version could re-evaluate the reversed condition, fail it, and skip the command — causing source/replica divergence. Unlike forward executable comments (/*!version */), where replication to same-or-higher versions guarantees the replica also executes, reversed comments have the opposite semantics and are not safe to preserve as-is. Fix (executed path): overwrite the version digits with spaces in the raw query buffer so the binlog contains /*!! (a non-versioned reversed executable comment) which the replica always executes regardless of its version. Added rpl_reversed_comments.test mirroring rpl_conditional_comments.test with reversed equivalents for all forward comment replication test cases: mix of applied/not-applied, prepared statements, unclosed comments, nested comments, delimiter edge case, and MariaDB-specific /*M!! syntax. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
1 parent 3a2f8e2 commit 0457aaa

File tree

3 files changed

+278
-0
lines changed

3 files changed

+278
-0
lines changed

‎mysql-test/suite/rpl/r/rpl_reversed_comments.result‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
include/master-slave.inc
2+
[connection master]
3+
CREATE TABLE t1(c1 INT);
4+
include/show_binlog_events.inc
5+
Log_name Pos Event_type Server_id End_log_pos Info
6+
master-bin.000001 # Gtid # # GTID #-#-#
7+
master-bin.000001 # Query # # use `test`; CREATE TABLE t1(c1 INT)
8+
9+
# Case 1:
10+
# ------------------------------------------------------------------
11+
# Mix of applied and not-applied reversed CCs in a single statement.
12+
# /*!!999999 ... */ executes (server version < 999999).
13+
# /*!!100000 ... */ does not execute (server version >= 100000).
14+
# The not-applied ones must become plain comments in the binlog.
15+
/*!!100000 --- */INSERT /*!!999999 INTO*/ /*!10000 t1 */ VALUES(10) /*!!100000 ,(11)*/;
16+
include/show_binlog_events.inc
17+
Log_name Pos Event_type Server_id End_log_pos Info
18+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
19+
master-bin.000001 # Query # # use `test`; /* 100000 --- */INSERT /*!! INTO*/ /*!10000 t1 */ VALUES(10) /* 100000 ,(11)*/
20+
master-bin.000001 # Query # # COMMIT
21+
connection slave;
22+
include/diff_tables.inc [master:t1,slave:t1]
23+
24+
# Case 2:
25+
# -----------------------------------------------------------------
26+
# Prepared statements with reversed executable comments.
27+
connection master;
28+
PREPARE stmt FROM 'INSERT INTO /*!!100000 blabla*/ t1 VALUES(60) /*!!100000 ,(61)*/';
29+
EXECUTE stmt;
30+
DROP TABLE t1;
31+
CREATE TABLE t1(c1 INT);
32+
EXECUTE stmt;
33+
connection slave;
34+
include/diff_tables.inc [master:t1,slave:t1]
35+
connection master;
36+
37+
SET @value=62;
38+
PREPARE stmt FROM 'INSERT INTO /*!!100000 blabla */ t1 VALUES(?) /*!!100000 ,(63)*/';
39+
EXECUTE stmt USING @value;
40+
DROP TABLE t1;
41+
CREATE TABLE t1(c1 INT);
42+
EXECUTE stmt USING @value;
43+
include/show_binlog_events.inc
44+
Log_name Pos Event_type Server_id End_log_pos Info
45+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
46+
master-bin.000001 # Query # # use `test`; INSERT INTO /* 100000 blabla*/ t1 VALUES(60) /* 100000 ,(61)*/
47+
master-bin.000001 # Query # # COMMIT
48+
master-bin.000001 # Gtid # # GTID #-#-#
49+
master-bin.000001 # Query # # use `test`; DROP TABLE `t1` /* generated by server */
50+
master-bin.000001 # Gtid # # GTID #-#-#
51+
master-bin.000001 # Query # # use `test`; CREATE TABLE t1(c1 INT)
52+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
53+
master-bin.000001 # Query # # use `test`; INSERT INTO /* 100000 blabla*/ t1 VALUES(60) /* 100000 ,(61)*/
54+
master-bin.000001 # Query # # COMMIT
55+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
56+
master-bin.000001 # Query # # use `test`; INSERT INTO /* 100000 blabla */ t1 VALUES(62) /* 100000 ,(63)*/
57+
master-bin.000001 # Query # # COMMIT
58+
master-bin.000001 # Gtid # # GTID #-#-#
59+
master-bin.000001 # Query # # use `test`; DROP TABLE `t1` /* generated by server */
60+
master-bin.000001 # Gtid # # GTID #-#-#
61+
master-bin.000001 # Query # # use `test`; CREATE TABLE t1(c1 INT)
62+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
63+
master-bin.000001 # Query # # use `test`; INSERT INTO /* 100000 blabla */ t1 VALUES(62) /* 100000 ,(63)*/
64+
master-bin.000001 # Query # # COMMIT
65+
connection slave;
66+
include/diff_tables.inc [master:t1,slave:t1]
67+
68+
# Case 3:
69+
# -----------------------------------------------------------------
70+
# Unclosed reversed executable comment — the '!!' must be restored
71+
# so the parser produces a proper syntax error.
72+
connection master;
73+
SELECT c1 FROM /*!!100000 t1 WHEREN;
74+
ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '/*!!100000 t1 WHEREN' at line 1
75+
76+
# Case 4:
77+
# -----------------------------------------------------------------
78+
# Nested comments inside reversed executable comments.
79+
insert t1 values (/*!!999999 1 /* foo */ */ + 2);
80+
insert t1 values (/*!!100000 10 /* foo */ */ + 20);
81+
include/show_binlog_events.inc
82+
Log_name Pos Event_type Server_id End_log_pos Info
83+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
84+
master-bin.000001 # Query # # use `test`; insert t1 values (/*!! 1 /* foo */ */ + 2)
85+
master-bin.000001 # Query # # COMMIT
86+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
87+
master-bin.000001 # Query # # use `test`; insert t1 values (/* 100000 10 (* foo *) */ + 20)
88+
master-bin.000001 # Query # # COMMIT
89+
connection slave;
90+
select * from t1;
91+
c1
92+
62
93+
3
94+
20
95+
connection master;
96+
97+
# Case 5:
98+
# -----------------------------------------------------------------
99+
# Delimiter change with reversed executable comment. A parse error
100+
# must not cause master/slave divergence.
101+
insert t1 values /*!! (100);insert t1 values */ (200) //
102+
ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'insert t1 values */ (200)' at line 1
103+
select * from t1;
104+
c1
105+
62
106+
3
107+
20
108+
connection slave;
109+
select * from t1;
110+
c1
111+
62
112+
3
113+
20
114+
connection master;
115+
116+
# Case 6:
117+
# -----------------------------------------------------------------
118+
# MariaDB-specific reversed comments /*M!!version */
119+
# /*M!!100000 */ does not execute (server >= 100000), must be plain
120+
# comment in binlog. /*M!!999999 */ executes (server < 999999).
121+
DROP TABLE t1;
122+
CREATE TABLE t1(c1 INT);
123+
INSERT /*M!!100000 blabla */ INTO t1 VALUES(70);
124+
INSERT INTO t1 VALUES(/*M!!999999 80 + */ 1);
125+
include/show_binlog_events.inc
126+
Log_name Pos Event_type Server_id End_log_pos Info
127+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
128+
master-bin.000001 # Query # # use `test`; INSERT /*M 100000 blabla */ INTO t1 VALUES(70)
129+
master-bin.000001 # Query # # COMMIT
130+
master-bin.000001 # Gtid # # BEGIN GTID #-#-#
131+
master-bin.000001 # Query # # use `test`; INSERT INTO t1 VALUES(/*M!! 80 + */ 1)
132+
master-bin.000001 # Query # # COMMIT
133+
connection slave;
134+
include/diff_tables.inc [master:t1,slave:t1]
135+
connection master;
136+
DROP TABLE t1;
137+
include/rpl_end.inc

‎mysql-test/suite/rpl/t/rpl_reversed_comments.test‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
###############################################################################
2+
# MDEV-39223: Reversed executable comments incorrectly written in binlog
3+
#
4+
# Mirrors rpl_conditional_comments.test for reversed executable comments
5+
# (/*!!version */ and /*M!!version */ syntax).
6+
#
7+
# - Use ' ' instead of '!' in the conditional comments which are not applied on
8+
# master. So they become common comments and will not be applied on slave.
9+
#
10+
# - Example:
11+
# 'INSERT INTO t1 VALUES (1) /*!!10000, (2)*/ /*!!999999 ,(3)*/
12+
# will be binlogged as
13+
# 'INSERT INTO t1 VALUES (1) /* 10000, (2)*/ /*!!999999 ,(3)*/'.
14+
###############################################################################
15+
source include/have_binlog_format_statement.inc;
16+
source include/master-slave.inc;
17+
18+
CREATE TABLE t1(c1 INT);
19+
source include/show_binlog_events.inc;
20+
let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
21+
22+
--echo
23+
--echo # Case 1:
24+
--echo # ------------------------------------------------------------------
25+
--echo # Mix of applied and not-applied reversed CCs in a single statement.
26+
--echo # /*!!999999 ... */ executes (server version < 999999).
27+
--echo # /*!!100000 ... */ does not execute (server version >= 100000).
28+
--echo # The not-applied ones must become plain comments in the binlog.
29+
30+
/*!!100000 --- */INSERT /*!!999999 INTO*/ /*!10000 t1 */ VALUES(10) /*!!100000 ,(11)*/;
31+
32+
source include/show_binlog_events.inc;
33+
let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
34+
sync_slave_with_master;
35+
--let $diff_tables= master:t1,slave:t1
36+
--source include/diff_tables.inc
37+
38+
--echo
39+
--echo # Case 2:
40+
--echo # -----------------------------------------------------------------
41+
--echo # Prepared statements with reversed executable comments.
42+
43+
--connection master
44+
PREPARE stmt FROM 'INSERT INTO /*!!100000 blabla*/ t1 VALUES(60) /*!!100000 ,(61)*/';
45+
EXECUTE stmt;
46+
DROP TABLE t1;
47+
CREATE TABLE t1(c1 INT);
48+
EXECUTE stmt;
49+
50+
sync_slave_with_master;
51+
--let $diff_tables= master:t1,slave:t1
52+
--source include/diff_tables.inc
53+
54+
--connection master
55+
--echo
56+
SET @value=62;
57+
PREPARE stmt FROM 'INSERT INTO /*!!100000 blabla */ t1 VALUES(?) /*!!100000 ,(63)*/';
58+
EXECUTE stmt USING @value;
59+
DROP TABLE t1;
60+
CREATE TABLE t1(c1 INT);
61+
EXECUTE stmt USING @value;
62+
63+
source include/show_binlog_events.inc;
64+
let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
65+
66+
sync_slave_with_master;
67+
--let $diff_tables= master:t1,slave:t1
68+
--source include/diff_tables.inc
69+
70+
--echo
71+
--echo # Case 3:
72+
--echo # -----------------------------------------------------------------
73+
--echo # Unclosed reversed executable comment — the '!!' must be restored
74+
--echo # so the parser produces a proper syntax error.
75+
76+
--connection master
77+
--error 1064
78+
SELECT c1 FROM /*!!100000 t1 WHEREN; #*/
79+
80+
--echo
81+
--echo # Case 4:
82+
--echo # -----------------------------------------------------------------
83+
--echo # Nested comments inside reversed executable comments.
84+
85+
insert t1 values (/*!!999999 1 /* foo */ */ + 2);
86+
insert t1 values (/*!!100000 10 /* foo */ */ + 20);
87+
source include/show_binlog_events.inc;
88+
sync_slave_with_master;
89+
select * from t1;
90+
connection master;
91+
92+
--echo
93+
--echo # Case 5:
94+
--echo # -----------------------------------------------------------------
95+
--echo # Delimiter change with reversed executable comment. A parse error
96+
--echo # must not cause master/slave divergence.
97+
98+
delimiter //;
99+
--error ER_PARSE_ERROR
100+
insert t1 values /*!! (100);insert t1 values */ (200) //
101+
delimiter ;//
102+
select * from t1;
103+
sync_slave_with_master;
104+
select * from t1;
105+
connection master;
106+
107+
--echo
108+
--echo # Case 6:
109+
--echo # -----------------------------------------------------------------
110+
--echo # MariaDB-specific reversed comments /*M!!version */
111+
--echo # /*M!!100000 */ does not execute (server >= 100000), must be plain
112+
--echo # comment in binlog. /*M!!999999 */ executes (server < 999999).
113+
114+
DROP TABLE t1;
115+
CREATE TABLE t1(c1 INT);
116+
let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1);
117+
INSERT /*M!!100000 blabla */ INTO t1 VALUES(70);
118+
INSERT INTO t1 VALUES(/*M!!999999 80 + */ 1);
119+
source include/show_binlog_events.inc;
120+
sync_slave_with_master;
121+
--let $diff_tables= master:t1,slave:t1
122+
--source include/diff_tables.inc
123+
124+
connection master;
125+
DROP TABLE t1;
126+
--source include/rpl_end.inc

‎sql/sql_lex.cc‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2479,6 +2479,17 @@ int Lex_input_stream::lex_one_token(YYSTYPE *yylval, THD *thd)
24792479
(version < 50700 || version > 99999 || maria_comment_syntax)) ||
24802480
(reversed_comment && MYSQL_VERSION_ID < version))
24812481
{
2482+
if (reversed_comment)
2483+
{
2484+
/*
2485+
Overwrite the version digits with spaces in the raw query
2486+
buffer so the binlog contains a non-versioned reversed
2487+
executable comment that the replica always executes.
2488+
*/
2489+
char *p= (char *) get_ptr();
2490+
for (uint i= 0; i < length; i++)
2491+
p[i]= ' ';
2492+
}
24822493
/* Accept 'M' 'm' 'm' 'd' 'd' */
24832494
yySkipn(length);
24842495
/* Expand the content of the special comment as real code */
@@ -2504,10 +2515,14 @@ int Lex_input_stream::lex_one_token(YYSTYPE *yylval, THD *thd)
25042515
being propagated infinitely (eg. to a slave).
25052516
*/
25062517
char *pcom= yyUnput(' ');
2518+
if (reversed_comment)
2519+
*(pcom - 1)= ' ';
25072520
comment_closed= ! consume_comment(1);
25082521
if (! comment_closed)
25092522
{
25102523
*pcom= '!';
2524+
if (reversed_comment)
2525+
*(pcom - 1)= '!';
25112526
}
25122527
/* version allowed to have one level of comment inside. */
25132528
}

0 commit comments

Comments
 (0)