Pg19 support - #8753
Open
ibrahim halatci (ihalatci) wants to merge 36 commits into
Open
Conversation
… CI image bump) (#8601) DESCRIPTION: Add PG19 build support; extension compiles and loads. First PR of the PG19 enablement series tracked in #8597. Achieves source-level compatibility with PG19 (`pg19devel`) and a clean `CREATE EXTENSION citus` while keeping PG17 and PG18 builds green. Regression-test correctness and PG19 feature integration land in follow-up PRs against the `pg19-support` development branch. ## Commits 1. **ci** — bump `image_suffix` to `-dev-e11d99c` (PG19-capable test images) and PG minors to 16.14/17.10/18.4; add `pg19_version` and wire `19devel` into the **build matrix only**. 2. **source-level compatibility + shmem sizing** — accept PG19 in `configure` / `pg_version_constants.h`; centralised compat shims in `pg_version_compat.h`; distributed/columnar header adaptations for renamed/retyped PG19 APIs; 128 KB shmem slack on PG19 for the new `ShmemInitHash` up-front allocation, plus the `maintenanced.c` double-count fix that change exposes. 3. **-Werror source fixes** — warning fixes across 56 files (old-style prototypes, strict forward declarations, implicit-fallthrough, new switch arms, API renames). **No compiler-flag demotion** — the build keeps the same warning/error flags as PG17/PG18. 4. **ruleutils_19.c scaffold** — clone of `ruleutils_18.c` retargeted at the PG19 version guard so the binary links (per the #7725 / #8010 precedent; the real upstream-derived port lands in a follow-up PR). Excluded from citus-style via `.gitattributes` — it is an upstream-derived clone exceeding the CI uncrustify 10,000-line limit. 5. **test** — add PG19 to the oldest-supported Citus version matrix. ## Scope / non-goals - **Build path only.** PG19 is intentionally NOT added to `test-citus`, `test-citus-failure`, `test-citus-cdc`, or the upgrade jobs: the regression suite needs the upstream-derived `ruleutils_19.c` from the follow-up PR, and no released Citus supports PG19 yet. Running the suite on this PR alone produces silent PG18-on-PG19 deparse corruption. - `multi_explain.c` is touched only for the `Instrumentation` / `NodeInstrumentation` rename; a separate `TupleDescFinalize` runtime fix lands in a later PR. ## Validation - PG19devel / PG17.10 / PG18.4: build exits 0 (validated in CI images). - `CREATE EXTENSION citus` succeeds on PG19. - No regression-suite changes here. Refs: #8597
DESCRIPTION: Port upstream PG19 ruleutils.c into Citus deparser tree. Mirrors #8010 (PG18) and #7725 (PG17). Replaces the ruleutils_19.c placeholder from the build-foundation PR with a proper port of upstream PG19's src/backend/utils/adt/ruleutils.c, produced by a 3-way merge (git merge-file: ruleutils_18.c as base, upstream PG18 and upstream PG19 as the two sides). All merge conflicts fell in blocks Citus had already stripped from _18.c or in shard-aware deparse variants, and were resolved by keeping the Citus side. The file keeps Citus' existing curation (deparse hooks, shard-aware UPDATE/DELETE branches) while picking up real upstream PG18->PG19 deparse changes. Without this, Citus on a PG19 backend would deparse worker-bound SQL using PG18 semantics -- silent corruption the moment a PG19-only node, clause, or formatting decision appears in the tree. The trailing #endif guard text is corrected to "(PG_VERSION_NUM >= PG_VERSION_19) && (PG_VERSION_NUM < PG_VERSION_20)" (the _17.c/_18.c snapshots carry stale guard comments; not propagated). The generate_function_name fgc_flags fix is absorbed naturally by the merge -- upstream PG19 already passes &fgc_flags at that call site. Excluded from citus-style via .gitattributes (added in the build-foundation PR): the file exceeds the CI uncrustify 10,000-line limit and is upstream-derived. Refs: #8597 --- Stacked on #8601 (`pg19-build-foundation`).
Turns on the PG19beta1 columns across the downstream Build & Test matrices now that the `build` job already proves PG19 compiles under `-Werror` and the ruleutils port (#8602) lands the runtime/regress fixes those suites depend on. ### Changes (`.github/workflows/build_and_test.yml`) - `test-citus`, `test-citus-failure`, `test-citus-cdc`: add `pg19_version` to the `pg_versions` array. - `test-arbitrary-configs`: add `pg19_version` to `matrix.pg_version`. - `test-pg-upgrade`: add the `18 -> 19` and `16 -> 19` upgrade pairs (adjacent + oldest -> newest, consistent with the existing 16->17 / 17->18 / 16->18 set). - `params`: bump `pg19_version` / `upgrade_pg_versions` from `19devel` to `19beta1`, and point `image_suffix` at the PG19beta1-enabled the-process images (`-dev-33b3cf5`, built from citusdata/the-process#222). ### Deliberately not changed - `test-citus-upgrade` stays at PG16/PG17 — no released Citus binary supports PG19, so there is nothing to upgrade *from*. ### Dependencies - Image set: citusdata/the-process#222 (PG19beta1 via apt) — **green**, images published under `-dev-33b3cf5`. - Runtime/regress fixes: ruleutils port #8602 plus the runtime/regress sub-issues must be on `pg19-support` for the new cells to pass. Closes #8615 Part of #8597
Stacked PR (PR2 of the PG19 stack). **Base = `pg19-ci-test-matrices` (#8616)**, which is itself stacked on `pg19-ruleutils-port` (#8602) → `pg19-support`. Review/merge in stack order. Fixes two PG19 runtime crashes that surface in normal Citus operation (outside the regression suite): 1. **`FuncnameGetCandidates()`** gained an `int *fgc_flags` out-parameter that the callee writes to unconditionally. The compat shim passed `NULL` and crashed on every by-name function lookup (e.g. the maintenance daemon). Now passes the address of an int compound literal. 2. **`TupleDesc->firstNonCachedOffsetAttr`**: PG19 added an offset cache that `BlessTupleDesc()` / `slot_deform_heap_tuple()` now assert is populated but no longer populate themselves. `BlessTupleDesc` is wrapped to call `TupleDescFinalize()` first (no-op shim on older majors), with explicit calls added at the four sites that deform hand-built TupleDescs. Also installs `ApplicationNameAssignHook` via union access on PG19. Closes #8610 Part of #8597
Stacked PR (PR3 of the PG19 stack). **Base = `pg19-runtime-fixes` (#8617)** → `pg19-ci-test-matrices` (#8616) → `pg19-ruleutils-port` (#8602) → `pg19-support`. Review/merge in stack order. PG19 enables parallel `CREATE INDEX` by default (`max_parallel_maintenance_workers` defaults to 2). Columnar's TableAM is always serial — `rs_parallel` is stored but never read, and the build path flushes pending writes, which is disallowed inside a parallel operation. This caused `CREATE INDEX` on columnar tables to fail on PG19. Three changes (all in `columnar_tableam.c`): 1. **`parallelscan_estimate`/`initialize`/`reinitialize`** now delegate to the `table_block_*` helpers, so callers that unconditionally size and initialize a `ParallelTableScanDesc` (e.g. PG19's parallel btree build) no longer abort. Columnar ignores the descriptor, so the written state is simply unused. 2. **`columnar_index_build_range_scan`** accepts a parallel scan descriptor by discarding it (`scan = NULL`) and running the serial path. 3. **`ColumnarProcessUtility`** forces `max_parallel_maintenance_workers=0` (via `NewGUCNestLevel`/`AtEOXact_GUC` around `PrevProcessUtilityHook`) for `CREATE INDEX` on a columnar AM relation, so the build runs serially. Closes #8611 Part of #8597 --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…8619) Stacked PR (PR3b of the PG19 stack). **Base = `pg19-columnar-index` (#8618)** → `pg19-runtime-fixes` (#8617) → `pg19-ci-test-matrices` (#8616) → `pg19-ruleutils-port` (#8602) → `pg19-support`. Review/merge in stack order. PG19 removed `get_relation_info_hook`, which is where pre-PG19 builds disable parallel query and index-only scans for columnar relations (columnar scans are always serial and cannot return tuples from an index). The Phase-1 build left a `TODO(PG19, #8614)` placeholder; this PR re-implements that suppression inside `ColumnarSetRelPathlistHook`: - forbid future parallel workers (`rel_parallel_workers = 0`, `consider_parallel = false`) and drop any already-generated partial paths; - clear each index's `canreturn` flags and strip any `IndexOnlyScan` paths that were already created (`set_rel_pathlist_hook` runs after path generation). The pre-PG19 `ColumnarGetRelationInfoHook` path is unchanged. Closes #8614 Part of #8597 --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## What Restores partitioned-index stripping on PostgreSQL 19, which regressed when PG19 removed the `get_relation_info_hook` that Citus relied on. This is a PG19-only planner-hook migration and a no-op on PG ≤ 18. The columnar `scan_analyze_next_tuple` signature item from the original scope was already fixed in the build-foundation PR #8601 and is intentionally **not** touched here. ## Why PG19 dropped `get_relation_info_hook` in favour of `build_simple_rel_hook`. Citus used that hook (`multi_get_relation_info_hook`) to strip partitioned indexes from partitioned-table plans. PR #8601 disabled it on PG19 with a `TODO(PG19 Phase 2)` marker, so index-stripping was temporarily absent on PG19, causing a client-backend crash on partitioned-table planning paths inside the regression suite (`make check`). ## How - Add a PG19-only `multi_build_simple_rel_hook` in `distributed_planner.c` (prototype in `distributed_planner.h`) matching the `build_simple_rel_hook` signature `(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)`. It delegates straight to the existing `multi_get_relation_info_hook` body, which re-derives the `RangeTblEntry` from `rel->relid` — so the removed `relationObjectId` / `inhparent` arguments are passed as `InvalidOid` / `false` and intentionally ignored, keeping behaviour identical across versions. - Register it via `build_simple_rel_hook` under `#if PG_VERSION_NUM >= PG_VERSION_19` in `shared_library_init.c`; the old `get_relation_info_hook` assignment is kept under the `#else` arm for PG ≤ 18. ## Files - `src/backend/distributed/planner/distributed_planner.c` - `src/include/distributed/distributed_planner.h` - `src/backend/distributed/shared_library_init.c` Item 1 (`src/backend/columnar/columnar_tableam.c`) is untouched — already fixed in #8601. ## Scope note An earlier revision of this PR also carried a `distribution_column.c` `AccessShareLock` change ("item 3"). That fix addresses a **PG16+** `relation_open()` assertion / relcache-lifetime issue that is not PG19-specific, so it has been **split out** into a separate PR targeting `main`. This PR is now scoped solely to the PG19 `build_simple_rel_hook` migration. ## Stacking Stacked on #8619 — base branch is `pg19-columnar-relinfo`. GitHub will auto-retarget this PR to `pg19-support` once #8619 merges. ## Cross-version gate - [x] **PG19 (19beta1):** full `-Werror` build green; `partitioned_indexes_create` (+ setup) regress passed, **zero diff** — exercises the partitioned-index planning path and proves the hook is active. - [x] **PG18 (18.4):** full `-Werror` build green; same focused regress passed, **zero diff** — the pre-PG19 `#else` arm is unchanged and neutral. - [x] **PG17 (17.10):** the migration is preprocessor-excluded (`#if PG_VERSION_NUM >= PG_VERSION_19`), so PG17 is structurally unaffected; the earlier full-scope gate confirmed a green PG17 `-Werror` build + regress. - [x] **check-style:** `citus_indent --check` clean on all touched files. All arms build clean under full `-Werror` (no flag demotion). The change is behaviourally confined to PG19; PG ≤ 18 is preprocessor-excluded and proven regression-neutral. Part of #8597. Relates to #8608. --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Part of #8597. Closes #8609 _(note: GitHub auto-close won't fire on a non-`main` base)._ ## Problem PG19 internalised LWLock tranche registration: `LWLockNewTrancheId()` now takes the tranche name and returns the id in one step, and the public two-step `LWLockRegisterTranche()` was removed (names now live in shared memory, visible to every backend). The PG19 foundation work (#8601) added a temporary shim that registered **every** Citus tranche under the placeholder name `"citus-deferred"`, so on PG19 every Citus lightweight-lock wait surfaced as `citus-deferred` in `pg_stat_activity.wait_event` — a diagnostics regression vs PG≤18 (ids stayed unique, so not a correctness bug, hence deferred to this PR). ## Change - Replace the `citus-deferred` placeholder shim in `pg_version_compat.h` with a single `LWLockNewTrancheIdCompat(name)` helper: - **PG19**: macro forwarding the name to one-step `LWLockNewTrancheId(name)`. - **PG≤18**: a `static inline` keeping the historical allocate-then-register two-step behind the same interface. - Thread the real per-subsystem name through at all 7 call sites and drop the standalone `LWLockRegisterTranche()` calls. - Remove the `pg_attribute_unused()` markers the foundation work added on the now-used `trancheName` locals (`shardsplit_shared_memory.c`, `backend_data.c`). - Keep the `NamedLWLockTranche` compat typedef (still required by Citus shmem structs on PG19). Call sites: `causal_clock.c`, `shared_connection_stats.c`, `shardsplit_shared_memory.c`, `cluster_changes_block.c`, `stat_tenants.c`, `maintenanced.c`, `backend_data.c`. PG≤18 behaviour is unchanged (identical allocate-then-register sequence); PG19 now reports the correct per-subsystem tranche name. ## Validation (WSL, pgenv) Each version built under full `-Werror` (no flag demotion), then a single-node functional check: `CREATE EXTENSION citus`, followed by 16-session contention on the cluster-clock LWLock (`citus_get_node_clock()`) while sampling `pg_stat_activity.wait_event`. | Version | `-Werror` build | warnings in touched files | `CREATE EXTENSION` | `wait_event` observed | `citus-deferred` | |---|---|---|---|---|---| | 17.10 | clean | none | OK | `Cluster Clock Setup Tranche` | 0 | | 18.4 | clean | none | OK | `Cluster Clock Setup Tranche` | 0 | | 19beta1 | clean | none | OK | `Cluster Clock Setup Tranche` | 0 | PG19 confirms the real per-subsystem name now surfaces (no longer `citus-deferred`); PG17/18 confirm the two-step path is regression-neutral. The one runtime-observable tranche (cluster clock) is verified directly; the other six use the identical helper and are covered by the `-Werror` build. Full multi-node regression runs in CI. --- **Draft** — held for review per the PG19 support effort coordination. --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DESCRIPTION: Normalize PostgreSQL 19 cosmetic output differences without weakening existing regression or expected-output oracles. Addresses #8659. ## Stack This PR is stacked on #8622 and targets `ihalatci-pg19-regress-crashes`. The planner crash fixes must land before these output normalizations because they expose several normalized regression paths. ## Changes Add narrowly scoped PG19 compatibility rules in `src/test/regress/bin/normalize.sed`. PG19 output is mapped back to canonical PG17/PG18 output; canonical expected files are left unchanged. The final rules preserve plan numbers, deparsed worker queries, CTE identity outside the affected line, tuple byte counts outside the two observed deltas, and generic function-resolution diagnostics outside the exact PG19 forms. ## Rule provenance | Added normalization rule | PG19 source commit | Attribution | |---|---|---| | Named `InitPlan`/`SubPlan` references and nested EXISTS numbering | `d3790f2af` | Inferred from "PG19 EXPLAIN cosmetic differences" | | Implicit `_ANY_subquery` naming | `64c9585de` | Inferred from "EXPLAIN/error output differences" | | `VACUUM ... PARALLEL` error wording | `64c9585de` | Inferred | | Partitioned-table primary-key error wording | `64c9585de` | Inferred | | Multiline `\d+` `Partitions:` lists | `0247b7ef1` | Directly documented | | `count(non-null-column)` / `count(*)` plan output | `d3790f2af` | Inferred from EXPLAIN changes | | Inlined CTE name suffix | `d3790f2af` | Inferred from EXPLAIN changes | | Exact tuple-data byte deltas | `2119b6796` | Directly documented | | Function-resolution `HINT`/`DETAIL` differences | `2119b6796` | Directly documented | | Extra PL/pgSQL function context | `11b27a791` | Directly documented | | Temporary-sequence dependency `DETAIL` | `11b27a791` | Directly documented | | Implicit foreign-key name suffix | `11b27a791` | Directly documented | | Append-partitioned shard length `139264` -> `131072` | `11b27a791` | Directly documented | The original Git objects are unavailable on the current remotes and searched local/WSL repositories. The split between `64c9585de` and `d3790f2af` is therefore inferred from their documented subjects; the other mappings are explicitly supported by the local PG19 plan. ## Validation - PostgreSQL 17.10, 18.4, and 19beta1 build cleanly with `CFLAGS=-Werror`. - PG17/PG18 `check-multi`, `check-multi-1`, and `check-multi-mx` pass apart from the existing `multi_extension.out` MX baseline. - PG19 `check-multi-1` passes. `check-multi` leaves only the four planned downstream files (`clock.out`, the two order-by files, and `multi_subquery_in_where_reference_clause.out`); `check-multi-mx` leaves only `multi_extension.out` and planned `multi_mx_explain.out`. - Applying the final normalizer to all 835 canonical expected files changes zero files. - Replaying PG19 CI artifacts resolves 21 intended result diffs: 12 in `check-multi`, 7 in `check-multi-1`, 1 in `check-multi-mx`, and 1 in `check-split`. --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… (#8624) Part of #8597 (PostgreSQL 19 support). Addresses #8613 — dispatch and discriminate the PG19 unified REPACK/CLUSTER command (`T_RepackStmt`). > Note: the base branch of this PR is `pg19-support` (the PG19 integration branch), not `main`, so GitHub will **not** auto-close #8613 on merge — it is tracked manually. ## What & why PG19 removed `ClusterStmt`/`T_ClusterStmt` and replaced them with the unified `RepackStmt`, which backs three commands distinguished by `RepackStmt.command`: `CLUSTER`, the new `REPACK`, and `VACUUM FULL`. (VACUUM FULL still dispatches through `T_VacuumStmt`, so it never reaches this path.) The foundation PR (#8601) added a compile-only shim aliasing `T_ClusterStmt` → `T_RepackStmt`; that makes the names compile but is not correct behaviour, because CLUSTER and REPACK now collide on a single node tag and must be told apart. This PR makes Citus dispatch and propagate the command correctly on PG19, while staying byte-for-byte regression-neutral on PG17/PG18. ## Approach (Option A) Citus propagates **REPACK exactly like CLUSTER**: the command is shipped to every shard placement through the existing CLUSTER code path. The worker name-relay (`RelayEventExtendNames` → `AppendShardIdToName`) mutates the parse tree in place, appending the shardId to **both** the target relation name **and** the index name (when `USING INDEX` is given), then `ProcessUtilityParseTree` executes the mutated node — there is no deparse-to-string step, so the existing relabel localizes REPACK's names with no new deparser work. CLUSTER vs REPACK are discriminated by `RepackStmt.command`, and the user-facing WARNING/ERROR wording is command-aware. ## Files - `src/include/pg_version_compat.h` — RepackStmt shim + version-portable helpers `ClusterStmtIsRepack()` / `ClusterStmtCommandName()` (compiled out on `< PG19`). - `src/backend/distributed/commands/cluster.c` — `PreprocessClusterStmt` is command-aware (correct REPACK vs CLUSTER wording in the no-relation WARNING, the partitioned WARNING, and the VERBOSE ERROR). - `src/backend/distributed/commands/distribute_object_ops.c` — `GetDistributeObjectOps` routes both CLUSTER and REPACK (shared `T_ClusterStmt`/`T_RepackStmt` tag). - `src/backend/distributed/relay/relay_event_utility.c` — name-relay reads the RepackStmt layout (relation + index name). - `src/test/regress/sql/pg19.sql`, `expected/pg19.out`, `expected/pg19_0.out` — new PG19-only acceptance test (+ the `< PG19` early-quit variant). - `src/test/regress/multi_1_create_citus_schedule` — registers the `pg19` test. ## Acceptance test (PG19-only, `pg19.sql`) The test distributes `repack_test` (4 shards, replication factor 1) and **demonstrates** (not merely asserts) that distributed REPACK works: - `REPACK <t> USING INDEX <idx>`, bare `REPACK <t>`, and `CLUSTER <t> USING <idx>` each **rewrite every shard placement** — the relfilenode changes on all shards (verified via `run_command_on_shards`). - **Distribution-invariance** after the rewrites — proves REPACK changes *storage*, not *distribution semantics*: - `placements_unchanged` — shard set + placement nodes identical before/after (symmetric `EXCEPT`). - `shard_count_unchanged` — shard count identical before/after. - `shard_row_mapping_unchanged` — per-shard `count(*)` identical before/after (no row crossed a shard boundary). - `distribution_unchanged` — `(partmethod, partkey, colocationid)` intact. - routing still works — router queries `a = 42 → (42, 2)` and `a = 100 → (100, 0)`; cross-shard aggregate `count = 100, sum = 5050`. - Command-aware messaging — `VERBOSE` → REPACK/CLUSTER-worded ERROR; partitioned distributed table → `not propagating REPACK command for partitioned table to worker nodes` WARNING. The `< PG19` early-quit path keeps PG17/PG18 on the unchanged `pg19_0.out`, so this test is invisible to older majors. ## Validation gate (WSL, pgenv, full `-Werror`, no demotion) | Version | Build (`-Werror`) | Regress | |---|---|---| | PG17.10 | clean | 4/4, no diffs (pg19 early-`\q` → unchanged `pg19_0.out`) | | PG18.4 | clean | 4/4, no diffs (pg19 early-`\q` → unchanged `pg19_0.out`) | | PG19beta1 | clean | 4/4 — distributed-REPACK acceptance test green (all invariance assertions `t`) | CLUSTER's existing tests/expected (`multi_index_statements`, `pg15`) are unchanged on all versions. --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8fc5a222-5fb6-48fd-9b27-cd0a418201d2
## Summary - make the partition-to-partition `fkey` and partition-to-reference-table `fkey_to_ref` checks mutually exclusive - use one cross-version expected output for PostgreSQL 17, 18, and 19 - remove the now-redundant numbered alternative expected output - leave `normalize.sed` and production code unchanged ## Coverage fix The test previously used `(id, value_1) = (0, 5)` while both foreign keys were unsatisfied. PostgreSQL 17/18 reported `fkey` first, while PostgreSQL 19beta2 reported `fkey_to_ref` first, so the row did not independently prove either check. The revised sequence makes each failure unambiguous: 1. Seed reference key `5`, then insert `(0, 5)` before partition `referencing_table_0` contains `id = 0`. Only `fkey` is unsatisfied: ```text foreign key constraint "fkey_xxxxxxx" Key (id)=(X) is not present in table "referencing_table_0_xxxxxxx". ``` 2. Insert `(0, 1)` into partition `referencing_table_0`, satisfying `fkey` for `id = 0`. Then insert `(0, 4)`, where reference key `4` is absent. Only `fkey_to_ref` is unsatisfied: ```text foreign key constraint "fkey_to_ref_xxxxxxx" Key (value_1)=(4) is not present in table "referenced_table_xxxxxxx". ``` 3. Insert `(0, 5)` successfully once both constraints are satisfied. ## Validation | PostgreSQL | Focused test | Expected output | |---|---|---| | 17.10 | Passed | Primary, byte-for-byte | | 18.4 | Passed | Primary, byte-for-byte | | 19beta2 | Passed | Primary, byte-for-byte | Each leg used a fresh Citus build with absolute PostgreSQL installation paths. The focused schedule included its required helper tests and all 5 tests passed. All three normalized results were identical (`sha256 8b6ab879b0581fed4fd1efbe023d896c00513c42f1d0b95ad8edddf523ae3527`). The primary output is normalization-idempotent, and `git diff --check` passes. Closes #8669 > [!NOTE] > This draft targets the non-default `pg19-support` branch. GitHub's closing keyword takes effect only when the change reaches the repository's default branch. --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Copilot-Session: 2a53d4d7-c12a-41cc-96ea-46be67eff49d
## Summary - suppress planner cost estimates in the two `clock` EXPLAIN checks that assert `cc_idx` usage - order all nine previously unordered PG17 partition-listing queries by `partition_name` - update only the corresponding regression expected output ## Validation All validation used WSL and absolute PostgreSQL installation paths. - PostgreSQL 17.10: `CFLAGS=-Werror` build/install; focused `clock` (7/7) and `pg17` (9/9) passed - PostgreSQL 18.4: `CFLAGS=-Werror` build/install; focused `clock` (7/7) and `pg17` (9/9) passed - PostgreSQL 19beta2: `CFLAGS=-Werror` build/install; focused `clock` (7/7) and `pg17` (9/9) passed - `citus_indent --quiet --check` - `git show --check` Closes #8668. GitHub will only auto-close the issue once this change becomes reachable from the default branch. Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Copilot-Session: 17c170a6-c9fa-40c1-b7af-b060988224fc
Stabilize PostgreSQL 19 ORDER BY pushdown plan assertions ## Context PostgreSQL BUG #19517 caused eager aggregation on semi/antijoin inner relations to return incorrect results on 19beta1 (`5|130`). Upstream commit `ffeda04259bb` fixes that bug, and the same reproducer returns the correct `5|26` on 19beta2. This PR therefore does not disable `enable_eager_aggregate` or `enable_hashagg` globally or in the regression harness. The H4 case in `multi_orderby_pushdown` is independent of that correctness fix. On 19beta2, with eager aggregation at its default, it still chooses a worker `Nested Loop` plan instead of the asserted `Sort -> Hash Semi Join` shape. Both `Citus Sorted Merge Adaptive` nodes remain, so pushdown itself is intact. This change stabilizes only the H4 EXPLAIN by wrapping it in a transaction and propagating transaction-local planner settings: ```sql BEGIN; SET LOCAL citus.propagate_set_commands TO 'local'; SET LOCAL enable_nestloop TO off; -- existing H4 EXPLAIN COMMIT; ``` The expected plan is otherwise unchanged and continues to assert sorted-merge pushdown and the worker hash semi-join shape. There are no production-code or regression-harness changes. ## Validation - PostgreSQL 17.10: `-Werror` build and focused `multi_orderby_pushdown` path, 15/15 passed. - PostgreSQL 18.4: `-Werror` build and focused path, 15/15 passed. - PostgreSQL 19beta2: `-Werror` build and focused path, 15/15 passed with default `enable_eager_aggregate=on`. - PostgreSQL 19beta2 upstream regression suite: 245/245 passed, including `eager_aggregate`. - PostgreSQL 19beta2 Citus semi/antijoin path: `multi_subquery_in_where_reference_clause`, 15/15 passed with eager aggregation enabled. - Perl syntax, style, and diff whitespace checks passed. Closes #8666 > This draft targets the non-default `pg19-support` branch. GitHub may defer automatic issue closure until the change reaches the default branch. --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Copilot-Session: bd1ff9e9-2f27-43d3-b9d7-f7d167e7e3c4
## Summary - disable `citus.allow_aggregate_worker_combine_on_internal_types` in `tdigest_aggregate_support` so the regression test continues to assert tdigest-specific distributed plan shapes instead of the generic internal-state worker-combine path - update the primary expected output and the existing `_0`/`_1` alternatives with the new `SET` echo - keep the existing alternative semantics: `_0` covers extension-absent output and `_1` covers accepted tdigest result-value variation - account for PostgreSQL 19 Beta 2's undefined-function `DETAIL` through the normalization already present on the refreshed `pg19-support` base (`ca86a5d95`); a separate `_2` alternative was validated during development but removed because that base normalization makes it redundant and unreachable ## Validation - `-Werror` builds: PostgreSQL 17.10, 18.4, and 19 Beta 2 - focused `tdigest_aggregate_support` schedule with tdigest installed: passed on PostgreSQL 17.10, 18.4, and 19 Beta 2 - focused schedule without tdigest: PostgreSQL 17.10 and 18.4 matched `_0`; PostgreSQL 19 Beta 2's raw `DETAIL` normalized and matched `_0` - `make check-style` - `git diff --check` Closes #8667 > This PR intentionally targets the non-default `pg19-support` branch. GitHub may not automatically close #8667 when this merges there; closure should be confirmed when the support branch is subsequently integrated into the default branch. Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 242038e7-c2b1-4341-b25d-a9f69a4e9981
## Summary - exclude PostgreSQL 19 packaging tests for Oracle Linux 8 and AlmaLinux 8, where PGDG packages are unavailable - exclude PostgreSQL 19 packaging tests for Ubuntu focal, while preserving the existing PostgreSQL 18 exclusion - keep PostgreSQL 19 coverage for AlmaLinux 9, Debian bullseye/bookworm, and Ubuntu jammy ## Validation - parsed the workflow YAML - verified the expanded PostgreSQL 19 matrix retains only supported packaging images - verified the branch is exactly one commit ahead of `pg19-support` with a one-file diff Tracks #8731. Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> # Conflicts: # .github/workflows/build_and_test.yml
Recognize PostgreSQL 19 property-graph dependency object classes in Citus compatibility code. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a1782b0d-8a8c-4d1a-9d71-9c58afe11a7d
Stabilize PostgreSQL 19 planner-sensitive output and make changed-test schedule dependencies repeat-safe. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a1782b0d-8a8c-4d1a-9d71-9c58afe11a7d
Resolve keyword RoleSpec targets safely in REASSIGN OWNED and cover multi-node propagation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a1782b0d-8a8c-4d1a-9d71-9c58afe11a7d
Normalize PostgreSQL 19 unnamed subquery labels and update the canonical expected output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a1782b0d-8a8c-4d1a-9d71-9c58afe11a7d
Fix PG19 publication exclusion deparsing, metadata activation, late distribution, and OID-rewrite membership restoration, with reviewed reference-table coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a1782b0d-8a8c-4d1a-9d71-9c58afe11a7d
Mark the required Citus metadata catalogs accessible to PG19 logical decoding and intentionally preserve that runtime invariant across extension downgrade. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a1782b0d-8a8c-4d1a-9d71-9c58afe11a7d
Closes #8732. ## Summary - pin `pglz` only around the heap TOAST-size assertion in `columnar_insert` - extend the existing EXPLAIN normalizer for PG19 `SubPlan any_N` names - add the PG19 alternate `VACUUM FULL` status output for `columnar_vacuum_vs_insert` All three failures are PostgreSQL output/configuration drift that predates `REL_19_BETA1`, not Beta2 regressions or columnar compatibility defects: - PostgreSQL `34dfca293432` made LZ4 the default TOAST compression when available - PostgreSQL `8c49a484e8eb` introduced textual subplan names such as `any_1` - PostgreSQL `ac58465e0618` introduced REPACK and the `repacking ... in physical order` wording Direct Beta1 and Beta2 runs reproduce the same behavior for all three cases. ## Tests - PG17.10: `check-columnar-minimal EXTRA_TESTS="columnar_insert columnar_chunk_filtering"`; `check-columnar-isolation` - PG18.4: `check-columnar-minimal EXTRA_TESTS="columnar_insert columnar_chunk_filtering"`; `check-columnar-isolation` - PG19beta2: `check-columnar-minimal EXTRA_TESTS="columnar_insert columnar_chunk_filtering"`; `check-columnar-isolation` Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary - send WAL receiver apply-status replies once per second in the Perl follower-cluster test harness - match the existing Python test harness behavior - preserve PostgreSQL and Citus production defaults ## Provenance - published from the explicitly approved commit `5e4ddc836a95df6cea9cceef33cb44d48559b736` - based directly on `pg19-support` at `ff8f1f8c46de4c3ed094c99796c9805529d1a328` - contains only the one-line `wal_receiver_status_interval=0` to `1` test-harness change ## Testing - PG19beta2 before fix: `make -C src/test/regress check-follower-cluster` timed out after 180 seconds while creating the follower topology - PG17.10 after fix: 8/8 follower-cluster tests passed - PG18.4 after fix: 8/8 follower-cluster tests passed - PG19beta2 after fix: 8/8 follower-cluster tests passed Closes #8736 Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Reconstruct partitioned-root publications from explicit catalog identities while preserving runtime leaf semantics, with expanded reviewed coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a1782b0d-8a8c-4d1a-9d71-9c58afe11a7d
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8753 +/- ##
==========================================
+ Coverage 88.69% 88.74% +0.05%
==========================================
Files 289 289
Lines 64889 65070 +181
Branches 8180 8201 +21
==========================================
+ Hits 57553 57747 +194
+ Misses 4973 4959 -14
- Partials 2363 2364 +1 🚀 New features to boost your workflow:
|
This was referenced Aug 15, 2026
Allow only the output plugins exercised by PG19 test clusters and remove the reverted GROUP BY ALL deparser access. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Pin the PG19 build and upgrade path to beta3 and use the matching development image. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Several CDC cases exercise nonblocking shard operations that create logical slots using the citus output plugin. Trust it only in PG19 CDC test clusters alongside pgoutput and wal2json. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Quote EXTRACT field identifiers in the PG19 ruleutils copy so crafted field names cannot escape into a second statement in distributed task SQL. Add a PG19 regression that exercises a malicious field name and verifies no worker-side object is created. Ported from PostgreSQL security commit a3832a7571013469dc81e35d0fe1c0066e86d893 (CVE-2026-15741). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ibrahim halatci (ihalatci)
changed the title
Fix PG19 relation JSON COPY TO routing. ## Problem Relation `COPY ... TO STDOUT (FORMAT json)` used Citus's direct shard COPY fast path. Concatenating shard-formatted JSON produced multiple arrays instead of one valid JSON document for `FORCE_ARRAY true`. The fast path also reported incorrect COPY protocol metadata and command tuple counts, especially for empty results. ## Fix - exclude `FORMAT json` from the direct `CitusCopyTo()` path so PostgreSQL formats one coordinator-owned JSON stream through `CitusCopySelect()` - construct the coordinator SELECT from `CopyGetAttnums()` so explicit normal columns preserve order while generated, system, missing, and duplicate columns retain native relation COPY validation - cover multi-shard, empty, single-shard, reference, local, Citus-local, streaming, explicit-column, escaping/null, encoding, option-error, and low-level libpq protocol behavior This intentionally trades direct shard streaming for coordinator query execution. Large JSON COPY operations may therefore use coordinator batching or spill, but PostgreSQL must own the whole JSON document framing and protocol state for correctness. ## Validation - PostgreSQL 16.14, 17.10, 18.4, and 19beta2 builds with `CFLAGS=-Werror` - `multi_copy` regression: 15/15 passed on PostgreSQL 16, 17, and 18 - combined `multi_copy` and `pg19` regression: 16/16 passed on PostgreSQL 19 - focused low-level libpq protocol/equivalence test: passed - targeted Black, isort, flake8, `citus_indent`, and diff checks: passed Closes #8760 --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1aa55534-88b0-4ef8-91b5-adba3eec71b1
## Summary Preserve a table `GRANT`/`REVOKE` statement's explicit `GRANTED BY` role when Citus deparses and propagates the command to physical shards. This PR is intentionally limited to table-grantor serialization in `commands/grant.c`. It is separate from #8766, which fixes shared-object `REVOKE` clause ordering in the deparser. The two are complementary halves of the same report and must not be conflated. Tracks #8759 Umbrella tracking: #8597 ## Root cause `PreprocessGrantStmt()` hand-builds the worker DDL string rather than routing through the deparser, and it never emitted `GrantStmt.grantor`. The explicit grantor was therefore silently dropped on the way to every worker. PostgreSQL 19 (commit `dd1398f1`) widened `GRANTED BY` to accept **any role the acting role inherits**, not just `current_user`. Once the grantor is dropped from the worker command, each worker independently runs `select_best_grantor()` and can pick a different role than the coordinator did. The result is a coordinator/shard ACL divergence. On PG16-18 the same statement requires `grantor = current_user`, so the omission is unobservable there. This is why the defect is only reachable, and only testable, on PG19. ## Impact Once the ACLs diverge, a later `REVOKE` that names the coordinator's grantor matches nothing on the shards. Concretely, the coordinator reports the privilege as revoked while the physical shards still grant it — a silent privilege leak that `has_table_privilege()` on the coordinator will not reveal. ## Fix `commands/grant.c` only, +16/-4. Builds a `grantedBy` fragment when `grantStmt->grantor` is set and appends it to both format strings. In `REVOKE` it lands after the grantee list and before `CASCADE`/`RESTRICT`, which is the same ordering #8766 establishes for shared objects, so the two scopes agree by construction. `GrantStmt.grantor` has existed since PG14, so no version gate is needed; the code is inert on PG16-18 where the field cannot be set to anything but the current role. ## Test design The scenario is deliberately discriminating. A third-party role owns the table and is **never named as grantor**, so the non-discriminating `owner == grantor` case cannot mask a failure. Two eligible grantors are inherited by the acting role, and the statement explicitly names the one the workers would *not* choose on their own. Coverage: - coordinator vs physical-shard `aclexplode(relacl).grantor` parity, read through `run_command_on_shards()` - selective-revoke parity via `has_table_privilege()` on coordinator and shards - a quoted grantor role name (`"Grant Owner"`), exercising `RoleSpecString(..., true)` - `WITH GRANT OPTION` and the matching `REVOKE GRANT OPTION FOR` Tests are gated on `server_version_ge_19` and are pure additions: zero removed lines in either `pg19.sql` or `pg19.out`. ## Negative control Reverting `grant.c` to stock, rebuilding, reinstalling and re-running turns `pg19` red. Verified twice in independent cycles, the second time with a binary-level gate confirming the built `citus.so` actually matched the intended source in each phase (`strings citus.so | grep -c 'GRANT %s ON %s TO %s%s%s'` -> stock 0, fixed 1). Four assertions flip, and **every one of them is shard-side**. All coordinator rows are byte-identical between the two runs and appear only as unchanged context: | assertion | stock (broken) | with fix | |---|---|---| | shard grantor | `pg19_grantor_a` | `pg19_grantor_b` | | shard privilege after selective revoke | `f` (coordinator reads `t`) | `t` | | shard grant option after `GRANT ... WITH GRANT OPTION` | `pg19_grantor_a, grantable=true` | `"Grant Owner", grantable=true` | | shard grant option after `REVOKE GRANT OPTION FOR` | `pg19_grantor_a, grantable=false` | `"Grant Owner", grantable=false` | The sharpest single line is row 2: `coordinator_privilege = t` sits two lines above `shard_privilege = f` as unchanged context in the same diff hunk. **Disclosed limitation, stated plainly:** rows 3 and 4 discriminate on grantor *identity* only, not on the `grantable` flag. Unfixed, the shard ACL entry and the un-attributed revoke both resolve to `pg19_grantor_a` and therefore coincide, so the grant option is still correctly dropped. Rows 1 and 2 carry the proof; rows 3 and 4 are corroborating. ## Validation - base is exactly `origin/pg19-support` @ `db67cd7e2` with zero intervening delta, so the validated tree is the pushed tree - 3 files, +305/-4; the two test files are append-only - `citus_indent --check` clean (exit 0, no `FAIL` lines) - full-cluster `pg19` regression on a 3-node PostgreSQL 19beta2 harness: `ok` with the fix, zero diff - `expected/pg19.out` is harness-generated, never hand-written, and re-verified byte-identical to a freshly generated `results/pg19.out` Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Summary PostgreSQL psql change `d6ab88d`, first present in PG19 Beta3, changed failed COPY input draining. These four failure injections abort before COPY_IN, so inline `\copy ... FROM STDIN` input can consume following SQL in psql echo-all regression scripts. - replace the three affected `failure_copy_to_reference` inputs and one affected `failure_copy_on_hash` input unconditionally with zero-row client-side `FROM PROGRAM 'true'` - keep the server-side Citus failure injection, errors, placement checks, and row-count assertions unchanged - update only the four corresponding echoed commands in the canonical expected files - use no version gate, alternative expected output, harness change, normalization, or global machinery `\copy ... FROM PROGRAM` is client-side and works consistently across the supported clients tested here, while supplying no rows for failures that occur before COPY_IN. ## Validation Exact commit/tree validated: `4ee87f66cf0bfbff7447df0e5da9dc24e9f30fd1` / `aef35426123a65d79cda324a583736fab67b1cf8`. Focused affected schedule: - PG19 Beta3: 6/6 - PG19 Beta2: 6/6 - PG18.4: 6/6 Full `check-failure`: - PG19 Beta3: 38/38 (94 seconds) - PG19 Beta2: 38/38 (99 seconds) - PG18.4: 38/38 (105 seconds) - PG17.10: 38/38 (95 seconds) - PG16.14: 38/38 (93 seconds) PG19 Beta2 validation used the existing isolated local compatibility shim that omits the Beta3-only `output_plugin_libraries` test setting; the shim is not part of this PR and was restored after validation. Final diff: 4 files, 8 insertions, 8 deletions. Closes #8780 --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Copilot-Session: 809b79c3-b004-47c3-8e1d-4ba1af5b9eb5
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
2 participants
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: Enables pg19 support for Citus