Pg18 ruleutils adaptation - #8010
Merged
Mehmet YILMAZ (m3hm3t) merged 17 commits intoJun 20, 2025
Merged
Conversation
Mehmet YILMAZ (m3hm3t)
changed the title
Mehmet YILMAZ (m3hm3t)
force-pushed
the
m3hm3t/pg18_support
branch
from
June 12, 2025 11:11
e7a3f1c to
6220ff5
Compare
…d53c1552efe85c3e8120356c22cd658163
d673eefd410cd7f5f95e418990146189f44811af
a93e2a1e25a6d5410abb1446637c4d9a4f24e35d
80feb727c869cc0b2e12bd1543bafa449be9c8e2
984410b923263cac901fa81e0efbe523e9c36df3
8b1b342544b69b281ffd3aafe594aec629ec4d3c
29f7ce6fe78e3f8d520764b5870453d791a3ca65
Mehmet YILMAZ (m3hm3t)
force-pushed
the
m3hm3t/pg18_ruleutils
branch
from
June 12, 2025 11:22
de951a5 to
1d32b38
Compare
…ion in planner files
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## m3hm3t/pg18_support #8010 +/- ##
=======================================================
- Coverage 89.14% 89.12% -0.03%
=======================================================
Files 284 284
Lines 61593 61595 +2
Branches 7707 7707
=======================================================
- Hits 54909 54895 -14
- Misses 4458 4472 +14
- Partials 2226 2228 +2 🚀 New features to boost your workflow:
|
Mehmet YILMAZ (m3hm3t)
marked this pull request as ready for review
June 12, 2025 11:51
52c707483ce4d0161127e4958d981d1b5655865e
247dea89f7616fdf06b7272b74abafc29e8e5860
| has_anonymous = true; | ||
| } | ||
|
|
||
| /* We're now done needing the colinfo's names_hash */ |
Contributor
There was a problem hiding this comment.
This seems to be put in the wrong place. Check the commit again - in front of it there is a line "changed_any = true;", not "has_anonymous = true;"
postgres/postgres@52c7074#diff-6c12827097e95912476267d73f3f56e8250b13aad37ab4286c3c7fdab7f30ec7R4480
Contributor
Author
There was a problem hiding this comment.
/* We're now done needing the colinfo's names_hash */
destroy_colinfo_names_hash(colinfo);
added right after where for loop ended. I thought that I should also add same place.
/*
* Remember if there is a reference to an anonymous column as named by
* char * FigureColname(Node *node)
*/
if (!has_anonymous && strcmp(real_colname, "?column?") == 0)
has_anonymous = true;
part seems that came from citus not from pg source.
Contributor
There was a problem hiding this comment.
You are right. Sorry about it.
Contributor
There was a problem hiding this comment.
- First of all, let's remove changes that are not in ruleutils_18.c - this PR should be only for ruleutils changes, let's keep it clean. You can include those in a separate PR.
I checked all the ruleutils commits one by one, please take care of the unchecked boxes:
- Nothing to do for postgres/postgres@218527d
- 🔴 postgres/postgres@247dea8
missing change, couldn't see:
case RTE_GROUP:
/*
* We couldn't get here: any Vars that reference the RTE_GROUP RTEAdd commentMore actions
* should have been replaced with the underlying grouping
* expressions.
*/
break;
-
🔴 postgres/postgres@52c7074
one change misplaced, left a comment #8010 (comment) -
Nothing to be done for postgres/postgres@fc0438b
-
Nothing to be done for postgres/postgres@89f908a
-
Nothing to be done for postgres/postgres@14e87ff
-
Changes needed for postgres/postgres@94131cd are complete
-
Nothing to be done for postgres/postgres@7a80e38
Naisila Puka (naisila)
suggested changes
Jun 18, 2025
Contributor
There was a problem hiding this comment.
Continuation: I checked all the ruleutils commits one by one, please take care of the unchecked boxes:
- 🔴 Minor, but we need to update copyright for 2025 postgres/postgres@50e6eb7
- Nothing to be done for postgres/postgres@ca87c41
- 🔴 postgres/postgres@d673eef need to open a separate PR for change from
nametoStringtype, since it's not related to PG18 support. - All done for postgres/postgres@a93e2a1
- 🔴 postgres/postgres@80feb72 the following change is missing in the PR
if (rte->rtekind == RTE_FUNCTION && rte->functions != NIL)
{
/* Since we're not creating Vars, rtindex etc. don't matter */
expandRTE(rte, 1, 0, VAR_RETURNING_DEFAULT, -1,Add commentMore actions
true /* include dropped */ , &colnames, NULL);
}- All done for postgres/postgres@984410b
- All done for postgres/postgres@8b1b342
- 🔴 need to fix the typo from postgres/postgres@78eda9e
- All done for postgres/postgres@29f7ce6
Contributor
Author
When added that part "case label value has already appeared in this switch at line 4856C/C++(1578)" Should I remove existing one and add one came with 18? |
50e6eb731d98ab6d0e625a0b87fb327b172bbebd
Contributor
Author
Do you mean that a PR will merge direct to main branch? Also I assume I need remove related commit from the this PR right? |
78eda9e2648ab3f8c0df0d4aca53eb0ad6399c59
Contributor
Author
|
Contributor
Author
|
Contributor
Yeah, exactly. Basically, it's not a change that is coming from PG18 so it would be a bit misleading to include it in PG18 support. |
Contributor
Author
I assume it is here. Am I wrong? |
Naisila Puka (naisila)
approved these changes
Jun 19, 2025
Naisila Puka (naisila)
left a comment
Naisila Puka (naisila)
left a comment
Contributor
There was a problem hiding this comment.
Mostly looks good to me.
Leave it up to you to take care of the following:
postgres/postgres@80feb72 the following change is not part of the PR, but already in pg18_support branch
if (rte->rtekind == RTE_FUNCTION && rte->functions != NIL)
{
/* Since we're not creating Vars, rtindex etc. don't matter */
expandRTE(rte, 1, 0, VAR_RETURNING_DEFAULT, -1,Add commentMore actions
true /* include dropped */ , &colnames, NULL);
}
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jun 20, 2025
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jul 1, 2025
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jul 1, 2025
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jul 1, 2025
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jul 1, 2025
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jul 1, 2025
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jul 1, 2025
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jul 1, 2025
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jul 1, 2025
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jul 1, 2025
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jul 1, 2025
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jul 2, 2025
Mehmet YILMAZ (m3hm3t)
added a commit
that referenced
this pull request
Jul 16, 2025
This PR provides successful build against PG18Beta1. RuleUtils PR was reviewed separately: #8010 ## PG 18Beta1–related changes for building Citus ### TupleDesc / Attr layout **What changed in PG:** Postgres consolidated the `TupleDescData.attrs[]` array into a more compact representation. Direct field access (tupdesc->attrs[i]) was replaced by the new `TupleDescAttr()` API. **Citus adaptation:** Everywhere we previously used `tupdesc->attrs[...]`, we now call `TupleDescAttr(tupdesc, idx)` (or our own `Attr()` macro) under a compatibility guard. * postgres/postgres@5983a4c General Logic: * Use `Attr(...)` in places where `columnar_version_compat.h` is included. This avoids the need to sprinkle `#if PG_VERSION_NUM` guards around each attribute access. * Use `TupleDescAttr(tupdesc, i)` when the relevant PostgreSQL header is already included and the additional macro indirection is unnecessary. ### Collation‐aware `LIKE` **What changed in PG:** The `textlike` operator now requires an explicit collation, to avoid ambiguous‐collation errors. Core code switched from `DirectFunctionCall2(textlike, ...)` to `DirectFunctionCall2Coll(textlike, DEFAULT_COLLATION_OID, ...)`. **Citus adaptation:** In `remote_commands.c` and any other LIKE call, we now use `DirectFunctionCall2Coll(textlike, DEFAULT_COLLATION_OID, ...)` and `#include <utils/pg_collation.h>`. * postgres/postgres@85b7efa ### Columnar storage API * Adapt `columnar_relation_set_new_filelocator` (and related init routines) for PG 18’s revised SMGR and storage-initialization hooks. * Pull in the new headers (`explain_format.h`, `columnar_version_compat.h`) so the columnar module compiles cleanly against PG 18. - heap_modify_tuple + heap_inplace_update only exist on PG < 18; on PG18 the in-place helper was removed upstream - postgres/postgres@a07e03f ### OpenSSL / TLS integration **What changed in PG:** Moved from the legacy `SSL_library_init()` to `OPENSSL_init_ssl(OPENSSL_INIT_LOAD_CONFIG, NULL)`, updated certificate API calls (`X509_getm_notBefore`, `X509_getm_notAfter`), and standardized on `TLS_method()`. **Citus adaptation:** We now `#include <openssl/opensslv.h>` and use `#if OPENSSL_VERSION_NUMBER >= 0x10100000L` to choose between` OPENSSL_init_ssl()` or `SSL_library_init()`, and wrap` X509_gmtime_adj()` calls around the new accessor functions. * postgres/postgres@6c66b74 ### Adapt `ExtractColumns()` to the new PG-18 `expandRTE()` signature PostgreSQL 18 postgres/postgres@80feb72 added a fourth argument of type `VarReturningType` to `expandRTE()`, so calls that used the old 7-parameter form no longer compile. This patch: * Wraps the `expandRTE(...)` call in a `#if PG_VERSION_NUM >= 180000` guard. * On PG 18+ passes the new `VAR_RETURNING_DEFAULT` argument before `location`. * On PG 15–17 continues to call the original 7-arg form. * Adds the necessary includes (`parser/parse_relation.h` for `expandRTE` and `VarReturningType`, and `pg_version_constants.h` for `PG_VERSION_NUM`). ### Adapt `ExecutorStart`/`ExecutorRun` hooks to PG-18’s new signatures PostgreSQL 18 postgres/postgres@525392d changed the signatures of the executor hooks: * `ExecutorStart_hook` now returns `bool` instead of `void`, and * `ExecutorRun_hook` drops its old `run_once` argument. This patch preserves Citus’s existing hook logic by: 1. **Adding two adapter functions** under `#if PG_VERSION_NUM >= PG_VERSION_18`: * `citus_executor_start_adapter(QueryDesc *queryDesc, int eflags)` Calls the old `CitusExecutorStart(queryDesc, eflags)` and then returns `true` to satisfy the new hook’s `bool` return type. * `citus_executor_run_adapter(QueryDesc *queryDesc, ScanDirection direction, uint64 count)` Calls the old `CitusExecutorRun(queryDesc, direction, count, true)` (passing `true` for the dropped `run_once` argument), and returns `void`. 2. **Installing the adapters** in `_PG_init()` instead of the original hooks when building against PG 18+: ```c #if PG_VERSION_NUM >= PG_VERSION_18 ExecutorStart_hook = citus_executor_start_adapter; ExecutorRun_hook = citus_executor_run_adapter; #else ExecutorStart_hook = CitusExecutorStart; ExecutorRun_hook = CitusExecutorRun; #endif ``` ### Adapt to PG-18’s removal of the “run\_once” flag from ExecutorRun/PortalRun PostgreSQL commit [[3eea7a0](https://github.com/postgres/postgres/commit/3eea7a0c97e94f9570af87317ce3f6a41eb62768)](https://github.com/postgres/postgres/commit/3eea7a0c97e94f9570af87317ce3f6a41eb62768) rationalized the executor’s parallelism logic by moving the “execute a plan only once” check into `ExecutePlan()` itself and dropping the old `bool run_once` argument from the public APIs: ```diff - void ExecutorRun(QueryDesc *queryDesc, - ScanDirection direction, - uint64 count, - bool run_once); + void ExecutorRun(QueryDesc *queryDesc, + ScanDirection direction, + uint64 count); ``` (and similarly for `PortalRun()`). To stay compatible across PG 15–18, Citus now: 1. **Updates all internal calls** to `ExecutorRun(...)` and `PortalRun(...)`: * On PG 18+, use the new three-argument form (`ExecutorRun(qd, dir, count)`). * On PG 15–17, keep the old four-arg form (`ExecutorRun(qd, dir, count, true)`) under a `#if PG_VERSION_NUM < 180000` guard. 2. **Guards the dispatcher hooks** via the adapter functions (from the earlier patch) so that Citus’s executor hooks continue to work under both the old and new signatures. ### Adapt to PG-18’s shortened PortalRun signature PostgreSQL 18’s refactoring (see commit [3eea7a0](postgres/postgres@3eea7a0)) also removed the old run_once and alternate‐dest arguments from the public PortalRun() API. The signature changed from: ```diff - bool PortalRun(Portal portal, - long count, - bool isTopLevel, - bool run_once, - DestReceiver *dest, - DestReceiver *altdest, - QueryCompletion *qc); + bool PortalRun(Portal portal, + long count, + bool isTopLevel, + DestReceiver *dest, + DestReceiver *altdest, + QueryCompletion *qc); ``` To support both versions in Citus, we: 1. **Version-guard each call** to `PortalRun()`: * **On PG 18+** invoke the new 6-argument form. * **On PG 15–17** fall back to the legacy 7-argument form, passing `true` for `run_once`. ### Add support for PG-18’s new `plansource` argument in `PortalDefineQuery`** PostgreSQL 18 extended the `PortalDefineQuery` API to carry a `CachedPlanSource *plansource` pointer so that the portal machinery can track cached‐plan invalidation (as introduced alongside deferred-locking in commit postgres/postgres@525392d. To remain compatible across PG 15–18, Citus now wraps its calls under a version guard: ```diff - PortalDefineQuery(portal, NULL, sql, commandTag, plantree_list, NULL); +#if PG_VERSION_NUM >= 180000 + /* PG 18+: seven-arg signature (adds plansource) */ + PortalDefineQuery( + portal, + NULL, /* no prepared-stmt name */ + sql, /* the query text */ + commandTag, /* the CommandTag */ + plantree_list, /* List of PlannedStmt* */ + NULL, /* no CachedPlan */ + NULL /* no CachedPlanSource */ + ); +#else + /* PG 15–17: six-arg signature */ + PortalDefineQuery( + portal, + NULL, /* no prepared-stmt name */ + sql, /* the query text */ + commandTag, /* the CommandTag */ + plantree_list, /* List of PlannedStmt* */ + NULL /* no CachedPlan */ + ); +#endif ``` ### Adapt ExecInitRangeTable() calls to PG-18’s new signature PostgreSQL commit [cbc127917e04a978a788b8bc9d35a70244396d5b](postgres/postgres@cbc1279) overhauled the planner API for range‐table initialization: **PG 18+**: added a fourth `Bitmapset *unpruned_relids` argument to support deferred partition pruning In Citus’s `create_estate_for_relation()` (in `columnar_metadata.c`), we now wrap the call in a compile‐time guard so that the code compiles correctly on all supported PostgreSQL versions: ``` /* Prepare permission info on PG 16+ */ #if PG_VERSION_NUM >= PG_VERSION_16 List *perminfos = NIL; addRTEPermissionInfo(&perminfos, rte); #else List *perminfos = NIL; /* unused on PG 15 */ #endif /* Initialize the range table, with the right signature for each PG version */ #if PG_VERSION_NUM >= PG_VERSION_18 /* PG 18+: four‐arg signature (adds unpruned_relids) */ ExecInitRangeTable( estate, list_make1(rte), perminfos, NULL /* unpruned_relids: not used by columnar */ ); #elif PG_VERSION_NUM >= PG_VERSION_16 /* PG 16–17: three‐arg signature (permInfos) */ ExecInitRangeTable( estate, list_make1(rte), perminfos ); #else /* PG 15: two‐arg signature */ ExecInitRangeTable( estate, list_make1(rte) ); #endif estate->es_output_cid = GetCurrentCommandId(true); ``` ### Adapt `pgstat_report_vacuum()` to PG-18’s new timestamp argument PostgreSQL commit [[30a6ed0ce4bb18212ec38cdb537ea4b43bc99b83](https://github.com/postgres/postgres/commit/30a6ed0ce4bb18212ec38cdb537ea4b43bc99b83)](https://github.com/postgres/postgres/commit/30a6ed0ce4bb18212ec38cdb537ea4b43bc99b83) extended the `pgstat_report_vacuum()` API by adding a `TimestampTz start_time` parameter at the end so that the VACUUM statistics collector can record when the operation began: ```diff /* PG ≤17: four-arg signature */ - void pgstat_report_vacuum(Oid tableoid, - bool shared, - double num_live_tuples, - double num_dead_tuples); +/* PG ≥18: five-arg signature adds a start_time */ + void pgstat_report_vacuum(Oid tableoid, + bool shared, + double num_live_tuples, + double num_dead_tuples, + TimestampTz start_time); ``` To support both versions, we now wrap the call in `columnar_tableam.c` with a version guard, supplying `GetCurrentTimestamp()` for PG-18+: ```c #if PG_VERSION_NUM >= 180000 /* PG 18+: include start_timestamp */ pgstat_report_vacuum( RelationGetRelid(rel), rel->rd_rel->relisshared, Max(new_live_tuples, 0), /* live tuples */ 0, /* dead tuples */ GetCurrentTimestamp() /* start time */ ); #else /* PG 15–17: original signature */ pgstat_report_vacuum( RelationGetRelid(rel), rel->rd_rel->relisshared, Max(new_live_tuples, 0), /* live tuples */ 0 /* dead tuples */ ); #endif ``` ### Adapt `ExecuteTaskPlan()` to PG-18’s expanded `CreateQueryDesc()` signature PostgreSQL 18 changed `CreateQueryDesc()` from an eight-argument to a nine-argument call by inserting a `CachedPlan *cplan` parameter immediately after the `PlannedStmt *plannedstmt` argument (see commit postgres/postgres@525392d). To remain compatible with PG 15–17, Citus now wraps its invocation in `local_executor.c` with a version guard: ```diff - /* PG15–17: eight-arg CreateQueryDesc without cached plan */ - QueryDesc *queryDesc = CreateQueryDesc( - taskPlan, /* PlannedStmt *plannedstmt */ - queryString, /* const char *sourceText */ - GetActiveSnapshot(),/* Snapshot snapshot */ - InvalidSnapshot, /* Snapshot crosscheck_snapshot */ - destReceiver, /* DestReceiver *dest */ - paramListInfo, /* ParamListInfo params */ - queryEnv, /* QueryEnvironment *queryEnv */ - 0 /* int instrument_options */ - ); +#if PG_VERSION_NUM >= 180000 + /* PG18+: nine-arg CreateQueryDesc with a CachedPlan slot */ + QueryDesc *queryDesc = CreateQueryDesc( + taskPlan, /* PlannedStmt *plannedstmt */ + NULL, /* CachedPlan *cplan (none) */ + queryString, /* const char *sourceText */ + GetActiveSnapshot(),/* Snapshot snapshot */ + InvalidSnapshot, /* Snapshot crosscheck_snapshot */ + destReceiver, /* DestReceiver *dest */ + paramListInfo, /* ParamListInfo params */ + queryEnv, /* QueryEnvironment *queryEnv */ + 0 /* int instrument_options */ + ); +#else + /* PG15–17: eight-arg CreateQueryDesc without cached plan */ + QueryDesc *queryDesc = CreateQueryDesc( + taskPlan, /* PlannedStmt *plannedstmt */ + queryString, /* const char *sourceText */ + GetActiveSnapshot(),/* Snapshot snapshot */ + InvalidSnapshot, /* Snapshot crosscheck_snapshot */ + destReceiver, /* DestReceiver *dest */ + paramListInfo, /* ParamListInfo params */ + queryEnv, /* QueryEnvironment *queryEnv */ + 0 /* int instrument_options */ + ); +#endif ``` ### Adapt `RelationGetPrimaryKeyIndex()` to PG-18’s new “deferrable\_ok” flag PostgreSQL commit postgres/postgres@14e87ff added a new Boolean `deferrable_ok` parameter to `RelationGetPrimaryKeyIndex()` so that the lock manager can defer unique‐constraint locks when requested. The API changed from: ```c RelationGetPrimaryKeyIndex(Relation relation) ``` to: ```c RelationGetPrimaryKeyIndex(Relation relation, bool deferrable_ok) ``` ```diff diff --git a/src/backend/distributed/metadata/node_metadata.c b/src/backend/distributed/metadata/node_metadata.c index e3a1b2c..f4d5e6f 100644 --- a/src/backend/distributed/metadata/node_metadata.c +++ b/src/backend/distributed/metadata/node_metadata.c @@ -2965,8 +2965,18 @@ */ - Relation replicaIndex = index_open(RelationGetPrimaryKeyIndex(pgDistNode), - AccessShareLock); + #if PG_VERSION_NUM >= PG_VERSION_18 + /* PG 18+ adds a bool "deferrable_ok" parameter */ + Relation replicaIndex = + index_open( + RelationGetPrimaryKeyIndex(pgDistNode, false), + AccessShareLock); + #else + Relation replicaIndex = + index_open( + RelationGetPrimaryKeyIndex(pgDistNode), + AccessShareLock); + #endif ScanKeyInit(&scanKey[0], Anum_pg_dist_node_nodename, BTEqualStrategyNumber, F_TEXTEQ, CStringGetTextDatum(nodeName)); ``` ```diff diff --git a/src/backend/distributed/operations/node_protocol.c b/src/backend/distributed/operations/node_protocol.c index e3a1b2c..f4d5e6f 100644 --- a/src/backend/distributed/operations/node_protocol.c +++ b/src/backend/distributed/operations/node_protocol.c @@ -746,7 +746,12 @@ if (!OidIsValid(idxoid)) { - idxoid = RelationGetPrimaryKeyIndex(rel); + /* Determine the index OID of the primary key (PG18 adds a second parameter) */ +#if PG_VERSION_NUM >= PG_VERSION_18 + idxoid = RelationGetPrimaryKeyIndex(rel, false); +#else + idxoid = RelationGetPrimaryKeyIndex(rel); +#endif } return idxoid; ``` Because Citus has always taken the lock immediately—just as the old two-arg call did—we pass `false` to keep that same immediate-lock behavior. Passing `true` would switch to deferred locking, which we don’t want. ### Adapt `ExplainOnePlan()` to PG-18’s expanded API PostgreSQL 18 extended postgres/postgres@525392d the `ExplainOnePlan()` function to carry the `CachedPlan *` and `CachedPlanSource *` pointers plus an explicit `query_index`, letting the EXPLAIN machinery track plan‐source invalidation. The old signature: ```c /* PG ≤17 */ void ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, struct ExplainState *es, const char *queryString, ParamListInfo params, QueryEnvironment *queryEnv, const instr_time *planduration, const BufferUsage *bufusage); ``` became, in PG 18: ```c /* PG ≥18 */ void ExplainOnePlan(PlannedStmt *plannedstmt, CachedPlan *cplan, CachedPlanSource *plansource, int query_index, IntoClause *into, struct ExplainState *es, const char *queryString, ParamListInfo params, QueryEnvironment *queryEnv, const instr_time *planduration, const BufferUsage *bufusage, const MemoryContextCounters *mem_counters); ``` To compile under both versions, Citus now wraps each call in `multi_explain.c` with: ```c #if PG_VERSION_NUM >= PG_VERSION_18 /* PG 18+: pass NULL for the new cached‐plan fields and zero for query_index */ ExplainOnePlan( plan, /* PlannedStmt *plannedstmt */ NULL, /* CachedPlan *cplan */ NULL, /* CachedPlanSource *plansource */ 0, /* query_index */ into, /* IntoClause *into */ es, /* ExplainState *es */ queryString, /* const char *queryString */ params, /* ParamListInfo params */ NULL, /* QueryEnvironment *queryEnv */ &planduration,/* const instr_time *planduration */ (es->buffers ? &bufusage : NULL), (es->memory ? &mem_counters : NULL) ); #elif PG_VERSION_NUM >= PG_VERSION_17 /* PG 17: same as before, plus passing mem_counters if enabled */ ExplainOnePlan( plan, into, es, queryString, params, queryEnv, &planduration, (es->buffers ? &bufusage : NULL), (es->memory ? &mem_counters : NULL) ); #else /* PG 15–16: original seven-arg form */ ExplainOnePlan( plan, into, es, queryString, params, queryEnv, &planduration, (es->buffers ? &bufusage : NULL) ); #endif ``` ### Adapt to the unified “index interpretation” API in PG 18 (commit a8025f544854) PostgreSQL commit postgres/postgres@a8025f5 generalized the old btree‐specific operator‐interpretation API into a single “index interpretation” interface: * **Renamed type**: `OpBtreeInterpretation` → `OpIndexInterpretation` * **Renamed function**: `get_op_btree_interpretation(opno)` → `get_op_index_interpretation(opno)` * **Unified field**: Each interpretation now carries `cmptype` instead of `strategy`. To build cleanly on PG 18 while still supporting PG 15–17, Citus’s shard‐pruning code now wraps these changes: ```c #include "pg_version_constants.h" #if PG_VERSION_NUM >= PG_VERSION_18 /* On PG 18+ the btree‐only APIs vanished; alias them to the new generic versions */ typedef OpIndexInterpretation OpBtreeInterpretation; #define get_op_btree_interpretation(opno) get_op_index_interpretation(opno) #define ROWCOMPARE_NE COMPARE_NE #endif /* … later, when checking an interpretation … */ OpBtreeInterpretation *interp = (OpBtreeInterpretation *) lfirst(cell); #if PG_VERSION_NUM >= PG_VERSION_18 /* use cmptype on PG 18+ */ if (interp->cmptype == ROWCOMPARE_NE) #else /* use strategy on PG 15–17 */ if (interp->strategy == ROWCOMPARE_NE) #endif { /* … */ } ``` ### Adapt `create_foreignscan_path()` for PG-18’s revised signature PostgreSQL commit postgres/postgres@e222534 reordered and removed a couple of parameters in the FDW‐path builder: * **PG 15–17 signature (11 args)** ```c create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel, PathTarget *target, double rows, Cost startup_cost, Cost total_cost, List *pathkeys, Relids required_outer, Path *fdw_outerpath, List *fdw_restrictinfo, List *fdw_private); ``` * **PG 18+ signature (9 args)** ```c create_foreignscan_path(PlannerInfo *root, RelOptInfo *rel, PathTarget *target, double rows, int disabled_nodes, Cost startup_cost, Cost total_cost, Relids required_outer, Path *fdw_outerpath, List *fdw_private); ``` To support both, Citus now defines a compatibility macro in `pg_version_compat.h`: ```c #include "nodes/bitmapset.h" /* for Relids */ #include "nodes/pg_list.h" /* for List */ #include "optimizer/pathnode.h" /* for create_foreignscan_path() */ #if PG_VERSION_NUM >= PG_VERSION_18 /* PG18+: drop pathkeys & fdw_restrictinfo, add disabled_nodes */ #define create_foreignscan_path_compat(a, b, c, d, e, f, g, h, i, j, k) \ create_foreignscan_path( \ (a), /* root */ \ (b), /* rel */ \ (c), /* target */ \ (d), /* rows */ \ (0), /* disabled_nodes (unused by Citus) */ \ (e), /* startup_cost */ \ (f), /* total_cost */ \ (g), /* required_outer */ \ (h), /* fdw_outerpath */ \ (k) /* fdw_private */ \ ) #else /* PG15–17: original signature */ #define create_foreignscan_path_compat(a, b, c, d, e, f, g, h, i, j, k) \ create_foreignscan_path( \ (a), (b), (c), (d), \ (e), (f), \ (g), (h), (i), (j), (k) \ ) #endif ``` Now every call to `create_foreignscan_path_compat(...)`—even in tests like `fake_fdw.c`—automatically picks the correct argument list for PG 15 through PG 18. ### Drop the obsolete bitmap‐scan hooks on PG 18+ PostgreSQL commit postgres/postgres@c395322 cleaned up the `TableAmRoutine` API by removing the two bitmap‐scan callback slots: * `scan_bitmap_next_block` * `scan_bitmap_next_tuple` Since those hook‐slots no longer exist in PG 18, Citus now wraps their NULL‐initialization in a `#if PG_VERSION_NUM < PG_VERSION_18` guard. On PG 15–17 we still explicitly set them to `NULL` (to satisfy the old struct layout), and on PG 18+ we omit them entirely: ```c #if PG_VERSION_NUM < PG_VERSION_18 /* PG 15–17 only: these fields were removed upstream in PG 18 */ .scan_bitmap_next_block = NULL, .scan_bitmap_next_tuple = NULL, #endif ``` ### Adapt `vac_update_relstats()` invocation to PG-18’s new “all\_frozen” argument PostgreSQL commit postgres/postgres@99f8f3f extended the `vac_update_relstats()` API by inserting a `num_all_frozen_pages` parameter between the existing `num_all_visible_pages` and `hasindex` arguments: ```diff - /* PG ≤17: */ - void - vac_update_relstats(Relation relation, - BlockNumber num_pages, - double num_tuples, - BlockNumber num_all_visible_pages, - bool hasindex, - TransactionId frozenxid, - MultiXactId minmulti, - bool *frozenxid_updated, - bool *minmulti_updated, - bool in_outer_xact); + /* PG ≥18: adds num_all_frozen_pages */ + void + vac_update_relstats(Relation relation, + BlockNumber num_pages, + double num_tuples, + BlockNumber num_all_visible_pages, + BlockNumber num_all_frozen_pages, + bool hasindex, + TransactionId frozenxid, + MultiXactId minmulti, + bool *frozenxid_updated, + bool *minmulti_updated, + bool in_outer_xact); ``` To compile cleanly on both PG 15–17 and PG 18+, Citus wraps its call in a version guard and supplies a zero placeholder for the new field: ```c #if PG_VERSION_NUM >= 180000 /* PG 18+: supply explicit “all_frozen” count */ vac_update_relstats( rel, new_rel_pages, new_live_tuples, new_rel_allvisible, /* allvisible */ 0, /* all_frozen */ nindexes > 0, newRelFrozenXid, newRelminMxid, &frozenxid_updated, &minmulti_updated, false /* in_outer_xact */ ); #else /* PG 15–17: original signature */ vac_update_relstats( rel, new_rel_pages, new_live_tuples, new_rel_allvisible, nindexes > 0, newRelFrozenXid, newRelminMxid, &frozenxid_updated, &minmulti_updated, false /* in_outer_xact */ ); #endif ``` **Why all_frozen = 0?** Columnar storage never embeds transaction IDs in its pages, so it never needs to track “all‐frozen” pages the way a heap does. Setting both allvisible and allfrozen to zero simply tells Postgres “there are no pages with the visibility or frozen‐status bits set,” matching our existing behavior. This change ensures Citus’s VACUUM‐statistic updates work unmodified across all supported Postgres versions.
ibrahim halatci (ihalatci)
added a commit
that referenced
this pull request
May 29, 2026
Scaffold only, cloned from 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). Exclude the file from citus-style in .gitattributes: it is an upstream-derived clone exceeding the CI uncrustify 10,000-line limit.
ibrahim halatci (ihalatci)
added a commit
that referenced
this pull request
May 29, 2026
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.
ibrahim halatci (ihalatci)
added a commit
that referenced
this pull request
May 29, 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
ibrahim halatci (ihalatci)
added a commit
that referenced
this pull request
Jun 6, 2026
… 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
ibrahim halatci (ihalatci)
added a commit
that referenced
this pull request
Jun 13, 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
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`).
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.
https://github.com/postgres/postgres/commits/master/src/backend/utils/adt/ruleutils.c