From b424320ffe0c82cbfd4c3a2bf5301825a253cddb Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Tue, 31 Mar 2026 21:40:57 +0000 Subject: [PATCH 1/2] MDEV-39223 Reversed executable comments incorrectly written in binlog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Reviewed-by: Brandon Nesterenko --- .../suite/plugins/r/server_audit.result | 4 +- .../suite/rpl/r/rpl_reversed_comments.result | 152 ++++++++++++++++++ .../suite/rpl/t/rpl_reversed_comments.cnf | 15 ++ .../suite/rpl/t/rpl_reversed_comments.test | 147 +++++++++++++++++ sql/sql_lex.cc | 15 ++ 5 files changed, 331 insertions(+), 2 deletions(-) create mode 100644 mysql-test/suite/rpl/r/rpl_reversed_comments.result create mode 100644 mysql-test/suite/rpl/t/rpl_reversed_comments.cnf create mode 100644 mysql-test/suite/rpl/t/rpl_reversed_comments.test diff --git a/mysql-test/suite/plugins/r/server_audit.result b/mysql-test/suite/plugins/r/server_audit.result index 3e432fb4e8dcb..265f0452c51e4 100644 --- a/mysql-test/suite/plugins/r/server_audit.result +++ b/mysql-test/suite/plugins/r/server_audit.result @@ -556,8 +556,8 @@ TIME,HOSTNAME,root,localhost:PORT,ID,ID,QUERY,sa_db,'/*M! SELECT 5 */',0 TIME,HOSTNAME,root,localhost:PORT,ID,ID,QUERY,sa_db,'/*!100100 SELECT 6 */',0 TIME,HOSTNAME,root,localhost:PORT,ID,ID,QUERY,sa_db,'/*M!100100 SELECT 7 */',0 TIME,HOSTNAME,root,localhost:PORT,ID,ID,QUERY,sa_db,'/*!! SELECT 8 */',0 -TIME,HOSTNAME,root,localhost:PORT,ID,ID,QUERY,sa_db,'/*!!999999 SELECT 9 */',0 -TIME,HOSTNAME,root,localhost:PORT,ID,ID,QUERY,sa_db,'/*M!!999999 SELECT 10 */',0 +TIME,HOSTNAME,root,localhost:PORT,ID,ID,QUERY,sa_db,'/*!! SELECT 9 */',0 +TIME,HOSTNAME,root,localhost:PORT,ID,ID,QUERY,sa_db,'/*M!! SELECT 10 */',0 TIME,HOSTNAME,root,localhost:PORT,ID,ID,QUERY,sa_db,'CREATE USER u1 IDENTIFIED BY *****',0 TIME,HOSTNAME,root,localhost:PORT,ID,ID,QUERY,sa_db,'GRANT ALL ON sa_db TO u2 IDENTIFIED BY *****',0 TIME,HOSTNAME,root,localhost:PORT,ID,ID,QUERY,sa_db,'CREATE USER u3 IDENTIFIED BY *****',0 diff --git a/mysql-test/suite/rpl/r/rpl_reversed_comments.result b/mysql-test/suite/rpl/r/rpl_reversed_comments.result new file mode 100644 index 0000000000000..92d1eb1425293 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_reversed_comments.result @@ -0,0 +1,152 @@ +include/rpl_init.inc [topology=1->2, 2->3] +connection server_1; +CREATE TABLE t1(c1 INT); +include/show_binlog_events.inc +Log_name Pos Event_type Server_id End_log_pos Info +master-bin.000001 # Gtid # # GTID #-#-# +master-bin.000001 # Query # # use `test`; CREATE TABLE t1(c1 INT) + +# Case 1: +# ------------------------------------------------------------------ +# Mix of applied and not-applied reversed CCs in a single statement. +# /*!!999999 ... */ executes (server version < 999999). +# /*!!100000 ... */ does not execute (server version >= 100000). +# The not-applied ones must become plain comments in the binlog. +/*!!100000 --- */INSERT /*!!999999 INTO*/ /*!10000 t1 */ VALUES(10) /*!!100000 ,(11)*/; +include/show_binlog_events.inc +Log_name Pos Event_type Server_id End_log_pos Info +master-bin.000001 # Gtid # # BEGIN GTID #-#-# +master-bin.000001 # Query # # use `test`; /* 100000 --- */INSERT /*!! INTO*/ /*!10000 t1 */ VALUES(10) /* 100000 ,(11)*/ +master-bin.000001 # Query # # COMMIT +include/save_master_gtid.inc +connection server_3; +include/sync_with_master_gtid.inc +include/diff_tables.inc [server_1:t1,server_3:t1] + +# Case 2: +# ----------------------------------------------------------------- +# Prepared statements with reversed executable comments. +connection server_1; +PREPARE stmt FROM 'INSERT INTO /*!!100000 blabla*/ t1 VALUES(60) /*!!100000 ,(61)*/'; +EXECUTE stmt; +DROP TABLE t1; +CREATE TABLE t1(c1 INT); +EXECUTE stmt; +include/save_master_gtid.inc +connection server_3; +include/sync_with_master_gtid.inc +include/diff_tables.inc [server_1:t1,server_3:t1] +connection server_1; + +SET @value=62; +PREPARE stmt FROM 'INSERT INTO /*!!100000 blabla */ t1 VALUES(?) /*!!100000 ,(63)*/'; +EXECUTE stmt USING @value; +DROP TABLE t1; +CREATE TABLE t1(c1 INT); +EXECUTE stmt USING @value; +include/show_binlog_events.inc +Log_name Pos Event_type Server_id End_log_pos Info +master-bin.000001 # Gtid # # BEGIN GTID #-#-# +master-bin.000001 # Query # # use `test`; INSERT INTO /* 100000 blabla*/ t1 VALUES(60) /* 100000 ,(61)*/ +master-bin.000001 # Query # # COMMIT +master-bin.000001 # Gtid # # GTID #-#-# +master-bin.000001 # Query # # use `test`; DROP TABLE `t1` /* generated by server */ +master-bin.000001 # Gtid # # GTID #-#-# +master-bin.000001 # Query # # use `test`; CREATE TABLE t1(c1 INT) +master-bin.000001 # Gtid # # BEGIN GTID #-#-# +master-bin.000001 # Query # # use `test`; INSERT INTO /* 100000 blabla*/ t1 VALUES(60) /* 100000 ,(61)*/ +master-bin.000001 # Query # # COMMIT +master-bin.000001 # Gtid # # BEGIN GTID #-#-# +master-bin.000001 # Query # # use `test`; INSERT INTO /* 100000 blabla */ t1 VALUES(62) /* 100000 ,(63)*/ +master-bin.000001 # Query # # COMMIT +master-bin.000001 # Gtid # # GTID #-#-# +master-bin.000001 # Query # # use `test`; DROP TABLE `t1` /* generated by server */ +master-bin.000001 # Gtid # # GTID #-#-# +master-bin.000001 # Query # # use `test`; CREATE TABLE t1(c1 INT) +master-bin.000001 # Gtid # # BEGIN GTID #-#-# +master-bin.000001 # Query # # use `test`; INSERT INTO /* 100000 blabla */ t1 VALUES(62) /* 100000 ,(63)*/ +master-bin.000001 # Query # # COMMIT +include/save_master_gtid.inc +connection server_3; +include/sync_with_master_gtid.inc +include/diff_tables.inc [server_1:t1,server_3:t1] + +# Case 3: +# ----------------------------------------------------------------- +# Unclosed reversed executable comment — the '!!' must be restored +# so the parser produces a proper syntax error. +connection server_1; +SELECT c1 FROM /*!!100000 t1 WHEREN; +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 + +# Case 4: +# ----------------------------------------------------------------- +# Nested comments inside reversed executable comments. +insert t1 values (/*!!999999 1 /* foo */ */ + 2); +insert t1 values (/*!!100000 10 /* foo */ */ + 20); +include/show_binlog_events.inc +Log_name Pos Event_type Server_id End_log_pos Info +master-bin.000001 # Gtid # # BEGIN GTID #-#-# +master-bin.000001 # Query # # use `test`; insert t1 values (/*!! 1 /* foo */ */ + 2) +master-bin.000001 # Query # # COMMIT +master-bin.000001 # Gtid # # BEGIN GTID #-#-# +master-bin.000001 # Query # # use `test`; insert t1 values (/* 100000 10 (* foo *) */ + 20) +master-bin.000001 # Query # # COMMIT +include/save_master_gtid.inc +connection server_3; +include/sync_with_master_gtid.inc +select * from t1; +c1 +62 +3 +20 +connection server_1; + +# Case 5: +# ----------------------------------------------------------------- +# Delimiter change with reversed executable comment. A parse error +# must not cause source/replica divergence. +insert t1 values /*!! (100);insert t1 values */ (200) // +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 +select * from t1; +c1 +62 +3 +20 +include/save_master_gtid.inc +connection server_3; +include/sync_with_master_gtid.inc +select * from t1; +c1 +62 +3 +20 +connection server_1; + +# Case 6: +# ----------------------------------------------------------------- +# MariaDB-specific reversed comments /*M!!version */ +# /*M!!100000 */ does not execute (server >= 100000), must be plain +# comment in binlog. /*M!!999999 */ executes (server < 999999). +DROP TABLE t1; +CREATE TABLE t1(c1 INT); +INSERT /*M!!100000 blabla */ INTO t1 VALUES(70); +INSERT INTO t1 VALUES(/*M!!999999 80 + */ 1); +include/show_binlog_events.inc +Log_name Pos Event_type Server_id End_log_pos Info +master-bin.000001 # Gtid # # BEGIN GTID #-#-# +master-bin.000001 # Query # # use `test`; INSERT /*M 100000 blabla */ INTO t1 VALUES(70) +master-bin.000001 # Query # # COMMIT +master-bin.000001 # Gtid # # BEGIN GTID #-#-# +master-bin.000001 # Query # # use `test`; INSERT INTO t1 VALUES(/*M!! 80 + */ 1) +master-bin.000001 # Query # # COMMIT +include/save_master_gtid.inc +connection server_3; +include/sync_with_master_gtid.inc +include/diff_tables.inc [server_1:t1,server_3:t1] +connection server_1; +DROP TABLE t1; +include/save_master_gtid.inc +connection server_3; +include/sync_with_master_gtid.inc +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_reversed_comments.cnf b/mysql-test/suite/rpl/t/rpl_reversed_comments.cnf new file mode 100644 index 0000000000000..cd4385fcb73e4 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_reversed_comments.cnf @@ -0,0 +1,15 @@ +!include ../my.cnf + +[mysqld.1] +server_id=1 + +[mysqld.2] +server_id=2 +log_slave_updates + +[mysqld.3] +server_id=3 + +[ENV] +SERVER_MYPORT_3= @mysqld.3.port +SERVER_MYSOCK_3= @mysqld.3.socket diff --git a/mysql-test/suite/rpl/t/rpl_reversed_comments.test b/mysql-test/suite/rpl/t/rpl_reversed_comments.test new file mode 100644 index 0000000000000..b5c9925be1abe --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_reversed_comments.test @@ -0,0 +1,147 @@ +############################################################################### +# MDEV-39223: Reversed executable comments incorrectly written in binlog +# +# Mirrors rpl_conditional_comments.test for reversed executable comments +# (/*!!version */ and /*M!!version */ syntax). +# +# Uses chain replication (1->2->3) to verify that binlog transformations +# survive re-binlogging through an intermediate server. +# +# - Use ' ' instead of '!' in the conditional comments which are not applied on +# the source. So they become common comments and will not be applied on replica. +# +# - Example: +# 'INSERT INTO t1 VALUES (1) /*!!10000, (2)*/ /*!!999999 ,(3)*/ +# will be binlogged as +# 'INSERT INTO t1 VALUES (1) /* 10000, (2)*/ /*!!999999 ,(3)*/'. +############################################################################### +source include/have_binlog_format_statement.inc; +--let $rpl_topology=1->2, 2->3 +--source include/rpl_init.inc + +--connection server_1 +CREATE TABLE t1(c1 INT); +source include/show_binlog_events.inc; +let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1); + +--echo +--echo # Case 1: +--echo # ------------------------------------------------------------------ +--echo # Mix of applied and not-applied reversed CCs in a single statement. +--echo # /*!!999999 ... */ executes (server version < 999999). +--echo # /*!!100000 ... */ does not execute (server version >= 100000). +--echo # The not-applied ones must become plain comments in the binlog. + +/*!!100000 --- */INSERT /*!!999999 INTO*/ /*!10000 t1 */ VALUES(10) /*!!100000 ,(11)*/; + +source include/show_binlog_events.inc; +let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1); +--source include/save_master_gtid.inc +--connection server_3 +--source include/sync_with_master_gtid.inc +--let $diff_tables= server_1:t1,server_3:t1 +--source include/diff_tables.inc + +--echo +--echo # Case 2: +--echo # ----------------------------------------------------------------- +--echo # Prepared statements with reversed executable comments. + +--connection server_1 +PREPARE stmt FROM 'INSERT INTO /*!!100000 blabla*/ t1 VALUES(60) /*!!100000 ,(61)*/'; +EXECUTE stmt; +DROP TABLE t1; +CREATE TABLE t1(c1 INT); +EXECUTE stmt; + +--source include/save_master_gtid.inc +--connection server_3 +--source include/sync_with_master_gtid.inc +--let $diff_tables= server_1:t1,server_3:t1 +--source include/diff_tables.inc + +--connection server_1 +--echo +SET @value=62; +PREPARE stmt FROM 'INSERT INTO /*!!100000 blabla */ t1 VALUES(?) /*!!100000 ,(63)*/'; +EXECUTE stmt USING @value; +DROP TABLE t1; +CREATE TABLE t1(c1 INT); +EXECUTE stmt USING @value; + +source include/show_binlog_events.inc; +let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1); + +--source include/save_master_gtid.inc +--connection server_3 +--source include/sync_with_master_gtid.inc +--let $diff_tables= server_1:t1,server_3:t1 +--source include/diff_tables.inc + +--echo +--echo # Case 3: +--echo # ----------------------------------------------------------------- +--echo # Unclosed reversed executable comment — the '!!' must be restored +--echo # so the parser produces a proper syntax error. + +--connection server_1 +--error 1064 +SELECT c1 FROM /*!!100000 t1 WHEREN; #*/ + +--echo +--echo # Case 4: +--echo # ----------------------------------------------------------------- +--echo # Nested comments inside reversed executable comments. + +insert t1 values (/*!!999999 1 /* foo */ */ + 2); +insert t1 values (/*!!100000 10 /* foo */ */ + 20); +source include/show_binlog_events.inc; +--source include/save_master_gtid.inc +--connection server_3 +--source include/sync_with_master_gtid.inc +select * from t1; +--connection server_1 + +--echo +--echo # Case 5: +--echo # ----------------------------------------------------------------- +--echo # Delimiter change with reversed executable comment. A parse error +--echo # must not cause source/replica divergence. + +delimiter //; +--error ER_PARSE_ERROR +insert t1 values /*!! (100);insert t1 values */ (200) // +delimiter ;// +select * from t1; +--source include/save_master_gtid.inc +--connection server_3 +--source include/sync_with_master_gtid.inc +select * from t1; +--connection server_1 + +--echo +--echo # Case 6: +--echo # ----------------------------------------------------------------- +--echo # MariaDB-specific reversed comments /*M!!version */ +--echo # /*M!!100000 */ does not execute (server >= 100000), must be plain +--echo # comment in binlog. /*M!!999999 */ executes (server < 999999). + +DROP TABLE t1; +CREATE TABLE t1(c1 INT); +let $binlog_start= query_get_value(SHOW MASTER STATUS, Position, 1); +INSERT /*M!!100000 blabla */ INTO t1 VALUES(70); +INSERT INTO t1 VALUES(/*M!!999999 80 + */ 1); +source include/show_binlog_events.inc; +--source include/save_master_gtid.inc +--connection server_3 +--source include/sync_with_master_gtid.inc +--let $diff_tables= server_1:t1,server_3:t1 +--source include/diff_tables.inc + +--connection server_1 +DROP TABLE t1; +--source include/save_master_gtid.inc +--connection server_3 +--source include/sync_with_master_gtid.inc + +--source include/rpl_end.inc diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index e1c7397fa6d94..c911fb3e296c7 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -2479,6 +2479,17 @@ int Lex_input_stream::lex_one_token(YYSTYPE *yylval, THD *thd) (version < 50700 || version > 99999 || maria_comment_syntax)) || (reversed_comment && MYSQL_VERSION_ID < version)) { + if (reversed_comment) + { + /* + Overwrite the version digits with spaces in the raw query + buffer so the binlog contains a non-versioned reversed + executable comment that the replica always executes. + */ + char *p= (char *) get_ptr(); + for (uint i= 0; i < length; i++) + p[i]= ' '; + } /* Accept 'M' 'm' 'm' 'd' 'd' */ yySkipn(length); /* 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) being propagated infinitely (eg. to a slave). */ char *pcom= yyUnput(' '); + if (reversed_comment) + *(pcom - 1)= ' '; comment_closed= ! consume_comment(1); if (! comment_closed) { *pcom= '!'; + if (reversed_comment) + *(pcom - 1)= '!'; } /* version allowed to have one level of comment inside. */ } From ca1b85f030fa8e4f3987b2f7520306ad87247146 Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Wed, 13 May 2026 18:50:50 +0000 Subject: [PATCH 2/2] MDEV-39421 Don't cache queries whose buffer is mutated by executable comments The lexer rewrites the raw query buffer when it takes a reversed executable comment (digits -> spaces) or skips a forward/reversed comment (the '!' -> space). With query_cache_strip_comments=OFF the cache key reads from this buffer, so the lookup key (built before parsing) and the store key (built after) disagree: the entry is inserted but never hit. Mark such queries as not safe to cache to avoid polluting the cache. When query_cache_strip_comments=ON caching remains enabled since its key comes from a separate comment-stripped copy of the query buffer. 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. --- ...query_cache_executable_comments-master.opt | 2 + .../query_cache_executable_comments.result | 172 ++++++++++++++++++ .../main/query_cache_executable_comments.test | 152 ++++++++++++++++ sql/sql_lex.cc | 14 ++ 4 files changed, 340 insertions(+) create mode 100644 mysql-test/main/query_cache_executable_comments-master.opt create mode 100644 mysql-test/main/query_cache_executable_comments.result create mode 100644 mysql-test/main/query_cache_executable_comments.test diff --git a/mysql-test/main/query_cache_executable_comments-master.opt b/mysql-test/main/query_cache_executable_comments-master.opt new file mode 100644 index 0000000000000..53b4ff314dfa2 --- /dev/null +++ b/mysql-test/main/query_cache_executable_comments-master.opt @@ -0,0 +1,2 @@ +--loose-query_cache_info +--plugin-load-add=$QUERY_CACHE_INFO_SO diff --git a/mysql-test/main/query_cache_executable_comments.result b/mysql-test/main/query_cache_executable_comments.result new file mode 100644 index 0000000000000..eebe37c89ce23 --- /dev/null +++ b/mysql-test/main/query_cache_executable_comments.result @@ -0,0 +1,172 @@ +# +# MDEV-39421: Queries with executable comments gets in query +# cache, but never hits. +# +# The lexer mutates the raw query buffer when: +# (a) a reversed comment /*!!NNNNNN ... */ is taken (digits -> spaces), +# (b) a forward or reversed comment is skipped (the '!' -> space). +# With query_cache_strip_comments=OFF (default), thd->base_query points +# into the same buffer the lexer mutates, so the lookup key (computed +# before parsing) and store key (computed after) disagree and the entry +# is inserted but never hit. +# +# Fix: in those two lexer sites, set lex->safe_to_cache_query=0 so the +# query is not cached in the first place. Caching still occurs when +# query_cache_strip_comments=ON. +# +SET @save_query_cache_type= @@global.query_cache_type; +SET @save_query_cache_size= @@global.query_cache_size; +SET GLOBAL query_cache_size= 1024*1024; +SET GLOBAL query_cache_type= ON; +SET LOCAL query_cache_type= ON; +CREATE TABLE t1 (c1 INT); +INSERT INTO t1 VALUES (1),(2),(3); +# +# Case 1: forward MySQL-compatible executable comment /*!NNNNNN ... */ +# +RESET QUERY CACHE; +FLUSH STATUS; +SELECT /*!50300 c1 */ FROM t1; +SELECT /*!50300 c1 */ FROM t1; +SHOW STATUS LIKE 'Qcache_inserts'; +Variable_name Value +Qcache_inserts 1 +SHOW STATUS LIKE 'Qcache_hits'; +Variable_name Value +Qcache_hits 1 +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +Variable_name Value +Qcache_queries_in_cache 1 +SELECT hits, statement_text FROM information_schema.query_cache_info; +hits statement_text +1 SELECT /*!50300 c1 */ FROM t1 +# +# Case 2: forward MariaDB-only executable comment /*M!NNNNNN ... */ +# +RESET QUERY CACHE; +FLUSH STATUS; +SELECT /*M!50300 c1 */ FROM t1; +SELECT /*M!50300 c1 */ FROM t1; +SHOW STATUS LIKE 'Qcache_inserts'; +Variable_name Value +Qcache_inserts 2 +SHOW STATUS LIKE 'Qcache_hits'; +Variable_name Value +Qcache_hits 2 +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +Variable_name Value +Qcache_queries_in_cache 1 +SELECT hits, statement_text FROM information_schema.query_cache_info; +hits statement_text +1 SELECT /*M!50300 c1 */ FROM t1 +# +# Case 3: reversed executable comment /*!!NNNNNN ... */ +# The lexer rewrites the version digits to spaces, so the query is +# marked not safe to cache and no entry is stored. +# +RESET QUERY CACHE; +FLUSH STATUS; +SELECT /*!!999999 c1 */ FROM t1; +SELECT /*!!999999 c1 */ FROM t1; +SHOW STATUS LIKE 'Qcache_inserts'; +Variable_name Value +Qcache_inserts 2 +SHOW STATUS LIKE 'Qcache_hits'; +Variable_name Value +Qcache_hits 2 +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +Variable_name Value +Qcache_queries_in_cache 0 +SELECT hits, statement_text FROM information_schema.query_cache_info; +hits statement_text +# +# Case 4: reversed MariaDB syntax executable comment /*M!!NNNNNN ... */ +# Same buffer mutation as case 3; not cached. +# +RESET QUERY CACHE; +FLUSH STATUS; +SELECT /*M!!999999 c1 */ FROM t1; +SELECT /*M!!999999 c1 */ FROM t1; +SHOW STATUS LIKE 'Qcache_inserts'; +Variable_name Value +Qcache_inserts 2 +SHOW STATUS LIKE 'Qcache_hits'; +Variable_name Value +Qcache_hits 2 +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +Variable_name Value +Qcache_queries_in_cache 0 +SELECT hits, statement_text FROM information_schema.query_cache_info; +hits statement_text +# +# Case 5: forward executable comment /*!999999 ... */ where the version +# is higher than the server, so the body is SKIPPED. The lexer +# overwrites the leading '!' with a space; not cached. +# +RESET QUERY CACHE; +FLUSH STATUS; +SELECT c1 /*!999999 + 1 */ FROM t1; +SELECT c1 /*!999999 + 1 */ FROM t1; +SHOW STATUS LIKE 'Qcache_inserts'; +Variable_name Value +Qcache_inserts 2 +SHOW STATUS LIKE 'Qcache_hits'; +Variable_name Value +Qcache_hits 2 +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +Variable_name Value +Qcache_queries_in_cache 0 +SELECT hits, statement_text FROM information_schema.query_cache_info; +hits statement_text +# +# Case 6: reversed executable comment /*!!100000 ... */ where the +# version is <= the server, so the body is SKIPPED; not cached. +# +RESET QUERY CACHE; +FLUSH STATUS; +SELECT c1 /*!!100000 + 1 */ FROM t1; +SELECT c1 /*!!100000 + 1 */ FROM t1; +SHOW STATUS LIKE 'Qcache_inserts'; +Variable_name Value +Qcache_inserts 2 +SHOW STATUS LIKE 'Qcache_hits'; +Variable_name Value +Qcache_hits 2 +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +Variable_name Value +Qcache_queries_in_cache 0 +SELECT hits, statement_text FROM information_schema.query_cache_info; +hits statement_text +# +# Case 7: with query_cache_strip_comments=ON, caching remains enabled. +# +SET LOCAL query_cache_strip_comments= ON; +RESET QUERY CACHE; +FLUSH STATUS; +SELECT /*!50300 c1 */ FROM t1; +SELECT /*!50300 c1 */ FROM t1; +SELECT /*!!999999 c1 */ FROM t1; +SELECT /*!!999999 c1 */ FROM t1; +SELECT c1 /*!999999 + 1 */ FROM t1; +SELECT c1 /*!999999 + 1 */ FROM t1; +SELECT c1 /*!!100000 + 1 */ FROM t1; +SELECT c1 /*!!100000 + 1 */ FROM t1; +SHOW STATUS LIKE 'Qcache_inserts'; +Variable_name Value +Qcache_inserts 6 +SHOW STATUS LIKE 'Qcache_hits'; +Variable_name Value +Qcache_hits 6 +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +Variable_name Value +Qcache_queries_in_cache 4 +SELECT hits, statement_text FROM information_schema.query_cache_info; +hits statement_text +1 SELECT /*!!999999 c1 */ FROM t1 +1 SELECT /*!50300 c1 */ FROM t1 +1 SELECT c1 /*!!100000 + 1 */ FROM t1 +1 SELECT c1 /*!999999 + 1 */ FROM t1 +SET LOCAL query_cache_strip_comments= OFF; +DROP TABLE t1; +SET GLOBAL query_cache_type= @save_query_cache_type; +SET GLOBAL query_cache_size= @save_query_cache_size; diff --git a/mysql-test/main/query_cache_executable_comments.test b/mysql-test/main/query_cache_executable_comments.test new file mode 100644 index 0000000000000..af4980af841d5 --- /dev/null +++ b/mysql-test/main/query_cache_executable_comments.test @@ -0,0 +1,152 @@ +--source include/have_query_cache.inc +--source include/not_embedded.inc + +--disable_ps_protocol +--disable_ps2_protocol +--disable_cursor_protocol +--disable_view_protocol + +--echo # +--echo # MDEV-39421: Queries with executable comments gets in query +--echo # cache, but never hits. +--echo # +--echo # The lexer mutates the raw query buffer when: +--echo # (a) a reversed comment /*!!NNNNNN ... */ is taken (digits -> spaces), +--echo # (b) a forward or reversed comment is skipped (the '!' -> space). +--echo # With query_cache_strip_comments=OFF (default), thd->base_query points +--echo # into the same buffer the lexer mutates, so the lookup key (computed +--echo # before parsing) and store key (computed after) disagree and the entry +--echo # is inserted but never hit. +--echo # +--echo # Fix: in those two lexer sites, set lex->safe_to_cache_query=0 so the +--echo # query is not cached in the first place. Caching still occurs when +--echo # query_cache_strip_comments=ON. +--echo # + +SET @save_query_cache_type= @@global.query_cache_type; +SET @save_query_cache_size= @@global.query_cache_size; + +SET GLOBAL query_cache_size= 1024*1024; +SET GLOBAL query_cache_type= ON; +SET LOCAL query_cache_type= ON; + +CREATE TABLE t1 (c1 INT); +INSERT INTO t1 VALUES (1),(2),(3); + +--echo # +--echo # Case 1: forward MySQL-compatible executable comment /*!NNNNNN ... */ +--echo # +RESET QUERY CACHE; +FLUSH STATUS; +--disable_result_log +SELECT /*!50300 c1 */ FROM t1; +SELECT /*!50300 c1 */ FROM t1; +--enable_result_log +SHOW STATUS LIKE 'Qcache_inserts'; +SHOW STATUS LIKE 'Qcache_hits'; +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +SELECT hits, statement_text FROM information_schema.query_cache_info; + +--echo # +--echo # Case 2: forward MariaDB-only executable comment /*M!NNNNNN ... */ +--echo # +RESET QUERY CACHE; +FLUSH STATUS; +--disable_result_log +SELECT /*M!50300 c1 */ FROM t1; +SELECT /*M!50300 c1 */ FROM t1; +--enable_result_log +SHOW STATUS LIKE 'Qcache_inserts'; +SHOW STATUS LIKE 'Qcache_hits'; +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +SELECT hits, statement_text FROM information_schema.query_cache_info; + +--echo # +--echo # Case 3: reversed executable comment /*!!NNNNNN ... */ +--echo # The lexer rewrites the version digits to spaces, so the query is +--echo # marked not safe to cache and no entry is stored. +--echo # +RESET QUERY CACHE; +FLUSH STATUS; +--disable_result_log +SELECT /*!!999999 c1 */ FROM t1; +SELECT /*!!999999 c1 */ FROM t1; +--enable_result_log +SHOW STATUS LIKE 'Qcache_inserts'; +SHOW STATUS LIKE 'Qcache_hits'; +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +SELECT hits, statement_text FROM information_schema.query_cache_info; + +--echo # +--echo # Case 4: reversed MariaDB syntax executable comment /*M!!NNNNNN ... */ +--echo # Same buffer mutation as case 3; not cached. +--echo # +RESET QUERY CACHE; +FLUSH STATUS; +--disable_result_log +SELECT /*M!!999999 c1 */ FROM t1; +SELECT /*M!!999999 c1 */ FROM t1; +--enable_result_log +SHOW STATUS LIKE 'Qcache_inserts'; +SHOW STATUS LIKE 'Qcache_hits'; +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +SELECT hits, statement_text FROM information_schema.query_cache_info; + +--echo # +--echo # Case 5: forward executable comment /*!999999 ... */ where the version +--echo # is higher than the server, so the body is SKIPPED. The lexer +--echo # overwrites the leading '!' with a space; not cached. +--echo # +RESET QUERY CACHE; +FLUSH STATUS; +--disable_result_log +SELECT c1 /*!999999 + 1 */ FROM t1; +SELECT c1 /*!999999 + 1 */ FROM t1; +--enable_result_log +SHOW STATUS LIKE 'Qcache_inserts'; +SHOW STATUS LIKE 'Qcache_hits'; +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +SELECT hits, statement_text FROM information_schema.query_cache_info; + +--echo # +--echo # Case 6: reversed executable comment /*!!100000 ... */ where the +--echo # version is <= the server, so the body is SKIPPED; not cached. +--echo # +RESET QUERY CACHE; +FLUSH STATUS; +--disable_result_log +SELECT c1 /*!!100000 + 1 */ FROM t1; +SELECT c1 /*!!100000 + 1 */ FROM t1; +--enable_result_log +SHOW STATUS LIKE 'Qcache_inserts'; +SHOW STATUS LIKE 'Qcache_hits'; +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +SELECT hits, statement_text FROM information_schema.query_cache_info; + +--echo # +--echo # Case 7: with query_cache_strip_comments=ON, caching remains enabled. +--echo # +SET LOCAL query_cache_strip_comments= ON; +RESET QUERY CACHE; +FLUSH STATUS; +--disable_result_log +SELECT /*!50300 c1 */ FROM t1; +SELECT /*!50300 c1 */ FROM t1; +SELECT /*!!999999 c1 */ FROM t1; +SELECT /*!!999999 c1 */ FROM t1; +SELECT c1 /*!999999 + 1 */ FROM t1; +SELECT c1 /*!999999 + 1 */ FROM t1; +SELECT c1 /*!!100000 + 1 */ FROM t1; +SELECT c1 /*!!100000 + 1 */ FROM t1; +--enable_result_log +SHOW STATUS LIKE 'Qcache_inserts'; +SHOW STATUS LIKE 'Qcache_hits'; +SHOW STATUS LIKE 'Qcache_queries_in_cache'; +--sorted_result +SELECT hits, statement_text FROM information_schema.query_cache_info; +SET LOCAL query_cache_strip_comments= OFF; + +DROP TABLE t1; + +SET GLOBAL query_cache_type= @save_query_cache_type; +SET GLOBAL query_cache_size= @save_query_cache_size; diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index c911fb3e296c7..18f1ca7bb11e6 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -2489,6 +2489,13 @@ int Lex_input_stream::lex_one_token(YYSTYPE *yylval, THD *thd) char *p= (char *) get_ptr(); for (uint i= 0; i < length; i++) p[i]= ' '; + /* + Mark as non-cacheable as we are mutating the buffer unless + strip_comments is enabled since a separate buffer copy is used + by the query cache + */ + if (!thd->variables.query_cache_strip_comments) + lex->safe_to_cache_query= 0; } /* Accept 'M' 'm' 'm' 'd' 'd' */ yySkipn(length); @@ -2517,6 +2524,13 @@ int Lex_input_stream::lex_one_token(YYSTYPE *yylval, THD *thd) char *pcom= yyUnput(' '); if (reversed_comment) *(pcom - 1)= ' '; + /* + Mark as non-cacheable as we are mutating the buffer unless + strip_comments is enabled since a separate buffer copy is used + by the query cache + */ + if (!thd->variables.query_cache_strip_comments) + lex->safe_to_cache_query= 0; comment_closed= ! consume_comment(1); if (! comment_closed) {