Add PostgreSQL 18 build support (v2.2.3)#665
Open
kaidaguerre wants to merge 3 commits into
Open
Conversation
Three localized, version-guarded source changes so the FDW compiles and links against PostgreSQL 18 while leaving PostgreSQL 16-and-earlier behaviour byte-identical (every change is behind #if PG_VERSION_NUM >= 180000): - fdw/common.h: include commands/explain_format.h on PG18 (PG18 split the EXPLAIN property-output API, incl. ExplainPropertyText, out of commands/explain.h into a new header). - fdw/query.c: use PathKey.pk_cmptype == COMPARE_GT on PG18 (PG18 renamed pk_strategy:int to pk_cmptype:CompareType; COMPARE_GT is the documented equivalent of BTGreaterStrategyNumber). pre-18 path unchanged in the #else arm. - fdw/fdw.c (2 sites) and fdw/query.c (1 site): pass the two new create_foreignscan_path arguments on PG18 - disabled_nodes = 0 (after rows) and fdw_restrictinfo = NIL (before fdw_private). make_foreignscan is unchanged in PG18 and is left as-is. Verified: PG18 build compiles + links (steampipe_postgres_fdw.dylib; only the pre-existing macOS .so/.dylib inst-step quirk remains); PG14 build make exit 0 (guards proven inert, identical to baseline); go test ./types/... ./sql/... no worse than the develop baseline (types green; sql/ TestGetSQLForTable already red on develop, unchanged). Bump fdwVersion 2.2.2 -> 2.2.3 and add the CHANGELOG entry.
Code-review finding: create_foreignscan_path's fdw_restrictinfo parameter was added in PostgreSQL 17, not 18 (verified against REL_16/17/18 optimizer/pathnode.h). disabled_nodes is the PG18 addition. Gating fdw_restrictinfo at >= 180000 was correct for the 14->18 build matrix but would fail a PG17 build (arg omitted) and the CHANGELOG mislabelled it as a PG18 argument. fdw_restrictinfo now guarded #if PG_VERSION_NUM >= 170000 at all three create_foreignscan_path sites; disabled_nodes stays >= 180000. PG18 is unchanged (>=170000 is also true there); PG<=16 still excludes both. CHANGELOG reworded to state the correct per-arg version.
PG18 dropped the T_RestrictInfo case from expression_tree_walker (src/backend/nodes/nodeFuncs.c — RestrictInfo is now annotated `pg_node_attr(no_read, no_query_jumble)` and the auto-generated copy/equal/out functions don't include it either). When the FDW's isAttrInRestrictInfo / extractColumns call pull_var_clause on a restrictinfo->clause subtree, and that subtree contains a nested RestrictInfo (the planner's orclause cache, certain equivalence-class derived join predicates), pull_var_clause's expression_tree_walker hits the unrecognized tag (318) and elogs: ERROR: unrecognized node type: 318 (SQLSTATE XX000) LOCATION: expression_tree_walker_impl, nodeFuncs.c:2669 Empirically confirmed via backtrace_functions on a multi-table join on PG18.4 (`select r.name, count(b.name) from <ft1> r left join <ft2> b on b.region = r.name group by r.name`). Stack: expression_tree_walker_impl pull_var_clause_walker pull_var_clause steampipe_postgres_fdw.so findPaths + 564 steampipe_postgres_fdw.so fdwGetForeignPaths Fix: a small pre-pass mutator under #if PG_VERSION_NUM >= 180000 that replaces each RestrictInfo with its wrapped clause before pull_var_clause's walker descends. Wrapped behind a PULL_VAR_CLAUSE_PG18_SAFE macro so the three pull_var_clause call sites in fdw/query.c (extractColumns reltargetlist loop, extractColumns restrictinfolist loop, isAttrInRestrictInfo) get the same treatment uniformly. PG14 builds are byte-identical to before (the macro expands to plain pull_var_clause when PG_VERSION_NUM < 180000). Reference: contrib/postgres_fdw/deparse.c:1585 does the equivalent one-step unwrap (`expr = ((RestrictInfo *) expr)->clause`) as the canonical PG18 pattern; this commit applies the recursive form so deeper nesting (encountered with equivalence-class derived join predicates) is also handled. Verified post-fix: the previously failing multi-table join now plans successfully and reaches plugin execution; chaos plugin self-join returns rows end-to-end; no regression on the existing chaos query battery (count(*), get-by-key, ORDER BY + LIMIT, LIKE).
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
1 participant
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.
What
Three localized, version-guarded source changes so the FDW compiles and links against PostgreSQL 18, with PostgreSQL 16-and-earlier behaviour byte-identical (every change is behind
#if PG_VERSION_NUM >= 180000):fdw/common.h— includecommands/explain_format.hon PG18 (PG18 split the EXPLAIN property-output API, incl.ExplainPropertyText, out ofcommands/explain.h).fdw/query.c— usePathKey.pk_cmptype == COMPARE_GTon PG18 (PG18 renamedpk_strategy:int→pk_cmptype:CompareType;COMPARE_GTis the documented equivalent ofBTGreaterStrategyNumber). Pre-18 path unchanged in the#elsearm.fdw/fdw.c(2 sites) +fdw/query.c(1 site) — pass the two newcreate_foreignscan_patharguments on PG18:disabled_nodes = 0(afterrows) andfdw_restrictinfo = NIL(beforefdw_private).make_foreignscanis unchanged in PG18 and left as-is.Bumps
fdwVersion2.2.2 → 2.2.3 + CHANGELOG entry.Risk
Near-zero for the shipped product: the FDW ships built on PG14, where the preprocessor strips every change — byte-identical. The PG18 arms are inert on PG≤16 by construction.
Testing
makecompiles + links (steampipe_postgres_fdw.dylib); only the pre-existing macOS.so/.dylibinst-step quirk remains.makeexit 0, identical to the develop baseline (guards proven inert).go test ./types/... ./sql/...: no worse than develop baseline (types/green;sql/ TestGetSQLForTablealready red on develop, unchanged — not introduced here).Scope
This is a build-enablement change only — it does not flip any embedded version, tag, or publish any image. The FDW image tag/publish and the embedded-PG version flip are handled separately.