PG19: build foundation (source-level compat, scaffold ruleutils_19.c, CI image bump) - #8601
Conversation
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. PG19 is intentionally not added to the regression, failure, cdc, or upgrade jobs yet (those need the upstream-derived ruleutils_19.c that lands in a follow-up).
Accept PG19 in configure/pg_version_constants.h and add centralised source-compat shims in pg_version_compat.h. Adapt distributed/columnar headers for renamed/retyped PG19 APIs. Add 128 KB shmem slack on PG19 to absorb ShmemInitHash up-front allocation, and drop a pre-existing hash double-count in maintenanced.c exposed by that change.
Source-level fixes across the distributed and columnar code to build clean under PG19's stricter warnings (old-style prototypes, strict forward declarations, implicit-fallthrough annotations, new switch arms, API renames). The build keeps the same warning/error flags as PG17/PG18 -- warnings are fixed at the source, not silenced.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## pg19-support #8601 +/- ##
================================================
- Coverage 88.76% 88.75% -0.01%
================================================
Files 288 288
Lines 64302 64316 +14
Branches 8091 8091
================================================
+ Hits 57075 57086 +11
- Misses 4888 4890 +2
- Partials 2339 2340 +1 🚀 New features to boost your workflow:
|
751c742 to
c1df190
Compare
Scaffold cloned from ruleutils_18.c retargeted at the PG19 version guard, plus the minimal source changes required for it to compile under PG19's -Werror (the real upstream-derived deparse port lands in a follow-up PR, per the #7725/#8010 precedent): - get_name_for_var_field(): handle the new PG19 RTE_GRAPH_TABLE rtekind in the switch (grouped with the subquery cases, as upstream does). - isSimpleNode(): replace the comment-style /* FALLTHROUGH */ with pg_fallthrough; (PG19 builds with -Wimplicit-fallthrough=5, which no longer accepts comment markers). - generate_function_name(): pass the new &fgc_flags argument to func_get_detail(), which gained a parameter in PG19. Exclude the file from citus-style in .gitattributes: it is an upstream-derived clone exceeding the CI uncrustify 10,000-line limit.
c1df190 to
ab238fc
Compare
| pg16_version: '{ "major": "16", "full": "16.14" }' | ||
| pg17_version: '{ "major": "17", "full": "17.10" }' | ||
| pg18_version: '{ "major": "18", "full": "18.4" }' | ||
| pg19_version: '{ "major": "19", "full": "19devel" }' |
There was a problem hiding this comment.
Does this mean that PG19 breakage will block unrelated PRs until full Citus support is added?
There was a problem hiding this comment.
not until we merge this to main. Intentionally keeping it on separate branch now. We can discuss when would be the best time to merge to main
| 17: "13.0.1", | ||
| 18: "15.0devel", | ||
| 18: "14.1.0", | ||
| 19: "15.0devel", |
There was a problem hiding this comment.
Is the change of PG18 intentional? This is the standard reassignment for each new PG version?
There was a problem hiding this comment.
yes, I think we missed to update this when we released Citus 14.0
Colm (colm-mchugh)
left a comment
There was a problem hiding this comment.
I've some basic questions, but lgtm overall.
- shared_library_init.c: add TODO(PG19 Phase 2) at the gated-off get_relation_info_hook, referencing #8608 (mirrors columnar_customscan.c). - columnar_customscan.c: reference #8608 from the existing PG19 Phase 2 TODO. - pg_version_compat.h: reword the ShmemInitHash comment to explain the removed init_size param; reference #8609 from the LWLockNewTrancheId 'citus-deferred' placeholder shim. - citus_ruleutils.c: hoist standard_conforming_strings out of the per-character loop in simple_quote_literal so the PG19 GetConfigOption macro is evaluated once per literal instead of once per byte.
06b1d07
into
pg19-support
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`).
## 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>
… (#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>
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 cleanCREATE EXTENSION cituswhile keeping PG17 and PG18 builds green.Regression-test correctness and PG19 feature integration land in
follow-up PRs against the
pg19-supportdevelopment branch.Commits
image_suffixto-dev-e11d99c(PG19-capable testimages) and PG minors to 16.14/17.10/18.4; add
pg19_versionandwire
19develinto the build matrix only.configure/pg_version_constants.h; centralised compat shims inpg_version_compat.h; distributed/columnar header adaptations forrenamed/retyped PG19 APIs; 128 KB shmem slack on PG19 for the new
ShmemInitHashup-front allocation, plus themaintenanced.cdouble-count fix that change exposes.
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.
ruleutils_18.cretargeted atthe PG19 version guard so the binary links (per the PG17 compatibility: ruleutils #7725 / Pg18 ruleutils adaptation #8010
precedent; the real upstream-derived port lands in a follow-up PR).
Excluded from citus-style via
.gitattributes— it is anupstream-derived clone exceeding the CI uncrustify 10,000-line limit.
Scope / non-goals
test-citus,test-citus-failure,test-citus-cdc, or the upgrade jobs: theregression suite needs the upstream-derived
ruleutils_19.cfrom thefollow-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.cis touched only for theInstrumentation/NodeInstrumentationrename; a separateTupleDescFinalizeruntimefix lands in a later PR.
Validation
CREATE EXTENSION citussucceeds on PG19.Refs: #8597