Fix query_processor_parser=1 parity bugs (first comment, SET TRANSACTION, KILL, IN lists)
Summary
Fixes the query_processor_parser=1 (ParserSQL mode) bugs found in the v3.0.12 pre-release review (parent #6439). The guiding rule: with *-query_processor_parser=1, routing, interception and multiplexing decisions must match query_processor_parser=0; when ParserSQL's output would change them, fall back to the legacy tokenizer/classifier. The parser=0 path is unchanged in behavior and cost, except for the MySQL SET ... TRANSACTION fixes under "Also in this PR", which apply to both modes.
ParserSQL mode is off by default in every tier, but doc/mysql-user-variable-tracking.md points users at it.
| Issue | Root cause | Fix |
|---|---|---|
| #6419 | first_comment was filled only on the tokenizer fallback path; fully parsed queries silently lost annotations (hostgroup=, query_timeout=, max_lag_ms=, min_gtid=, …), including for prepared statements | On the full-AST path, run the legacy first-comment extractor with the same options (skipped with a single memchr when the query has no /*) |
| #6420 | ParserSQL 1.1.0 digest of SET GLOBAL/LOCAL/SESSION TRANSACTION dropped the scope, so the session matched it as plain SET TRANSACTION, parse2() returned nothing, and the client got OK without the statement being applied or forwarded | Vendor ParserSQL 1.1.1, which keeps the scope; SET ... TRANSACTION uses the ParserSQL digest again (same as parser=0 except keyword case and the space after a list comma). GLOBAL/PERSIST/PERSIST_ONLY and PgSQL SESSION CHARACTERISTICS still don't match the session regex and are forwarded |
| #6421 | ParserSQL has no KILL type → MYSQL_COM_QUERY_UNKNOWN → KILL forwarded to a backend (could kill an unrelated thread) | Statements ParserSQL leaves UNKNOWN fall back to the legacy classifier. Also fixes a per-query query_prefix leak in parser mode (query_digests=0 + commands_stats=1) |
| #6428 | The DIGEST emitter collapses every IN list to IN (?) and keeps only the first VALUES row, hiding LAST_INSERT_ID(), @@IDENTITY, GET_LOCK(), user variables, pg_try_advisory_lock()… from the session/connection checks | If a collapsed list or a later VALUES row contains anything other than literals/placeholders, use the legacy digest; literal-only lists keep the ParserSQL digest |
Also in this PR
- ParserSQL 1.1.1 (release, PR #71): archive byte-identical to
git archive --format=tar.gz --prefix=ParserSQL-1.1.1/ v1.1.1(same method reproduces the 1.1.0 archive), SHA-256 recorded indeps/parsersql/README.md. SET LOCAL TRANSACTION(MySQL): LOCAL is a synonym of SESSION. It is added tomatch_regexes[2](nowMYSQL_SET_TRANSACTION_DIGEST_REGEX) and toparse2(), so it is tracked likeSET SESSION TRANSACTIONinstead of being forwarded with the hostgroup locked. PgSQL is unchanged:PgSQL_Sessionnever uses itsmatch_regexes[2].parse2()partial parses: the digest collapses whitespace butparse2()accepted only spaces, and it stopped at the first characteristic.SET SESSION\nTRANSACTION READ ONLYwas acknowledged with no effect, and... READ COMMITTED, READ ONLYapplied only the isolation level.parse2()now accepts any whitespace and returns nothing if input is left over. The session then forwards the statement (unable_to_parse_set_statement()) instead of replying OK. This applies to both parser modes.set_parser_algorithm=3partial parses:parsersql_parse_set_mysql()/parsersql_parse_set_pgsql()now requirestatus == OK && full_input, like the digest adapter. A prefix-only parse (SET a=1 junk,SET NAMES utf8 junk,SET a=1; SELECT 1) is unparsed. Trailing;and whitespace still parse. The session tracking path already rejected these inputs, so this makes the rule hold for every caller. A multi-statement SET locks the hostgroup before any parser runs, as with algorithms 1/2.
PgSQL parser-mode classification of pg_cancel_backend/pg_terminate_backend (#6422) is fixed in the separate PostgreSQL PR.
Note for release notes
With parser=1, statements whose IN lists / VALUES rows contain non-literals now get the parser=0 digest text and hash (e.g. PgSQL IN ($1::int,$2::int) changes from IN (?) to IN ($1,$2)). Query rules matching on parser=1 digests of such statements, and digest stats continuity, are affected.
ParserSQL 1.1.1 also changes parser=1 digests: SET [GLOBAL|SESSION|LOCAL|PERSIST|PERSIST_ONLY] TRANSACTION ... keeps its scope and is uppercase/whitespace-normalized (1.1.0: SET TRANSACTION ... for every scope), PgSQL SET ROLE / SET SESSION AUTHORIZATION / SET CONSTRAINTS are emitted in full (1.1.0: SET ), and multi-value search_path keeps all values.
In both parser modes, SET LOCAL TRANSACTION is now tracked like SET SESSION TRANSACTION (no hostgroup lock), and a SET [SESSION] TRANSACTION that parse2() cannot parse in full (several characteristics) is forwarded with the hostgroup locked instead of being acknowledged with only the first one applied. Tabs or newlines inside the statement no longer produce an OK with no effect.
Not in this PR (tracked in the issues)
- Upstream ParserSQL IN/VALUES collapsing (the
emit_set_transactionfix is in 1.1.1, vendored here). doc/mysql-user-variable-tracking.mdrecommendingmysql-set_parser_algorithm=3.
Refs #6420, #6428. Closes #6419, closes #6421. Part of #6439.
Tests
- New unit tests (unit-tests-g1):
parsersql_first_comment_unit-t(103),parsersql_set_transaction_digest_unit-t(140: every scope form in both dialects, regex outcome on ParserSQL vs legacy digest,parse2()LOCAL/whitespace/partial, PgSQL ROLE/AUTHORIZATION/CONSTRAINTS/search_path digests),parsersql_command_type_unit-t(76),parsersql_in_values_digest_unit-t(74). - New TAP tests comparing
parser=0andparser=1end to end:reg_test_6419_parsersql_first_comment-t,pgsql-reg_test_6419_parsersql_first_comment-t,reg_test_6420_parsersql_set_transaction-t,reg_test_6421_parsersql_kill-t,reg_test_6428_parsersql_in_values-t(legacy-g1, mysql84-g1). reg_test_6420_parsersql_set_transaction-talso checks thatSET LOCAL TRANSACTIONis tracked without a hostgroup lock and applied, and thatSET SESSION TRANSACTION ... READ COMMITTED, READ ONLYapplies both characteristics.parsersql_unit-t(248) covers partial SET parses withset_parser_algorithm=3.- Each new test was confirmed to fail without its fix.
- Local runs: unit-tests-g1 194/194; all new and existing parser-mode TAP tests pass in legacy-g1, mysql84-g1, legacy-g4, mysql84-g4, set_parser_algorithm_3-g1. Debug (PROXYSQL40) and Stable-tier release builds compile cleanly.
- ParserSQL 1.1.1 follow-up, local runs:
parsersql_*_unit-t,query_processor_unit-t,pgsql_query_processor_unit-t,digest_stats_unit-t,setparser_parsersql_test(517),parsersql_digest_test(63),setparser_test2/3all pass;reg_test_6420_parsersql_set_transaction-tandreg_test_6397-set_local-tpass in mysql84-g1 and legacy-g1 (20/20 and 261/261 each). The fail-without-fix check was not repeated for the 1.1.1 follow-up assertions.
Summary by cubic
Fixes parity bugs in ParserSQL mode (*/query_processor_parser=1), where routing, interception, and multiplexing decisions diverged from legacy mode. Whenever ParserSQL's output would change a decision, the code now falls back to the legacy tokenizer/classifier; the parser=0 path is unchanged in behavior and cost.
- Fully parsed queries now extract first-comment annotations (
hostgroup=,query_timeout=,min_gtid=, …), which they previously lost (#6419). - Vendored ParserSQL 1.1.1 keeps the scope of
SET ... TRANSACTION, soSET GLOBAL/LOCAL TRANSACTIONno longer matches the session-level handler that replied OK without applying the statement (#6420).SET LOCAL TRANSACTIONis now tracked likeSET SESSION TRANSACTION, and aSET SESSION TRANSACTIONthatparse2()cannot parse in full is forwarded with the hostgroup locked instead of acknowledged with only part applied. - Statements ParserSQL classifies as UNKNOWN now fall back to the legacy classifier, so KILL is intercepted instead of forwarded to a backend that could kill an unrelated thread (#6421). This also fixes a per-query
query_prefixleak. - Statements whose IN lists or later VALUES rows contain non-literals now fall back to the legacy digest, restoring detection of
LAST_INSERT_ID(),@@IDENTITY, andGET_LOCK()for routing and multiplexing (#6428). - A SET statement that ParserSQL parses only in part (e.g.
SET a=1 junk) is treated as unparsed, so the session forwards it instead of tracking the prefix.
Note. With parser=1, the digest text and hash of those IN/VALUES statements now match parser=0, so query rules matching on parser=1 digests and digest-stats continuity are affected; ParserSQL 1.1.1 also normalizes SET ... TRANSACTION digests (scope kept, uppercase, whitespace-normalized). The new tests' admin fixtures qualify temp-table drops with temp. so they recover cleanly from interrupted runs.
Closes #6419 and #6421; refs #6420 and #6428.
Summary by CodeRabbit
- Bug Fixes
- Improved SQL command classification consistency across parser modes, including for
KILLstatements. - Improved query digest handling for transaction-setting statements, non-literal expressions in
INlists and multi-rowVALUESclauses, and leading SQL comments. - Preserved first-comment routing annotations across MySQL and PostgreSQL parser modes.
- Improved consistency for transaction settings, query interruption, and connection termination across parser modes.
- Incomplete or malformed
SETstatements are no longer treated as successfully parsed; they are forwarded for handling.
- Improved SQL command classification consistency across parser modes, including for