PG19: fix runtime bugs hit outside the regression suite#8617
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## pg19-support #8617 +/- ##
===============================================
Coverage ? 88.93%
===============================================
Files ? 288
Lines ? 64371
Branches ? 8092
===============================================
Hits ? 57251
Misses ? 4783
Partials ? 2337 🚀 New features to boost your workflow:
|
9d9144a to
3a0a90e
Compare
There was a problem hiding this comment.
Pull request overview
This PR is part of the PG19 support stack and addresses PG19-only runtime crashes by updating Citus compatibility shims and ensuring newly-required tuple descriptor finalization is performed before slot deformation.
Changes:
- Fix PG19
FuncnameGetCandidates()compatibility shim by providing a validfgc_flagsout-parameter. - Ensure PG19 TupleDesc offset-cache initialization via
TupleDescFinalize()(including a PG19BlessTupleDesc()wrapper and explicit finalize calls at key call sites). - Update GUC override logic for PG19’s
config_genericlayout change when installing theapplication_nameassign hook.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/include/pg_version_compat.h | Updates PG19 compatibility macros (fgc_flags shim; TupleDescFinalize + BlessTupleDesc wrapper; no-op TupleDescFinalize on older majors). |
| src/backend/distributed/stats/query_stats.c | Finalizes SRF-provided TupleDesc before using it with slot deformation on PG19. |
| src/backend/distributed/shared_library_init.c | Adjusts PG19 GUC struct access to install ApplicationNameAssignHook safely. |
| src/backend/distributed/planner/multi_explain.c | Finalizes a hand-built TupleDesc used for EXPLAIN ANALYZE result collection on PG19. |
| src/backend/distributed/executor/distributed_intermediate_results.c | Finalizes hand-built TupleDescs used with tuple deformation/processing on PG19. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #define BlessTupleDesc(td) \ | ||
| (TupleDescFinalize(td), (BlessTupleDesc) (td)) |
There was a problem hiding this comment.
Good catch. Replaced the comma-expression macro with a single-evaluation static inline citus_BlessTupleDesc() helper defined before the macro, so the argument is only evaluated once and side-effecting call sites like BlessTupleDesc(CreateTemplateTupleDesc(...)) are safe. Verified with a full backend build against PG19beta1 (no warnings/errors).
ad4ccab to
2a197c5
Compare
Two PG19 runtime crashes that surface in normal Citus operation (not just the regression tests): 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). Pass the address of an int compound literal instead. 2. PG19 added TupleDesc->firstNonCachedOffsetAttr, an offset cache that BlessTupleDesc()/slot_deform_heap_tuple() now assert is populated but no longer populate themselves. Wrap BlessTupleDesc to call TupleDescFinalize() first, add a no-op shim on older majors, and call it explicitly at the three sites that deform hand-built TupleDescs. Also installs ApplicationNameAssignHook via union access on PG19. DESCRIPTION: Fix PG19 runtime crashes in function lookup and TupleDesc handling Closes #8610 Part of #8597
3a0a90e to
2ff1cfd
Compare
alperkocatas
left a comment
There was a problem hiding this comment.
Nit: For item 2: The message says "call it explicitly at the three sites," but there are four explicit TupleDescFinalize() calls (two in distributed_intermediate_results.c, one in multi_explain.c, one in query_stats.c). The query_stats.c one is arguably a separate "defensive SRF" case rather than a hand-built desc — worth tightening the wording so the count matches.
Stacked PR (PR2 of the PG19 stack). Base =
pg19-ci-test-matrices(#8616), which is itself stacked onpg19-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):
FuncnameGetCandidates()gained anint *fgc_flagsout-parameter that the callee writes to unconditionally. The compat shim passedNULLand crashed on every by-name function lookup (e.g. the maintenance daemon). Now passes the address of an int compound literal.TupleDesc->firstNonCachedOffsetAttr: PG19 added an offset cache thatBlessTupleDesc()/slot_deform_heap_tuple()now assert is populated but no longer populate themselves.BlessTupleDescis wrapped to callTupleDescFinalize()first (no-op shim on older majors), with explicit calls added at the four sites that deform hand-built TupleDescs.Also installs
ApplicationNameAssignHookvia union access on PG19.Closes #8610
Part of #8597