proxysql

Commit Graph

Author	SHA1	Message	Date
René Cannaò	741ce1e36f	fix(pgsql): native query error must not be misclassified as broken connection is_connection_in_reusable_state() called PQtransactionStatus(pgsql_conn) directly; in native mode pgsql_conn is NULL so libpq returns PQTRANS_UNKNOWN, making the session treat a normal backend query error (ErrorResponse + ReadyForQuery, the connection is still idle/reusable) as a broken connection and retry instead of forwarding the error (with its SQLSTATE) to the client. Derive the transaction status from the natively-tracked ReadyForQuery byte in native mode.	2 weeks ago
René Cannaò	21600ac7cf	fix(pgsql): native Query message must have exactly one NUL terminator The native simple-query path appended a NUL to query.length bytes, but the client-query callers (async_query with pgsql_real_query.QuerySize) pass a length that already includes the trailing NUL, producing a malformed double-NUL Query body. PostgreSQL rejects it with 08P01 'invalid message format', breaking the backend connection. Normalize to the SQL up to the first NUL (bounded by query.length) plus a single terminator, matching PQsendQuery semantics. The strlen()-based callers (async_send_simple_command/init_connect) are unaffected.	2 weeks ago
René Cannaò	6e344f04e5	feat(pgsql): native simple-query + simple-command execution (stream-through results) [Task 1.6c / Phase 2 core]	2 weeks ago
René Cannaò	f953a3f137	feat(pgsql): native backend TLS (SSLRequest + client handshake + cert verification) [Task 1.6b]	2 weeks ago
René Cannaò	b027c8c21d	fix(pgsql): native connect entry guards (handler first-call flag, async_connect assert) + timeout teardown [Task 1.6a]	2 weeks ago
René Cannaò	b7bedf06f2	feat(pgsql): native PostgreSQL connect + auth (trust/md5/SCRAM, no TLS) [Task 1.6a]	2 weeks ago
René Cannaò	4e90372ef4	feat(pgsql): native SCRAM-SHA-256 client exchange over libscram with proof-correctness test	2 weeks ago
René Cannaò	ad31ded2fb	feat(pgsql): native md5 password auth builder with known-vector unit test	2 weeks ago
René Cannaò	6ed54c51bf	feat(pgsql): native startup + SSLRequest encoders with unit tests	2 weeks ago
René Cannaò	6874b5a9a2	fix(pgsql): harden backend framer (msg-length cap, realloc/overflow checks, noncopyable)	2 weeks ago
René Cannaò	544799257c	feat(pgsql): native backend message framer with partial-message handling + unit tests Add PgSQL_Backend_Msg_Framer: a pure wire-message framer for the native PostgreSQL backend protocol. It accepts fed bytes (possibly partial) and yields complete messages (type byte + 4-byte big-endian length-prefixed body), signaling FRAME_NEED_MORE on incomplete trailing bytes and FRAME_ERROR on a malformed length. Header stays light (cstdint/cstddef only). Destructor frees the realloc'd buffer to avoid a leak on long-lived connections. Wire it into libproxysql.a via _OBJ_CXX in lib/Makefile. Also fix a pre-existing duplicate vec.o on the unit-test link line: the test/tap/tests/unit/Makefile appended SQLITE3_LDIR/vec.o to STATIC_LIBS in two separate PROXYSQL40 blocks, producing ~98 duplicate-symbol errors that broke every unit test under PROXYSQL40. Keep the single append after the autodetection block.	2 weeks ago
René Cannaò	e5e99e3898	feat(pgsql): native_mode dispatch skeleton with libpq fallback (Phase 0 inert)	2 weeks ago
René Cannaò	d539351a0f	feat(pgsql): add native_mode + backend protocol member to PgSQL_Connection	2 weeks ago
René Cannaò	eef410e5ed	feat(pgsql): add pgsql-use_native_backend_protocol runtime variable (default off)	2 weeks ago
Rahim Kanji	eed52db897	Merge branch 'v3.0' into v3.0_partition-fairness	1 month ago
Rahim Kanji	6909011b67	perf(partition): promote longest-waiting B session via max_connect_time fold Under sustained contention ProcessAllSessions_Partition leaves the B (CONNECTING_SERVER) band unordered, so the pass can serve newer waiters ahead of older ones until they hit connect_timeout_server_max and abort. Track the oldest waiter during the classifier's is_B arm (no second pass) and swap it to running_end. Key on max_connect_time, which the is_B test already loads -- the min-compare is a register compare with no extra memory access, and the smallest value is the waiter closest to the timeout abort. Gated by \|B\| > PARTITION_FAIRNESS_MIN_B (=4) and partition_active.	1 month ago
René Cannaò	a5bb7f640f	Merge branch 'v3.0' into v3.0_partition-gate	1 month ago
René Cannaò	46ab989adb	Merge commit from fork fix(security): reject addresses after PROXY UNKNOWN (CVE-2026-48772 / GHSA-gw94-85m2-x8v2)	1 month ago
René Cannaò	7dbe1f7029	Merge commit from fork fix(security): bound first-packet recv() in data streams (CVE-2026-48773 / GHSA-58ww-865x-grpr)	1 month ago
René Cannaò	3e7b77456e	Merge branch 'v3.0' into v3.0_partition-gate	1 month ago
René Cannaò	8025acdf60	Merge branch 'v3.0' into v3.0_pgsql_options_5801	1 month ago
René Cannaò	cda58357b2	Merge branch 'v3.0' into feature/ci-codecov-unit-tests	1 month ago
Rene Cannao	b04caafef1	fix(coverage): unbreak lcov capture for {MySQL,PgSQL}_Session.cpp Both lib/PgSQL_Session.cpp and lib/MySQL_Session.cpp contain a doxygen @note that literally spells out the strings `LCOV_EXCL_START` and `LCOV_EXCL_STOP` while documenting why those directives wrap the `assert(0)` block below the comment. lcov's geninfo scans every source line (comments included) for those tokens and treats them as real directives -- so the doc occurrence opens an exclude block, the real opener on the next assert opens it again, and geninfo aborts with: ERROR: <file>: overlapping exclude directives. Found LCOV_EXCL_START at line N - but no matching LCOV_EXCL_STOP for LCOV_EXCL_START at line M That error short-circuits the LCOV capture path in `test/infra/control/run-unit-tests-asan-coverage.bash` so `coverage/lcov.info` is never produced. The downstream "Upload coverage LCOV file" workflow step has been silently warning "No files found" for at least three runs (compare artifact listings of the three most recent CI-unit-tests-asan-coverage successes -- all zero artifacts), which in turn means the new Codecov upload step in #5817 also had nothing to send. Fix: rewrite the @note paragraph in both files so it describes what the directives do without literally containing the token strings, and explicitly call out that this is intentional. The wrapper directives around the actual `assert(0)` are unchanged. A clean lcov capture validates with `make ubuntu24-tap` followed by `docker compose run --rm --entrypoint bash ubuntu24_dbg_build -lc \ 'cd /opt/proxysql && test/infra/control/run-unit-tests-asan-coverage.bash'` which should produce a non-empty `coverage/lcov.info`.	1 month ago
Rahim Kanji	e491c899ad	refactor(pgsql): address PR review on #5801 options handling - append untracked options in place to avoid temporary strings - bail out in the regression test when setup/introspection queries fail	1 month ago
Rahim Kanji	093131cf27	Merge branch 'v3.0' into v3.0_pgsql_options_5801	1 month ago
Rahim Kanji	4bb067bf59	fix(pgsql): accumulate multiple untracked options startup parameters The handler overwrote untracked_option_parameters on each iteration, forwarding only the last unrecognized parameter to the backend. Append them instead so all are preserved.	1 month ago
Rahim Kanji	5e7677cb4d	fix(pgsql): strip trailing space from backend options startup parameter PgBouncer rejects an options value ending in whitespace; PostgreSQL tolerates it. Join tokens with a leading separator so it never does. Fixes #5801	1 month ago
René Cannaò	35cad4b4f4	Merge branch 'v3.0' into fix/kill-proxysqlgenai-build-flag-v2	1 month ago
Rene Cannao	10c2c237e9	chore(sonar): use snprintf for DNS-counter row formatting SonarCloud Quality Gate on PR #5809 is still ERROR despite all 0 TO_REVIEW hotspots being cleared (NOSONAR commits `6f6349973` + `de37c64f5`): the failing condition is `new_security_rating = 4` (D) caused by three CRITICAL sprintf vulnerabilities in lib/PgSQL_Thread.cpp introduced by `031f6fa1f` (the std::atomic adapter for dns_cache counters). The `buf` buffer (char[256] at line 4410) is large enough that overflow was extremely unlikely in practice -- %llu fits in 20 chars -- but Sonar correctly flags sprintf as a banned API for new code. Replace with snprintf(buf, sizeof(buf), ...) which has the same formatting behaviour but bounds the write to the actual buffer size. No behaviour change for the produced output; just closes the last Sonar Quality Gate condition.	1 month ago
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	a315ebe0be	fix(set-walker #150 ): document algo-3-vs-algo-0/1/2 whitespace divergence #150 in pgsql-set_parameter_validation_test-t (`SET search_path = '"$user" , public'`) sees algorithm-dependent SHOW output because of an existing destructive behaviour in the regex SET parser path: PgSQL_Set_Stmt_Parser::set_query() runs remove_spaces() on the whole SET query, which collapses every whitespace run -- INCLUDING runs inside string literals -- to a single space before parsing. So under algorithms 0/1/2, PG receives `'"$user" , public'` and SHOW returns `"""$user"" , public"`. Under algorithm 3 (ParserSQL walker) the query is parsed as-is, the original `'"$user" , public'` reaches PG, and SHOW returns `"""$user"" , public"`. Algo 3 is the more correct behaviour (PG would never see the destructive collapse) but diverges from algorithms 0/1/2's historical behaviour. Earlier this session I considered making the walker call remove_spaces() too just to bring algo 3 in line. That would have papered over the issue by propagating the pre-existing data-mangling bug into the new code path. Better: - Leave the walker honest -- it stays the more correct path. - Document the algo-0/1/2 quirk inline in PgSQL_Session.cpp at the parser-selection site so future readers know why the two paths don't match byte-for-byte. - Teach the test about the divergence: SetCommandTest gains an optional `expected_value_algo3` field, populated only for the affected case (#150). At test startup we query `pgsql-set_parser_algorithm` from proxysql admin once and pick the appropriate expected value per case. A real fix would be to make remove_spaces() string-literal-aware (or stop running it on SET commands altogether). That's a larger change with risk to other tests calibrated against today's behaviour, so it's deferred to its own ticket; this commit keeps the regression green on both algorithm groups (PASS 1/345, FAIL 0/345 locally for both pgsql-set_parameter_validation_test-t under set_parser_algorithm_3-g1 and legacy-g6).	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	35f84c384b	fix(pgsql-session): don't lock hostgroup on parse fail for tracked vars When the SET parser returns an empty map for a SET that targets a proxysql-tracked variable (search_path, datestyle, ...), the previous behaviour called unable_to_parse_set_statement(), which set *lock_hostgroup = true. From that point on, the entire SET-handling function was short-circuited for the rest of the session, so every subsequent SET -- including ones that DO parse and would have been caught by the per-variable validator -- bypassed proxysql entirely and went straight to PG. That cascade is what pgsql-set_parameter_validation_test-t case #184 hit: case #172's malformed `SET search_path = "unclosed_quote, public` caused the parser to (correctly) reject, locked the hostgroup, and from then on case #184's 64-char delimited identifier `SET search_path TO "1234...64chars..."` skipped the search_path validator's "> 63 chars -> reject" check and reached PG, which silently accepts/truncates such identifiers. The test then saw the SET succeed unexpectedly and reported the misleading "Value changed unexpectedly from '"$user"' to ''" message (test_ok flipped before the SHOW value was read; new_value defaults to "" in that branch). The fix: at this specific call site (inside the `match_regexes[1]->match(dig)` block, i.e. we already know the SET targets a tracked variable), drop the unable_to_parse_set_statement() call. Log a warning and return false so the SET is forwarded to PG normally. PG itself rejects truly malformed SETs (which was already the observed behaviour for cases #172-#183), and subsequent valid SETs on the same connection continue to flow through proxysql's validator. The locking branch at the outer `else` (when the regex didn't match -- i.e. genuinely unknown SET) is unchanged. End-to-end verification gates on CI -- there's no local PG infra in this build -- but the in-process unit test TestWalkerToValidatorChain184 (added in the previous commit) already confirmed that walker + validator together reject the 64-char input, which is exactly the path #184 now reaches with the lock removed. Diagnostic SET-VALIDATE proxy_info logging from the previous commit is removed.	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	6f63499731	chore(sonar): silence cpp:S2245 PRNG false-positives in DNS jitter sites Two SonarCloud "weak-cryptography" hotspots flagged uses of mt19937 (DNS_Cache.cpp:117) and rand() (PgSQL_Monitor.cpp:2956). Both are non-cryptographic jitter sources -- one to spread the DNS cache TTL refresh, the other to spread the DNS bookkeeping refresh interval -- with no security boundary touching them. Adding inline NOSONAR annotations with the reasoning so future reviewers (human or bot) don't have to re-derive it.	1 month ago
Rene Cannao	0d39ac6991	build: kill PROXYSQLGENAI flag, PROXYSQL40=1 builds all plugins PROXYSQLGENAI was a leftover from before the genai plugin carve-out. All genai/MCP/AI/RAG code now lives in plugins/genai/ as a runtime loadable .so. Since the plugin chassis (PROXYSQL40=1) already builds and loads the genai .so, a separate flag serves no purpose. Entrypoint changes (all three -- rhel, suse, deb): - Build: pass PROXYSQL40=1 (not PROXYSQLGENAI=1) to make. - Packaging: copy genai .so alongside mysqlx when PROXYSQL40=1. - protobuf-devel install: gate on PROXYSQL40=1 (needed for mysqlx). Automated packaging verification added to all entrypoints: rpm -qpl / dpkg -c confirms both .so files are in the package. ELF smoke test: validates .so is a valid shared object exporting proxysql_plugin_descriptor_v1 symbol. Fails build on mismatch. Test Makefiles: PROXYSQLGENAI -> PROXYSQL40 for the vec.o link gate (test/tap/tests/Makefile and test/deps/cluster_simulator/Makefile). Updated remaining PROXYSQLGENAI references to PROXYSQL40 in: CLAUDE.md, include/cpp.h, lib/proxy_sqlite3_symbols.cpp, src/main.cpp (3x comments), proxysql.spec, test/infra/control/start-proxysql-isolated.bash, test/infra/control/run-unit-tests-asan-coverage.bash, and 14 genai test file comments.	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	031f6fa1f5	fix(build): adapt callers of dns_cache counters to std::atomic The previous commit promoted MySQL_Monitor::dns_cache_{queried, lookup_success,record_updated} to std::atomic<unsigned long long> but missed several call sites that pass them through implicit copy: * lib/MySQL_Thread.cpp sprintf("%llu", GloMyMon->dns_cache_) — varargs cannot copy std::atomic. lib/PgSQL_Thread.cpp sprintf("%llu", GloPgMon->dns_cache_) — same, for the PgSQL counters. lib/MySQL_Monitor.cpp p_update_counter(..., GloMyMon->dns_cache_) — works via implicit operator T(), but explicit .load() is clearer and matches the rest of the file. Promote PgSQL_Monitor::dns_cache_ to std::atomic for the same race that motivated the MySQL side (resolver workers increment, stats readers and p_update_metrics read from another thread), and add .load() at every read site so we don't depend on implicit conversion. Verified with a local 'make -j32 debug' build in the integration worktree.	1 month ago
Rene Cannao	ca145d5026	fix(pgsql): atomic shutdown flag; honor dns_lookup fallback contract GloPgMon->shutdown was a plain bool, written by main.cpp's teardown and read by the monitor / DNS resolver loops on worker threads — a data race that can let the loops spin past shutdown or observe a stale value indefinitely. Promote to std::atomic<bool> and switch the readers/writer to acquire/release ordering. PgSQL_Monitor::dns_lookup() was also returning an empty string when GloPgMon / dns_cache_thread weren't initialized yet, ignoring the return_hostname_if_lookup_fails contract. Move the fallback out of the dns_cache_thread branch so startup / shutdown behave like a normal cache miss for callers that asked for the hostname fallback. Addresses CodeRabbit review feedback on PR #5806.	1 month ago
Rene Cannao	039d538c2a	fix(dns): correct inet_pton check; make DNS cache counters atomic validate_ip() treated inet_pton(...) != 0 as success, but inet_pton returns 1 / 0 / -1; a -1 (e.g. unsupported family) would be reported as a valid IP. Compare against == 1 instead. The DNS cache counters (dns_cache_queried / dns_cache_lookup_success / dns_cache_record_updated) were declared as plain unsigned long long on MySQL_Monitor. Resolver workers update them atomically via __sync_fetch_and_add, but p_update_metrics() reads them from a different thread without a barrier, racing the writers. Promote to std::atomic<unsigned long long> and switch DNS_Cache to fetch_add / the atomic pointer type so both ends are consistent. Addresses CodeRabbit review feedback on PR #5806.	1 month ago
René Cannaò	1d52499791	Merge pull request #5808 from sysown/issue_5639 fix(session): avoid double-free of stmt_meta pkt in LargePacket (#5639)	1 month ago
René Cannaò	af4aa17346	Merge pull request #5807 from sysown/fix/5790-mariadb-collation-255 fix(core): validate_charset must replace collation 255 on MariaDB backends (#5790)	1 month ago
René Cannaò	fadffdbcbd	Merge pull request #5806 from sysown/pgsql_dns_cache Independent DNS cache for PgSQL (fixes #5768)	1 month ago
René Cannaò	642b2327b7	Merge pull request #5805 from sysown/fix/parsersql-1.0.3-pg-set-fixes fix: pgsql digest fixtures (typecast spacing) + ParserSQL 1.0.3 bump (PG SET TIME ZONE + multi-value lists)	1 month ago
Rene Cannao	3fd7aae059	fix(core): wrap MON_GALERA case body in braces (compile fix) The previous commit introduced local variable declarations with initializers (`const char *sv = …`, `bool is_mariadb = …`, etc.) directly inside `case MON_GALERA:` of the `switch` in `init_async()` at lib/MySQL_Monitor.cpp:742. The sibling `case MON_CLOSE_CONNECTION:` / `MON_CONNECT:` / `MON_AWS_AURORA:` labels then jump past these initializations, which GCC rejects: MySQL_Monitor.cpp:779:14: error: jump to case label MySQL_Monitor.cpp:781:14: error: jump to case label MySQL_Monitor.cpp:783:14: error: jump to case label Wrap the `#else` body of the MON_GALERA case in `{ ... }` so the declarations live in their own scope and no sibling case can jump over them. No behavior change. The async path at lib/MySQL_Monitor.cpp:2299 was already inside its own `{ ... }` block (added with the original code, not by this patch), so it compiled fine and needs no change. The `MySQL_Session.cpp` edits live inside `if / else if` chains with their own braces, also unaffected.	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	f2b53ea6e3	Address findings from independent diff review A subagent code review on top of the CodeRabbit + Gemini bot reviews found five additional issues that the bots missed. All five are fixed here. 1. LOAD PGSQL SERVERS TO RUNTIME was waking MySQL's resolver, not PgSQL's. PgSQL_HostGroups_Manager::commit() inherited a MySQL_Monitor::trigger_dns_cache_update() call from a copy of the MySQL HGM, so after a LOAD pgsql hostnames had to wait for the refresh interval (default 60 s) before becoming resolvable — defeating the warm-cache-before-traffic intent that motivated issue #5768. Route to PgSQL_Monitor::trigger_dns_cache_update(). Verified directly: with refresh_interval=60000 ms, inserting a pgsql_servers row + LOAD now produces record_updated=1 within ~4 s instead of waiting a full minute. 2. DNSResolverWorker could deadlock shutdown. monitor_dns_resolver_thread isn't noexcept (vector growth, set_thread_name, etc. can throw); if it threw, the std::promise on DNS_Resolve_Data was never satisfied, the producer's future::get() blocked forever, the resolver loop never reached the shutdown check, and pthread_join in main hung. Wrap the call in try/catch and force-satisfy the promise with a failure result on any uncaught exit. 3. Test step 3 wasn't actually exercising the connect-path lookup it claimed. hammer_proxy() routes through the proxy front-end, which in standard test infra resolves through the default hostgroup — typically configured with an IP literal (see e.g. test/infra/docker-pgsql16-single/conf/proxysql/config.sql). IP literals bypass dns_lookup via validate_ip(), so the dns_cache_queried bumps observed came from the resolver loop alone, not from the connect path. Step 3 would have passed even if PgSQL_Connection::connect_start_DNS_lookup() was wired up wrong. Install a pgsql_query_rules row from inside the test that routes testuser to hostgroup 999, so the proxy actually calls into the cache on client connects. Clean up the rule in the test exit path. 4. Use-after-move in the _dns_cache_update debug log. Both PgSQL_Monitor::_dns_cache_update and MySQL_Monitor::_dns_cache_update logged debug_iplisttostring(ip_address) after std::move'ing ip_address into add_if_not_exist(). The moved-from vector is in a "valid but unspecified" state — typically empty — so the log printed "IP:[]" instead of the IPs. Capture the string before the move. Same bug also fixed in the bookkeeper expired-record requeue path inside PgSQL_Monitor::monitor_dns_cache. 5. monitor_dns_cache had inconsistent indentation inside the `if (hostnames.empty()) { ... } else { ... }` block introduced when I wrapped the original goto-based control flow. The braces were correct but the body sat at the wrong tab depth, making the scope visually misleading and prone to mis-edits. Re-indented. Findings deliberately skipped from the review (rationale in commit body): - "thread_local shared_ptr<DNS_Cache> keeps stale cache alive across GloPgMon recreate" — pre-existing MySQL pattern; GloPgMon is never destroyed mid-process today. - "PgSQL_monitor_dns_cache_pthread can deref GloPgMon during a tight shutdown" — shutdown ordering in main.cpp sets GloPgMon->shutdown before joining the thread and deletes GloPgMon only afterwards, matching the MySQL pattern. - "cache only seeded on ASYNC_CONNECT_SUCCESSFUL" — matches MySQL; pre-existing scope choice. - "validate_ip rejects bracketed IPv6" — pre-existing in MySQL; no current configuration path produces bracketed IPv6 in pgsql_servers. - "Test relies on outbound DNS for example.com" — matches the upstream MySQL test_dns_cache test which uses google.com / yahoo.com / amazon.com. Established convention. - "ai_family hard-coded to AF_UNSPEC for PgSQL" — already commented in code; follow-up if/when pgsql-resolution_family is added.	1 month ago
Rene Cannao	81bcc802da	fix(core): apply MariaDB / MySQL 9.x detection to sibling version checks The same mental defect fixed in lib/MySQL_Variables.cpp:validate_charset() (commit before this one) exists at four other call sites in core. All four use a first-character or 3-byte string-prefix comparison against server_version and consequently misclassify either MariaDB or MySQL 9.x (or both). Two are latent (MariaDB happens to land on the still-working branch); the two in MySQL_Monitor.cpp are an active break on MySQL 9.x Galera/PXC, because `INFORMATION_SCHEMA.GLOBAL_STATUS` was removed in MySQL 8.4 and MySQL 9.x falls into the else branch that targets it. Audited sites and fixes: lib/MySQL_Session.cpp:2607-2615 -- tx_isolation / tx_read_only Pre-fix: `strncmp(sv,"8",1)==0` chose `transaction_isolation` / `transaction_read_only`; everything else got `tx_isolation` / `tx_read_only`. MySQL 9.x fell to the else branch, but `tx_isolation` was removed in MySQL 8.4; on MySQL 9.x the SET fails. MariaDB also fell to the else branch -- coincidentally correct, since MariaDB keeps `tx_isolation` across all versions. Post-fix: use the modern name only for non-MariaDB MySQL >= 8.0 (any major), so MySQL 8.0/8.4/9.x all get `transaction_isolation` and MariaDB 10/11 keeps `tx_isolation`. lib/MySQL_Monitor.cpp:749 (sync MON_GALERA setup) lib/MySQL_Monitor.cpp:2297 (async MON_GALERA dispatch) Pre-fix: `strncmp(sv,"5.7",3)==0 \|\| strncmp(sv,"8",1)==0` picked performance_schema.global_status; everything else used INFORMATION_SCHEMA.GLOBAL_STATUS. MySQL 9.x fell to the else branch and queries INFORMATION_SCHEMA.GLOBAL_STATUS, which was removed in MySQL 8.4 -- monitor query fails on MySQL 9.x Galera/PXC. MariaDB Galera correctly fell to the else branch and uses information_schema, which is where MariaDB keeps GLOBAL_STATUS. Post-fix: use performance_schema for non-MariaDB MySQL >= 5.7 (so MySQL 5.7, 8.0, 8.4, 9.x all share that path); MariaDB Galera and legacy MySQL/PXC < 5.7 retain the information_schema path. Pattern used at all sites (matching the existing fix in validate_charset): const char sv = ...->mysql->server_version; bool is_mariadb = (sv != NULL && strstr(sv, "MariaDB") != NULL); int sv_major = (sv != NULL) ? atoi(sv) : 0; No dedicated test added for the Monitor / SET TRANSACTION paths in this commit: the Monitor change is exercised by any mariadb10-galera or mysql{8,9}x- CI group with Galera/PXC backends, and the transaction_isolation change by SET-isolation-level coverage in existing TAP groups. The validate_charset regression test added in the prior commit on this branch still covers the original #5790 path.	1 month ago
Rene Cannao	79414c10f1	fix(core): guard atoi(server_version) in validate_charset (#5790 review) Address @gemini-code-assist review on PR #5807: the previous patch unconditionally evaluated `atoi(sv)` in the disjunction `(is_mariadb \|\| atoi(sv) < 8)`. If `sv` was NULL, the short-circuit on `is_mariadb` (false) would still reach `atoi(NULL)`, which is undefined behavior per the C standard. In practice `myconn->mysql->server_version` is non-NULL by the time validate_charset() runs (CHANGING_CHARSET is only reachable after the backend handshake has populated server_version), so this is defensive, not a real crash path. Still essentially free, and keeps the guard consistent with the existing NULL check on `strstr(sv, "MariaDB")`. No behavior change for any reachable input.	1 month ago

1 2 3 4 5 ...

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