Skip to content

PG19: build foundation (source-level compat, scaffold ruleutils_19.c, CI image bump) - #8601

Merged
ibrahim halatci (ihalatci) merged 6 commits into
pg19-supportfrom
pg19-build-foundation
Jun 6, 2026
Merged

PG19: build foundation (source-level compat, scaffold ruleutils_19.c, CI image bump)#8601
ibrahim halatci (ihalatci) merged 6 commits into
pg19-supportfrom
pg19-build-foundation

Conversation

@ihalatci

Copy link
Copy Markdown
Contributor

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 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 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

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

codecov Bot commented May 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.63636% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.75%. Comparing base (dee8ec1) to head (cef7e4a).

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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.
Comment thread src/backend/distributed/shared_library_init.c
Comment thread src/include/pg_version_compat.h
Comment thread src/include/pg_version_compat.h
Comment thread src/include/pg_version_compat.h Outdated
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" }'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that PG19 breakage will block unrelated PRs until full Citus support is added?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the change of PG18 intentional? This is the standard reassignment for each new PG version?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I think we missed to update this when we released Citus 14.0

@colm-mchugh Colm (colm-mchugh) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@ihalatci
ibrahim halatci (ihalatci) merged commit 06b1d07 into pg19-support Jun 6, 2026
128 of 135 checks passed
@ihalatci
ibrahim halatci (ihalatci) deleted the pg19-build-foundation branch June 6, 2026 12:44
ibrahim halatci (ihalatci) added a commit that referenced this pull request Jun 16, 2026
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`).
ibrahim halatci (ihalatci) added a commit that referenced this pull request Jul 16, 2026
## 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>
ibrahim halatci (ihalatci) added a commit that referenced this pull request Jul 16, 2026
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&le;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&le;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&le;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>
ibrahim halatci (ihalatci) added a commit that referenced this pull request Jul 16, 2026
… (#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Citus does not build on PostgreSQL 19

2 participants