12 Commits (feature/pgsql-native-backend-protocol)

Author	SHA1	Message	Date
Rene Cannao	de37c64f5e	chore(sonar): tighten NOSONAR placement for remaining hotspots ... Two SonarCloud hotspots from the previous round didn't actually pick up the inline annotations: - lib/DNS_Cache.cpp: the NOSONAR comment was on the line above the `std::mt19937 gen(...)` construction. Sonar attributes the hotspot to the construction line itself, so the comment needs to be inline on that line. Moved it. Kept the explanatory comment block above. - test/tap/tests/setparser_parsersql_test.cpp: new hotspot on the strlen() call in the inline_validate_search_path helper added in commit 950f6415b. Test code, caller-supplied null-terminated C string, and the SIZE_MAX guard on the very next line bounds the result. Annotated inline. No behaviour change; only Sonar annotation cleanups. Should clear the last two TO_REVIEW hotspots on PR #5809.	1 month ago
Rene Cannao	d7976aca13	fix(set-walker): PG walker gates on full-input AST coverage (not just status) ... Pair-fix for the previous don't-lock change. ParserSQL returns ParseResult::OK even when its AST only covers a prefix of the input and silently drops the trailing junk -- for example `SET search_path = public,,schema1` returns OK with an AST covering `SET search_path = public`, and the `,,schema1` tail is lost. With the new no-lock path that landed in 35f84c384, the session was then handing `[public]` to the validator, the validator was happily accepting `public` as a valid single-element search_path, proxysql was tracking the SET as successful, and syncing `SET search_path TO public` to the backend -- accepting SQL that PG itself would have rejected for the original `public,,schema1` input. The same lossiness affected several other negative cases: - `SET search_path = "$user", "$invalid"@schema` (the `@schema` was lost) - `SET search_path = "schema1" "schema2"` (the second ident was lost) - `SET search_path = "\"unterminated"` - `SET search_path = "valid",, "invalid"` Fix: in parsersql_parse_set_pgsql, require the AST to cover every non-cosmetic byte of the input before letting the walker run. The new ast_covers_full_input() helper finds the rightmost byte any AST descendant touches, then skips trailing whitespace, a single optional trailing comma (which the regex parser strips and #171 `SET search_path TO "$user" ,` relies on), a single optional semicolon, and embedded NUL bytes (QueryPointer occasionally includes one). If anything else remains, the parse is treated as failed and the walker returns an empty map -- which the don't-lock path then forwards to PG for the authoritative reject. The right-most byte computation accounts for delimited-identifier and string-literal nodes whose value_ptr/value_len cover only the content -- their closing delimiter byte is treated as part of the coverage when the opening delimiter matches. MySQL is unchanged (its walker continues to accept PARTIAL parses for the legitimate `(SELECT ...)` RHS shapes the existing test fixtures rely on). Locally pgsql-set_parameter_validation_test-t is now down from 4 sub-failures (post-742c049) to 1 (case #150, the single-quoted-string whitespace-collapse issue, which is a separate PG-side rendering question unrelated to the walker / validator path). Tests: 8 new cases in setparser_parsersql_test.cpp TestPartialAstGate covering 4 legitimate cases that must stay non-empty and 4 malformed cases that must now return empty. All pass.	1 month ago
Rene Cannao	950f6415b5	test+debug(set_param_validation #184): chain test + temp validator logging ... Investigation of pgsql-set_parameter_validation_test-t case #184 (64-char delimited identifier > 63-char PG limit) has been blocked by divergence between static analysis and observed CI behaviour: - Static analysis: walker emits 66-char `"<64 digits>"` string, the search_path validator's quoted-branch counts effective_len=64, triggers `> 63` reject, returns false. Session error path generates an error packet to the client. SET should fail. - Observed in CI: no `[ERROR] invalid value for parameter "search_path"` log line at the time of case #184, despite the same log capturing every other validator-reject (client_encoding, DateStyle, IntervalStyle, etc.). Test reports `actual_success=true` (SET unexpectedly succeeded). The `"Value changed unexpectedly from '"$user"' to ''"` message is the test framework's artefact when test_ok flips before the value-comparison read. To pin where the disconnect actually is: * setparser_parsersql_test.cpp adds an inline copy of pgsql_variable_validate_search_path plus 3 strict cases plus a walker -> session-join -> validator chain test exercising the exact 64-char input. All 5 new assertions pass locally, confirming walker + validator are individually correct (the validator does reject 64 chars; walker does emit 66-char output). Chain test produces what production should be feeding the validator. * PgSQL_Session.cpp adds two `proxy_info` lines around the validator call in the SET-tracking block (pre and post). These will print value1 + validator return to the CI proxysql.log so we can see what proxysql actually feeds the validator for #184 and what it gets back. To be reverted once #184 is fixed.	1 month ago
Rene Cannao	742c049959	deps(parsersql): bump to 1.0.8; strict tests for parse-fail cases ... ParserSQL 1.0.8 (PR ProxySQL/ParserSQL#44, tag v1.0.8) fixes scan_double_quoted_identifier and scan_backtick_identifier so an unclosed delimiter emits TK_ERROR instead of silently consuming everything to EOF as one identifier. Closes the case-#172 cascade in pgsql-set_parameter_validation_test-t under set_parser_algorithm=3: `SET search_path = "unclosed_quote, public` now returns an empty walker map (parse failed) so the session falls through to unable_to_parse_set_statement() and the SET is forwarded to PG as-is. PG rejects with a syntax error, search_path stays unchanged, and the test sees the expected unchanged value (which in turn unblocks the cascading case-#184). Tests: added TestEmptyOnParseFail in setparser_parsersql_test.cpp with byte-exact assertions that the walker now returns an empty map for both the PG unclosed-`"` and MySQL unclosed-`` ` `` shapes (294/294 local). SHA256 parsersql-1.0.8.tar.gz: 9bdaf58c207a556c117f2322c3f532e367e631459e158e30b5139604818ef8e1	1 month ago
Rene Cannao	d629aa001b	deps(parsersql): bump to 1.0.7; test/reg_4072: 9us -> 8us ... ParserSQL 1.0.7 (PR ProxySQL/ParserSQL#43, tag v1.0.7) allows `$` as a PostgreSQL identifier continuation char, per the PG lexical-syntax spec. Before this, `SET search_path = schema$1` truncated to `schema` and the trailing `$1` fell through as a placeholder, leaving case 169 of pgsql-set_parameter_validation_test-t failing under set_parser_algorithm=3 even after the walker-side fixes in commit b54aaa5f7. End-to-end validated locally: setparser_parsersql_test-t now passes its byte-exact regression cases for `SET search_path = schema$1` and `SET search_path = my$schema$2_name` (292/292 total). reg_test_4072-show-warnings-t: drop the slow-consumer sleep from 9us to 8us. The 9us margin re-flaked across three groups (legacy-g2, legacy-g2-genai, mysql84-g2) on the prior CI run. 8us shaves ~20% off the sleep floor (~16s vs ~20s at 10us) while keeping enough back-pressure to still exercise the original #4072 reproducer. Doc comment updated with the new ratio and the noted history of the 9us attempt. SHA256 parsersql-1.0.7.tar.gz: c4029f6bf0a1774ecbcb95eb842fe6aa682a8ba36ec2badf49241d1ff61c5608	1 month ago
Rene Cannao	b54aaa5f76	fix(set-walker): preserve raw source for function calls and delimited idents ... The ParserSQL-backed SET walker (algorithm 3) was round-tripping RHS values through the Emitter, which normalises whitespace (adds ", " between function call args) and drops delimiter quotes on identifiers (NODE_IDENTIFIER emits its content without re-quoting). Both behaviours diverged from the regex-based parsers used in algorithms 0-2. The function-call drift cascaded across set_testing-t because the wrong `sql_mode = concat(@@sql_mode, ',X')` value (with extra space) persisted on the connection and tripped every subsequent variable comparison until reset. The delimited-ident drift caused pgsql-set_parameter_validation_test-t to treat `"MixedCase"` as `mixedcase` and `"$user"` as the current-user substitution token (rather than the literal identifier the user wrote). Walker fixes: - NODE_FUNCTION_CALL -> paren-match in original query buffer from value_ptr, return the verbatim substring. - NODE_IDENTIFIER/NODE_COLUMN_REF with FLAG_IDENT_DELIMITED -> splice the surrounding quote chars back in from value_ptr-1 / value_ptr+value_len. Both helpers are no-ops when value_ptr is outside the query buffer (defensive fallback to the existing emit_node_text path). Tests: adds two strict (byte-exact) regression suites in setparser_parsersql_test.cpp -- the existing normalize_value() collapses whitespace and quote-style differences, which is what hid the bugs in the first place. The new TestStrictFunctionCall and TestStrictPgsqlIdent helpers compare verbatim so any future re-introduction is caught. Local: 288/288 unit tests pass.	1 month ago
Rene Cannao	6f1e9c4ac2	deps(parsersql): bump to 1.0.6; add README; adapt setparser test ... ParserSQL 1.0.6 ships ten fixes from the full set_parser_algorithm=3 audit across three upstream PRs: From PR #39 (initial six -- "P1..P6"): * P1 FLAG_IDENT_DELIMITED on NODE_IDENTIFIER / NODE_COLUMN_REF when the source token was backtick / double-quote delimited. * P2 bare PG `$word` -> ParseResult::ERROR (was PARTIAL with null AST). * P3 `SET SCHEMA name` (PG) emitted as VAR_ASSIGNMENT(search_path, value), per the documented equivalence. * P4 `SET SEED N` (PG) emitted as VAR_ASSIGNMENT(seed, value) (lower-cased canonical name). * P6 `SET @@\`var\``, `SET @\`var\``, `SET @@\`scope\`.\`var\`` -- full canonical names emitted (backticks stripped, no truncation of the closing delimiter). From PR #40 (post-audit sweep): * PG non-GUC SET forms recognized as new node types instead of being mis-classified as unknown GUC assignments: NODE_SET_ROLE (SET [LOCAL] ROLE { name|'name'|NONE|DEFAULT }); NODE_SET_SESSION_AUTHORIZATION (SET SESSION AUTHORIZATION ...); NODE_SET_CONSTRAINTS (SET CONSTRAINTS {ALL|name[,..]} {DEFERRED|IMMEDIATE}). SET SESSION CHARACTERISTICS AS TRANSACTION ... emits the same NODE_SET_TRANSACTION shape as plain SET SESSION TRANSACTION. * Schema-qualified PG GUC names (SET pg_catalog.search_path TO ..., SET myapp.setting = ...) -- combined into one canonical <schema>.<name> IDENTIFIER (was PARTIAL). * SET TIME ZONE INTERVAL '1' HOUR -- full literal source span preserved as LITERAL_STRING (was dropping value + unit). * Clearly-malformed SET inputs (SET=1, SET x=;, SET x=, SET x=,foo, SET search_path TO, bare SET, SET GLOBAL) now return ParseResult::ERROR (were PARTIAL with null AST). From PR #42 (hot-fix on top of #40): * parse_set() PARTIAL -> ERROR downgrade only fires when no assignment was parsed at all. Multi-assignment SETs that contain one malformed element alongside well-formed ones (e.g. `SET sql_mode='X', whatever=, autocommit=1`) keep their successful assignments in the AST and surface as PARTIAL -- the consumer can use the well-formed entries. The tarball is `git archive --format=tar.gz --prefix=ParserSQL-1.0.6/ v1.0.6` against ProxySQL/ParserSQL @ v1.0.6 (sha256 2b94e1512c22955875a5b2501ec868ac8fee48a03ca8c6b496324c5f7540aa67). Also: * deps/parsersql/README.md documents the upstream repo (https://github.com/ProxySQL/ParserSQL), the bump procedure, the "fix upstream, don't work around in ProxySQL" policy with a rubric, and the v1.0.3 / 1.0.4 / 1.0.5 / 1.0.6 audit-history with PR links. * test/tap/tests/setparser_parsersql_test.cpp updated for two consequences of the parser improvements: - normalize_value() now strips a single layer of matching outer quotes (', ", `) before comparison. The walker now preserves outer quoting for PostgreSQL values that have NO_STRIP_VALUE semantics (search_path: `"$user"`, `'public,,schema1'`), so the regex parser's stripped output and the ParserSQL adapter's quoted output are semantically equivalent. - TestParse() / TestParsePgsql() accept "parsersql produced output that the regex parser cannot" as a strict improvement when the fixture file's `Expected{}` is empty. The fixture is shared with setparser_test / setparser_test2 / setparser_test3 which exercise the regex parser, so the `{}` entries document regex-parser limits, not correctness contracts. ParserSQL is allowed to be better. Validation gate before this commit (per the lesson from the v1.0.4/1.0.5 churn): every fix landed in ParserSQL only after the ProxySQL consumer tests were green against the local build. ParserSQL `make test` 1254/1254 ProxySQL `make debug` clean link ProxySQL `make build_tap_test_debug` 344/344 build ProxySQL unit tests (test/tap/tests/unit/) 43/43 ProxySQL setparser_test, _test2, _test3 1 + 226 + 224 ProxySQL setparser_parsersql_test (integration) 270/270	1 month ago
Rene Cannao	4eb52abf2c	fix(pgsql,test): PG multi-value SET dispatch + test-fixture cleanup ... Follow-up to the cluster-2 ParserSQL 1.0.3 bump (this same PR), addressing review feedback from coderabbit + gemini. ## 1. PgSQL_Session: accept multi-value SET vectors (coderabbit, Major) The previous commit `5a48004af` taught the adapter to walk every RHS sibling so PG multi-value lists (e.g. `SET search_path TO "$user", public`) come through as a `std::vector<std::string>` of every value. But `lib/PgSQL_Session.cpp:4516` still had `size() > 2` → `unable_to_parse_set_statement()`, which would have caused 3+ value PG SETs to lock the hostgroup instead of being processed. Worse, the downstream value-consumer at the same site used only `it->second.front()`, silently dropping every value past the first for valid 2-value SETs too. Fix: * Drop the upper bound from the cardinality check (keep `< 1` so empty value lists still fail loudly). * Join every value in the vector into `value1` with the comma-separated form PG itself uses for these parameters (`a, b, c`). Single-value SETs hit the loop with `size()==1` and take just `it->second.front()`, preserving the previous behaviour. The validator / hash-compare / tracker / param_status code below the join is unchanged — it now receives the full joined value instead of the truncated first element. ## 2. setparser_parsersql_test: drop INTERVAL test that locked in incomplete behaviour (subagent correctness review, Major) `parsersql_pgsql_time_zone` previously asserted that `SET TIME ZONE INTERVAL '7' HOUR` produces `timezone = "INTERVAL"`, which is the *current* observable output of ParserSQL — the expression parser does not yet consume the `'7' HOUR` modifier chain. Encoding that as expected output would flip the test red when ParserSQL is later improved to capture the full interval expression, conflating an improvement with a regression. Removed the case; left a comment in the fixture explaining the gap so a future contributor adds the case back together with the ExpressionParser fix. ## 3. Diag-output cleanup (gemini, cosmetic) * `%lu` → `%zu` for `size_t` (portability — `size_t` is not always `unsigned long` on 32-bit / Windows). * Extract the per-vector " | "-separated diag-string builder into a single `join_values_for_diag()` helper used by both `TestParse` (now also dumps full vector on mismatch, where it previously printed just `vals[0]`) and `TestParsePgsql`. ## Verification * `./setparser_parsersql_test`: 260/270 ok; the 10 remaining not-ok are unchanged pre-existing failures (`sql_mode 23-25`, `Set1_v2 0-1`), nothing new from this commit. * `pgsql_time_zone` group: 6 assertions (3 cases × 2) — confirms the INTERVAL case is removed cleanly. * `pgsql_search_path` group: 16 assertions still all pass — the multi-value join in PgSQL_Session does not affect the parser- side test (which goes through the adapter, not the session).	1 month ago
Rene Cannao	5a48004af5	fix(deps): bump ParserSQL to v1.0.3 — PG SET TIME ZONE alias + multi-value list ... ParserSQL v1.0.2's PG SET parser had two bugs that were hidden in CI until PR #5760 fixed `test/infra/control/ensure-infras.bash` to actually dispatch `pre-proxysql.sql` hooks (previously only `.bash` hooks ran). With the harness fix in place, `set_parser_algorithm_3-g1` for the first time ran its tests under `pgsql-set_parser_algorithm=3`, surfacing: * `pgsql-set_parameter_validation_test-t` — `SET search_path TO "$user", public` parsed as `search_path = "$user"`, dropping every value after the first comma. * `pgsql-set_statement_test-t` — `SET TIME ZONE 'UTC'` parsed as `time = ZONE`, dropping the actual timezone value entirely. ## Upstream fix ParserSQL v1.0.3 (ProxySQL/ParserSQL PR #38) is the upstream parser fix: * PG `SET TIME ZONE <value>` is now recognised as the PG alias for `SET TimeZone = <value>`. The two-word keyword is detected via case-insensitive identifier lookahead since the tokenizer has no dedicated TK_TIME / TK_ZONE keyword. * PG `parse()` no longer shares MySQL's comma loop. In PG, the comma after the first value is value-list continuation — each subsequent expression is appended as another child of the same NODE_VAR_ASSIGNMENT node, alongside the first RHS. Full upstream test suite: 1223/1223 pass; 0 regressions. ## ProxySQL-side changes * `deps/parsersql/parsersql-1.0.3.tar.gz` — new tarball, replaces 1.0.2. * `deps/parsersql/parsersql` symlink retargeted to `parsersql-1.0.3`. * `lib/Query_Processor_ParserSQL.cpp::walk_set_stmt` — the PG multi-value fix means a single NODE_VAR_ASSIGNMENT can have multiple RHS siblings. The adapter now iterates `target->next_sibling` chain instead of reading a single sibling, so the `map<string, vector<string>>` for `SET search_path TO 'a', 'b', 'c'` carries `["a", "b", "c"]` rather than `["a"]`. Fully backward-compatible for MySQL (always one RHS). * `test/tap/tests/setparser_parsersql_test.cpp` — adds four permanent regression-test groups exercising the library + adapter end-to-end without a live backend: * `mysql_filtered_set` (7 cases) — verifies the filtered-SET variables parse cleanly so the downstream filter logic gets the input it expects. * `mysql_set_testing` (4 cases) — multi-variable comma-separated SET samples from `set_testing-240.csv`. * `pgsql_search_path` (8 cases) — single-value and multi-value shapes from `pgsql-set_parameter_validation_test-t`. * `pgsql_time_zone` (4 cases) — every `SET TIME ZONE ...` shape exercised by `pgsql-set_statement_test-t`. The 4 new assertions for the `INTERVAL '7' HOUR` case capture today's actual ParserSQL output (`timezone = [INTERVAL]`) — the expression parser does not yet consume the full interval modifier chain. The test reflects observable behaviour so it stays green; tightening to capture the full interval value is a future ExpressionParser improvement. ## Test plan - [x] Local rebuild of ProxySQL from the new 1.0.3 tarball: clean. - [x] `./setparser_parsersql_test`: 46 new assertions, all pass. - [x] No regressions in the 256 pre-existing assertions. The same 10 pre-existing failures (sql_mode 23-25, Set1_v2 0-1) — unrelated to this work, present before the upgrade. - [ ] CI on this branch: TAP groups `legacy-g2`, `mysql84-g2`, and `set_parser_algorithm_3-g1` are expected to flip green for the ParserSQL-related failures. Two non-ParserSQL bugs on the algorithm=3 ProxySQL session-dispatch path (`test_filtered_set_statements-t`, `set_testing-t`) remain to be investigated separately — the adapter now returns the correct maps, but downstream dispatch still misbehaves. ## Dependency Should not be merged before ProxySQL/ParserSQL PR #38 is merged and a v1.0.3 tag is cut. The vendored tarball in this commit was built from that PR's branch head; once the upstream tag exists the tarball SHA should be identical to a fresh `git archive --prefix=ParserSQL-1.0.3/ v1.0.3 | gzip` from the upstream tag.	1 month ago
Rene Cannao	1d1e117ffc	fix: restore setparser_test_common.h to original and relocate parsersql_syntax_errors[] ... The shared header setparser_test_common.h must not contain ParserSQL-specific test expectations. syntax_errors[] and Set1_v2[] entries are reverted to their original regex-parser expectations (empty results for invalid SQL). The parsersql_syntax_errors[] array is moved into setparser_parsersql_test.cpp where ParserSQL-specific data belongs.	2 months ago
Rene Cannao	ee3d3e1642	fix: add parsersql-specific syntax-error test expectations ... - Add parsersql_syntax_errors[] array with correct ParserSQL expectations: CONCA, mismatched brackets, and SELCT are all extracted structurally by ParserSQL (unlike the regex parser) - setparser_parsersql_test-t now uses parsersql_syntax_errors instead of the generic syntax_errors array - setparser_test2-t and setparser_test3-t keep using syntax_errors with empty expectations for invalid SQL (regex parser behavior)	2 months ago
Rene Cannao	889c319420	test: add SET parser and digest adapter validation tests	2 months ago