Quote EXTRACT fields and bump PostgreSQL minors - #8804
Merged
ibrahim halatci (ihalatci) merged 12 commits intoSep 2, 2026
Conversation
Match PostgreSQL's CVE-2026-15741 stable-branch fix by quoting EXTRACT field identifiers in the copied ruleutils implementations. Add version-specific regression coverage that verifies statement-like field text cannot escape the deparsed expression. Fixes #8803 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8804 +/- ##
==========================================
- Coverage 88.74% 88.73% -0.01%
==========================================
Files 289 289
Lines 64992 65013 +21
Branches 8200 8204 +4
==========================================
+ Hits 57676 57689 +13
- Misses 4950 4953 +3
- Partials 2366 2371 +5 🚀 New features to boost your workflow:
|
Restore the pg16 search path after reconnecting so the test table is removed by the existing schema cleanup on every repetition. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
DESCRIPTION: Fail clearly when output_plugin_libraries disallows citus plugin Validation PR for the-process [#242](citusdata/the-process#242). Repoints CI at the release images carrying the new PostgreSQL minors published 2026-08-14/15. | | before | after | |---|---|---| | PG16 | 16.14 | 16.15 | | PG17 | 17.10 | 17.11 | | PG18 | 18.4 | 18.6 | `image_suffix`: `-ved0dad9` Note: PostgreSQL **18.5 was never released** (withdrawn after a regression), so 18.4 -> 18.6 is the correct step. --- Validation surfaced two independent PostgreSQL security changes in this minor set that break Citus. When a `COPY` fails early, the new psql silently swallows every line up to and including the next `\.`. Tests that relied on the old behaviour were adjusted: a terminating `\.` was added where one was missing, stray statements after a swallowed block were removed, and expected output was trimmed for the lines that no longer execute. PostgreSQL 14.24, 15.19, 16.15, 17.11 and 18.6 add an `output_plugin_libraries` GUC. Only the libraries it lists may be used as logical decoding output plugins. It defaults to `pgoutput, test_decoding` and is `superuser`-settable, so it takes a reload rather than a restart. **Upgrade note for operators.** Citus uses an output plugin named `citus` for logical replication during non-blocking shard splits. On these minors the following fail until the plugin is allowed: * `citus_split_shard_by_split_points(..., 'force_logical')` and `'auto'` * `create_distributed_table_concurrently()` * `citus_isolate_tenant_to_new_shard(..., 'force_logical'` / `'auto')` Shard **moves** and the rebalancer are unaffected -- they use `pgoutput`. CDC through the `pgoutput` shim is unaffected; CDC through `wal2json` needs the same treatment, exactly as it does on vanilla PostgreSQL. Remediation, on **every** node: ```sql ALTER SYSTEM SET output_plugin_libraries = pgoutput, test_decoding, citus; SELECT pg_reload_conf(); ``` The GUC is `GUC_LIST_QUOTE`, so quoting the whole list (`'pgoutput, test_decoding, citus'`) stores it as a single name and does **not** work. Use the bare list above, or quote each element individually. This PR does not work around the restriction -- allowing a decoder is deliberately an operator action. Instead Citus now **fails fast and clearly**: a preflight check runs against the source node before any shards, publications or replication slots are created, and raises an error naming the plugin, the node, the current allowlist value, and a ready-to-paste `ALTER SYSTEM` hint. The preflight parses the GUC the same way PostgreSQL's own `SplitGUCList` does: all whitespace is trimmed from each element, and surrounding quotes are removed only when an element is fully quoted, with interior `""` collapsed to `"`. Verified against 16.15 with `pg_create_logical_replication_slot` as ground truth. The regression and CDC harnesses start their own clusters, so in CI *we* are the operator: `pg_regress_multi.pl` and `cdctestlib.pm` now write the allowlist themselves, guarded by a probe of `postgresql.conf.sample` so they stay compatible with older minors where the GUC does not exist. To keep the un-remediated path covered, a new `check-split-output-plugin-denied` job deliberately skips that override (via `CITUS_TEST_SKIP_OUTPUT_PLUGIN_ALLOWLIST=1`) and asserts that a non-blocking split fails with the new error and leaks no shards, publications or replication slots. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c6370b2-06fd-4491-bf92-ecb811d34518 (cherry picked from commit c457188)
Preserve the PG19 support changes while forward-porting the PostgreSQL minor upgrade from release-14.0. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep the merged PG19 test files byte-identical outside the minor-upgrade changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep overlapping PG19 COPY adaptations byte-identical to main after the minor-upgrade forward port. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ibrahim halatci (ihalatci)
changed the title
Use the live the-process #244 dev images, which retain PG19 while updating PG17 and PG18. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This was referenced Aug 27, 2026
Run one shared, repeat-safe regression from the N-1-excluded create-Citus schedule across supported PostgreSQL versions. PG19 keeps its dedicated coverage from #8753. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ibrahim halatci (ihalatci)
added a commit
that referenced
this pull request
Aug 28, 2026
## Summary - backport the focused CVE-2026-15741 EXTRACT field quoting fix from #8804 - apply `quote_identifier(TextDatumGetCString(con->constvalue))` only to the PostgreSQL 15, 16, and 17 ruleutils copies supported by release-13.2 - add one shared, repeat-safe regression under `multi_1_create_citus_schedule`, keeping it out of the N-1 matrix whose older libraries intentionally lack the fix - leave unsupported PostgreSQL 18 code untouched Fixes #8803. ## Validation - PostgreSQL 15.19 + Citus build with `-Werror` - PostgreSQL 16.15 + Citus build with `-Werror` - PostgreSQL 17.11 + Citus build with `-Werror` - shared `extract_deparse` regression on PG15.19, PG16.15, and PG17.11 - each supported version: `--repeat 8 --use-whole-schedule-line` (24/24 target executions passed) - `git diff --check` - SQL snapshot consistency check ## CI triage All functional checks pass, including PG15-17 builds and regressions, style/snapshot checks, and PG17 N-1 compatibility jobs. The remaining 32 `Test flakyness` failures reproduce the existing `multi_follower_dml` failure on the latest unmodified `release-13.2` run: https://github.com/citusdata/citus/actions/runs/32951828115. The `codecov/project` failure is downstream of those baseline failures. --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 49709f7a-73da-4256-8380-8e46d17f61e0
ibrahim halatci (ihalatci)
added a commit
that referenced
this pull request
Aug 28, 2026
## Summary Backport the focused EXTRACT identifier quoting parity fix from [#8804](#8804) to `release-14.0` for PostgreSQL 16, 17, and 18. - quote EXTRACT field identifiers in the copied PG16/PG17/PG18 `ruleutils` implementations - add one shared, repeat-safe `extract_deparse` regression covering every supported PostgreSQL major - keep that new-binary assertion in `multi_1_create_citus_schedule`, outside the 14.2 N-1 mixed-library matrix - preserve the existing shard-aware Citus deparser changes Fixes [#8803](#8803). Main PR: [#8804](#8804) ## Validation - built against PostgreSQL 16.15, 17.11, and 18.6 in isolated WSL prefixes - pre-consolidation PG16/PG17/PG18 focused tests passed eight repetitions with whole schedule lines - final shared `extract_deparse` test passed focused PG16 validation; identical shared SQL/expected output is scheduled for PG16-18 - PG16 and PG18 full owning schedules passed 39/39 before consolidation - PG17 full schedule was 188/190; both failures reproduced unchanged on pristine `release-14.0` and did not touch EXTRACT output - Citus formatting/style checks, SQL snapshots, and `git diff --check` passed - follow-up isolates the new assertion from expected pre-fix 14.2 coordinator-library behavior in N-1 CI ## CI triage All current-version, feature, packaging, style, snapshot, and relevant N-1 checks pass. The remaining `Test Citus Lib N-1 / PG18 - check-multi` failure and all 32 `Test flakyness` failures match unmodified `release-14.0` baseline run https://github.com/citusdata/citus/actions/runs/32951765361. The N-1 failure is unrelated `multi_insert_select` AIO DEBUG output; the flakiness gate runs broad release-vs-main history and fails pre-existing tests. `codecov/project` is downstream of those baseline failures. --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ibrahim halatci (ihalatci)
added a commit
that referenced
this pull request
Aug 28, 2026
## Summary Backport the focused EXTRACT identifier quoting parity fix from [#8804](#8804) to `release-12.1` for PostgreSQL 14, 15, and 16. - quote EXTRACT field identifiers in the copied PG14/PG15/PG16 `ruleutils` implementations - add one shared, repeat-safe `extract_deparse` regression covering every supported PostgreSQL major - run it from release-12's existing current-library `multi_1_schedule`; this branch predates the N-1 mixed-library schedule split - preserve the existing shard-aware Citus deparser changes Fixes [#8803](#8803). Main PR: [#8804](#8804) ## Validation - `-Werror` builds against PostgreSQL 14.24, 15.19, and 16.15 - shared `extract_deparse` regression: 8/8 repetitions on each supported PostgreSQL major - expected output generated with the release-12 regression runner - shared SQL matches the main-branch test byte-for-byte; expected output matches the proven release runner output - relevant style, schedule, SQL snapshot, and diff checks Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
Copilot resolve the merge conflicts in this pull request |
…ract-quoting # Conflicts: # .github/workflows/build_and_test.yml # src/backend/distributed/deparser/ruleutils_16.c Co-authored-by: ihalatci <10450368+ihalatci@users.noreply.github.com>
Contributor
Resolved by merging |
the-process#244 has merged, so master now publishes stable images for PostgreSQL 17.11 / 18.6 / 19beta3. Replace the temporary -dev-aeb145c pin with the released -vdba9cbb suffix. citusupgradetester is built and pushed for PG18, but the matrix only listed PG17, so the PG18 image was never exercised. Add it. PG19 stays out because no released Citus supports it yet, which is why the the-process Makefile filters PG19 out of CITUS_UPGRADE_PG_VERSIONS. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c6370b2-06fd-4491-bf92-ecb811d34518
psql only echoes non-empty input lines, so the blank lines in extract_deparse.sql never reach the results file. The expected file carried five of them and check-multi-1-create-citus failed on PG17, PG18 and PG19. Drop those blank lines; the expected file is now byte-identical to the results produced on all three majors. pg_regress_multi.pl also set output_plugin_libraries='pgoutput,citus' for PG19 without honouring CITUS_TEST_SKIP_OUTPUT_PLUGIN_ALLOWLIST, so check-split-output-plugin-denied still allowed the citus plugin there and the split was not rejected. That block is redundant: PG19 ships output_plugin_libraries in postgresql.conf.sample, so the skip-aware block below already covers it with a superset value. Remove it, which lets the denied target fall back to the PostgreSQL default on PG19 as it already does on PG17 and PG18. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c6370b2-06fd-4491-bf92-ecb811d34518
The flaky test detector runs newly added regression tests through run_test.py, which starts the cluster with the regular options. That puts "citus" in output_plugin_libraries, so the shard split succeeds and the expected "not allowed" error never appears. The test only behaves as written under the check-split-output-plugin-denied target, which sets CITUS_TEST_SKIP_OUTPUT_PLUGIN_ALLOWLIST. Add it to the existing skip list next to the upgrade and snapshot based node addition tests. The test keeps running on PG17, PG18 and PG19 through its own target, so coverage is unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7c6370b2-06fd-4491-bf92-ecb811d34518
ibrahim halatci (ihalatci)
enabled auto-merge (squash)
September 1, 2026 16:08
ibrahim halatci (ihalatci)
requested review from
Colm (colm-mchugh) and
Onur Tirtir (onurctirtir)
and
a balanced review from Copilot
September 1, 2026 16:10
Copilot started reviewing on behalf of
ibrahim halatci (ihalatci)
September 1, 2026 16:10
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Secures EXTRACT deparsing and updates testing for the latest PostgreSQL minors.
Changes:
- Quotes EXTRACT field identifiers in PG17/PG18 deparsers.
- Handles
output_plugin_librariesfor shard splitting and CDC tests. - Updates PostgreSQL versions and PG18 upgrade coverage in CI.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/backend/distributed/deparser/ruleutils_17.c |
Safely quotes EXTRACT fields. |
src/backend/distributed/deparser/ruleutils_18.c |
Safely quotes EXTRACT fields. |
src/backend/distributed/operations/shard_split.c |
Preflights the output-plugin allowlist. |
src/test/cdc/t/cdctestlib.pm |
Configures required CDC plugins. |
src/test/regress/pg_regress_multi.pl |
Configures supported test plugins. |
src/test/regress/Makefile |
Adds the negative split target. |
src/test/regress/split_output_plugin_denied_schedule |
Defines isolated split coverage. |
src/test/regress/sql/split_output_plugin_denied.sql |
Tests denied split cleanup. |
src/test/regress/expected/split_output_plugin_denied.out |
Records expected denial output. |
src/test/regress/sql/extract_deparse.sql |
Tests EXTRACT injection prevention. |
src/test/regress/expected/extract_deparse.out |
Records EXTRACT test output. |
src/test/regress/multi_1_create_citus_schedule |
Schedules EXTRACT coverage. |
.github/workflows/build_and_test.yml |
Bumps minors, images, and PG18 upgrade testing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| WHERE id = 1; | ||
| END IF; | ||
| EXCEPTION | ||
| WHEN invalid_parameter_value THEN NULL; |
Contributor
There was a problem hiding this comment.
nit: this assertion is negative only; the expected failure is swallowed, and the only check is that injected doesnt exist. To future-proof, a positive check - eg EXTRACT ('year' FROM ts) returns 2026 pins the deparse path. Not blocking though, only consider if its likely a future change could stop the expression being pushed down, or changes the SQLSTE.
Contributor
Author
There was a problem hiding this comment.
Good catch, and it turned out to be more than a nit - fixed in #8821.
Two silent-pass paths existed: the schema issue you flagged below, and this one - if a planner change ever stopped the expression being pushed down, nothing would run on a worker and the test would still pass. Added the positive control you suggested:
SELECT EXTRACT('year' FROM ts) = 2026 AS extract_field_pushdown_works
FROM extract_deparse_source
WHERE id = 1;It also fixes a second gap: on PG19 the DO block is a no-op by design (guarded on server_version_num < 190000), so this file's only assertion was vacuous there. The positive control is unguarded, so it runs on every version.
| -- PG19 has equivalent coverage in pg19.sql. | ||
| DO $$ | ||
| BEGIN | ||
| IF current_setting('server_version_num')::int < 190000 THEN |
Contributor
There was a problem hiding this comment.
nit: is literal value (190000 here) the convention for PG versions in regress tests, or do we use macros / constant defs ?
Contributor
Author
There was a problem hiding this comment.
Checked - the literal is the convention; there's no macro or helper for version gating in regress tests.
server_version_num appears in only two places under src/test/regress/sql/: this file, and multi_orderby_limit_pushdown.sql:186, which uses the identical idiom:
IF current_setting('server_version_num')::int >= 190000 THENLeaving as-is - noted in #8821.
|
|
||
| SELECT bool_and(result::boolean) AS extract_field_injection_blocked | ||
| FROM run_command_on_workers($$ | ||
| SELECT to_regclass('extract_deparse.injected') IS NULL |
Contributor
There was a problem hiding this comment.
Nit: This expression only detects an injected table if it lands in that schema; something like SELECT count(*) FROM pg_class WHERE relname = 'injected' would be schema-agnostic.
Contributor
Author
There was a problem hiding this comment.
You're right, and this was actually a false negative rather than a style issue - fixed in #8821.
The injected table never lands in that schema. The deparser fully qualifies task SQL on purpose (PushEmptySearchPath() - "Set search_path to NIL so that all objects outside of pg_catalog will be schema-prefixed", ruleutils_17.c:655 / _18.c:683 / _19.c:689), so Citus doesn't send a SET search_path for SELECT task execution. A successful CREATE TABLE injected() therefore goes to the worker session's default search_path (public), not extract_deparse.
So to_regclass('extract_deparse.injected') IS NULL returned true even if the injection had succeeded - the test could not detect the vulnerability it exists to catch.
Went with your suggestion, which also matches citus_internal_distribute_object.sql (L181/188/190/203/210). Same fix applied to pg19.sql, which had the same problem with pg19_repack.injected.
Colm (colm-mchugh)
approved these changes
Sep 2, 2026
Colm (colm-mchugh)
left a comment
Colm (colm-mchugh)
left a comment
Contributor
There was a problem hiding this comment.
few non-blocking nits, LGTM overall.
ibrahim halatci (ihalatci)
deleted the
ihalatci-backport-extract-quoting
branch
September 2, 2026 11:41
Contributor
Author
Colm (@colm-mchugh) please see follow up PR #8821 - I had already marked this auto-complete. |
ibrahim halatci (ihalatci)
pushed a commit
that referenced
this pull request
Sep 2, 2026
Matches build_and_test.yml after #8804. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DESCRIPTION: Quote EXTRACT fields and support latest PostgreSQL minors
Fixes #8803.
Changes
ruleutils_19.calready carries the equivalent fix from Pg19 support #8753;output_plugin_librarieshandling and modern psql COPY behavior;test-citus-upgradematrix.PG18 citus upgrade coverage
citusupgradetesteris built and published for PG18 (CITUS_UPGRADE_VERSIONS_18=v14.2.0), but the matrix listed only PG17, so that image was built and never exercised. This adds it, matching the convention onrelease-14.0, which lists every supported major except the newest.PG19 stays out deliberately. The image compiles released Citus from source, and no released Citus supports PG19 yet, which is why the-process filters it out of
CITUS_UPGRADE_PG_VERSIONS. PG19 upgrade paths are still covered bytest-pg-upgrade, whose matrix already includes 17→18, 18→19, and 17→19.Cross-repo dependency
The test images are built by the-process#244, stacked above the-process#240. Both have merged, along with #8757 and #8764, so this PR is last in that sequence and now pins the released
-vdba9cbbsuffix.Validation
-Werrorbuilds: PostgreSQL 17.11, 18.6, and 19beta3extract_deparse: 8/8 repetitions on each of PostgreSQL 17.11, 18.6, and 19beta3split_output_plugin_deniedregression on PG18.6citus_indent --checkpg_regress_multi.plsyntax check-vdba9cbbsuffix before pinningKnown limitation carried over from the release branches
The
output_plugin_librariespreflight splits the GUC on commas, so a plugin name containing a comma inside a quoted element ("a,b") is not parsed the way PostgreSQL'sSplitGUCListwould parse it. This is unreachable for the namecitusand matches what already shipped on release-14.0, release-13.2, and release-12.1.