Fix query_processor_parser=1 parity bugs (first comment, SET TRANSACTION, KILL, IN lists)

Merged
RE
17d ago
+2328-58

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.

IssueRoot causeFix
#6419first_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 statementsOn the full-AST path, run the legacy first-comment extractor with the same options (skipped with a single memchr when the query has no /*)
#6420ParserSQL 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 forwardedVendor 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
#6421ParserSQL 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)
#6428The 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 checksIf 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 in deps/parsersql/README.md.
  • SET LOCAL TRANSACTION (MySQL): LOCAL is a synonym of SESSION. It is added to match_regexes[2] (now MYSQL_SET_TRANSACTION_DIGEST_REGEX) and to parse2(), so it is tracked like SET SESSION TRANSACTION instead of being forwarded with the hostgroup locked. PgSQL is unchanged: PgSQL_Session never uses its match_regexes[2].
  • parse2() partial parses: the digest collapses whitespace but parse2() accepted only spaces, and it stopped at the first characteristic. SET SESSION\nTRANSACTION READ ONLY was acknowledged with no effect, and ... READ COMMITTED, READ ONLY applied 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=3 partial parses: parsersql_parse_set_mysql() / parsersql_parse_set_pgsql() now require status == 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_transaction fix is in 1.1.1, vendored here).
  • doc/mysql-user-variable-tracking.md recommending mysql-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=0 and parser=1 end 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-t also checks that SET LOCAL TRANSACTION is tracked without a hostgroup lock and applied, and that SET SESSION TRANSACTION ... READ COMMITTED, READ ONLY applies both characteristics. parsersql_unit-t (248) covers partial SET parses with set_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/3 all pass; reg_test_6420_parsersql_set_transaction-t and reg_test_6397-set_local-t pass 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, so SET GLOBAL/LOCAL TRANSACTION no longer matches the session-level handler that replied OK without applying the statement (#6420). SET LOCAL TRANSACTION is now tracked like SET SESSION TRANSACTION, and a SET SESSION TRANSACTION that parse2() 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_prefix leak.
  • 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, and GET_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 KILL statements.
    • Improved query digest handling for transaction-setting statements, non-literal expressions in IN lists and multi-row VALUES clauses, 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 SET statements are no longer treated as successfully parsed; they are forwarded for handling.