12 Commits (v3.0_fix_codecov-coverage-path-prefix)

Author	SHA1	Message	Date
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	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	1621ea2e2c	fix(pgsql): preserve outer quotes on PG SET values in the walker ... walk_set_stmt unconditionally called finalize_var_value (which calls strip_quotes) on every value, so `SET search_path TO '$user, public'` arrived at the session handler as `$user, public` -- outer single quotes already stripped. The handler then asked the validator to treat the whole thing as a comma-separated identifier list and the unquoted `$user` token was rejected with invalid value for parameter "search_path": "$user, public" For PostgreSQL, this breaks the PGTRACKED_VAR_OPT_NO_STRIP_VALUE contract: variables with that flag (currently only search_path) are supposed to retain their quoting because `"name"` vs `name` vs `'string'` are semantically distinct in PG. Variables that DO want stripping already route through PgSQL_Session.cpp's unquote_if_quoted() path, so their behavior is unchanged. MySQL still uses finalize_var_value -- single-value semantics, no need to preserve quotes through the walker. Fixes the 3 cases in test/tap/tests/pgsql-set_statement_test-t.cpp under set_parser_algorithm=3: SET search_path TO '\$user, public' (case 26) SET search_path TO 'public, \$user' (case 27) SET search_path TO 'public, \$user, pg_catalog' (case 28) These had silently passed before the CI infra fix in PR #5760 because the set_parser_algorithm_3-g1 pre-proxysql.sql hook was not being executed and proxysql ran with the default SET parser.	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	5d6ba67d97	fix: add NOSONAR annotations for SonarCloud security hotspots ... - S5813 (strlen safety): all 3 instances use string literal prefixes or null-terminated array entries where strlen is compile-time evaluated or trivially safe - S7635/S7637 (GitHub Actions): branch ref and secrets:inherit match the pattern used by all other caller workflows in the repo	2 months ago
Rene Cannao	a1d3156adc	fix: resolve SonarCloud S134 nesting issue and setparser test expectations ... - Extract extract_names_values() and extract_charset_values() from walk_set_stmt() to reduce nesting depth (S134 at line 416) - Fix setparser_test2-t and setparser_test3-t expectations: the regex parser only recognizes CONCAT/REPLACE/IFNULL, so CONCA (typo), mismatched brackets, and SELCT all return empty results. Update the test data to expect empty maps instead of parsed values.	2 months ago
Rene Cannao	0f96ae1fe7	fix: resolve all 12 SonarCloud issues (S3776, S134, S1231, M23_090) ... - Extract skip_quoted_char() from extract_paren_expr() to reduce cognitive complexity (S3776, S134) in Query_Processor_ParserSQL.cpp - Extract resolve_var_value() and finalize_var_value() from walk_set_stmt() to reduce complexity from 39 to under 25 - Extract qp_digest_parsersql() and qp_digest_legacy() from query_parser_init() to reduce complexity from 30 to under 25 - Add NOSONAR annotations for C-style casts on string literals that follow existing codebase patterns (M23_090) - Add NOSONAR annotations for free() calls on C-allocated memory in test code (S1231)	2 months ago
Rene Cannao	deb1c46173	fix: address code review findings C1-C3, I1, I3 ... C1: NODE_SET_CHARSET map key corrected to character_set_results C2: query_processor_parser==1 now also routes SET through ParserSQL C3: qp->digest initialized to 0 on parse failure I1: header docs corrected (tokenizer-based, not libinjection) I3: defensive NODE_SET_TRANSACTION case added to SET walker	2 months ago
Rene Cannao	889c319420	test: add SET parser and digest adapter validation tests	2 months ago
Rene Cannao	62491629bb	docs: add documentation to ParserSQL adapter layer	2 months ago
Rene Cannao	d6e559a2ed	fix: add proxysql.h include and register adapter in Makefile OBJ list	2 months ago
Rene Cannao	90b1897764	feat: implement ParserSQL adapter layer	2 months ago