On Sat, Sep 12, 2026 at 8:04 AM Nikolay Samokhvalov <[email protected]> wrote:
> I'd keep the dead list for now. No runtime issue found in v2.
Hi Amit,
I kept iterating with our new PostgresAI harness and found one more
issue while testing v2.
`ri_HashCompareOp()` can process a cast invalidation inside
`fmgr_info_cxt()`, before publishing `entry->info`. The callback sees
NULL and has nothing to detach, so the outer call publishes the old
cast information afterward.
In the affected backend, a direct cast maps 2 to 102, but RI still
looks for parent 2. A fresh backend uses the new cast. This can occur
when function initialization processes a pending cast invalidation
during cache construction.
Reproduced on master and PG19. The same comparison-cache construction
sequence is present in the PG14–18 v2 patches.
Function initialization can load a C library and run its `_PG_init()`.
Nested RI from there can populate the same cache entry. The outer call
then overwrites it, leaking the comparison context; fast-path metadata
has the same problem.
In case helpful, attached is an incremental fix for master/PG19, on
top of v2. It retries after invalidation and keeps an entry already
populated by a nested call. The fast-path check uses a per-constraint
generation, since a nested reload can set `valid` back to true before
the outer call resumes. PG14–18 need only the comparison-cache
changes.
The fix passes checks for invalidation during construction, nested
initialization, and both together. On master, regression, isolation,
and injection-point suites pass with the earlier memory-test
correction applied.
Thanks,
Nik
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index e2ca06b9..d60198bf 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -123,6 +123,7 @@ typedef struct RI_ConstraintInfo
{
Oid constraint_id; /* OID of pg_constraint entry (hash key) */
bool valid; /* successfully initialized? */
+ uint64 cache_generation; /* increments when entry is invalidated */
Oid constraint_root_id; /* OID of topmost ancestor constraint;
* same as constraint_id if not inherited */
uint32 oidHashValue; /* hash value of constraint_id */
@@ -338,6 +339,9 @@ static FastPathMeta *ri_fpmeta_dead_list = NULL;
/* Comparison call information detached by InvalidateCastCacheCallBack(). */
static RI_CompareInfo *ri_compare_dead_list = NULL;
+/* Incremented whenever the comparison cache is invalidated. */
+static uint64 ri_compare_cache_generation = 0;
+
/*
* Local function prototypes
*/
@@ -2511,7 +2515,10 @@ ri_LoadConstraintInfo(Oid constraintOid)
&constraintOid,
HASH_ENTER, &found);
if (!found)
+ {
riinfo->valid = false;
+ riinfo->cache_generation = 0;
+ }
else if (riinfo->valid)
return riinfo;
@@ -2665,6 +2672,7 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
riinfo->oidHashValue == hashvalue ||
riinfo->rootHashValue == hashvalue)
{
+ riinfo->cache_generation++;
riinfo->valid = false;
/*
@@ -2704,6 +2712,7 @@ InvalidateCastCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
HASH_SEQ_STATUS status;
RI_CompareHashEntry *entry;
+ ri_compare_cache_generation++;
hash_seq_init(&status, ri_compare_cache);
while ((entry = hash_seq_search(&status)) != NULL)
{
@@ -3785,56 +3794,79 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo,
{
FastPathMeta *fpmeta;
MemoryContext context;
+ uint64 generation;
- Assert(riinfo != NULL && riinfo->valid);
- Assert(riinfo->fpmeta == NULL);
-
- /* Keep incomplete metadata subject to normal error cleanup. */
- context = AllocSetContextCreate(CurTransactionContext,
- "RI fast-path finfo scratch",
- ALLOCSET_SMALL_SIZES);
- fpmeta = MemoryContextAllocZero(context, sizeof(FastPathMeta));
- fpmeta->scratch_cxt = context;
- for (int i = 0; i < riinfo->nkeys; i++)
+ for (;;)
{
- Oid eq_opr = riinfo->pf_eq_oprs[i];
- Oid typeid = RIAttType(fk_rel, riinfo->fk_attnums[i]);
- Oid lefttype;
- RI_CompareInfo *entry = ri_HashCompareOp(eq_opr, typeid);
- int idx_col;
+ Assert(riinfo != NULL && riinfo->valid);
+ if (riinfo->fpmeta != NULL)
+ return;
+ generation = riinfo->cache_generation;
- /*
- * Find the index column position for this constraint key. The FK
- * constraint may reference columns in a different order than they
- * appear in the PK index, so we must map pk_attnums[i] to the
- * corresponding index column position.
- */
- for (idx_col = 0; idx_col < riinfo->nkeys; idx_col++)
+ /* Keep incomplete metadata subject to normal error cleanup. */
+ context = AllocSetContextCreate(CurTransactionContext,
+ "RI fast-path finfo scratch",
+ ALLOCSET_SMALL_SIZES);
+ fpmeta = MemoryContextAllocZero(context, sizeof(FastPathMeta));
+ fpmeta->scratch_cxt = context;
+ for (int i = 0; i < riinfo->nkeys; i++)
{
- if (idx_rel->rd_index->indkey.values[idx_col] == riinfo->pk_attnums[i])
- break;
+ Oid eq_opr = riinfo->pf_eq_oprs[i];
+ Oid typeid = RIAttType(fk_rel, riinfo->fk_attnums[i]);
+ Oid lefttype;
+ RI_CompareInfo *entry = ri_HashCompareOp(eq_opr, typeid);
+ int idx_col;
+
+ /*
+ * Find the index column position for this constraint key. The FK
+ * constraint may reference columns in a different order than they
+ * appear in the PK index, so we must map pk_attnums[i] to the
+ * corresponding index column position.
+ */
+ for (idx_col = 0; idx_col < riinfo->nkeys; idx_col++)
+ {
+ if (idx_rel->rd_index->indkey.values[idx_col] == riinfo->pk_attnums[i])
+ break;
+ }
+ Assert(idx_col < riinfo->nkeys);
+
+ /* 1-based attribute number */
+ fpmeta->index_attnos[i] = idx_col + 1;
+
+ fmgr_info_copy(&fpmeta->cast_func_finfo[i], &entry->cast_func_finfo,
+ fpmeta->scratch_cxt);
+ fmgr_info_copy(&fpmeta->eq_opr_finfo[i], &entry->eq_opr_finfo,
+ fpmeta->scratch_cxt);
+ fpmeta->regops[i] = get_opcode(eq_opr);
+
+ get_op_opfamily_properties(eq_opr,
+ idx_rel->rd_opfamily[idx_col],
+ false,
+ &fpmeta->strats[i],
+ &lefttype,
+ &fpmeta->subtypes[i]);
}
- Assert(idx_col < riinfo->nkeys);
-
- /* 1-based attribute number */
- fpmeta->index_attnos[i] = idx_col + 1;
-
- fmgr_info_copy(&fpmeta->cast_func_finfo[i], &entry->cast_func_finfo,
- fpmeta->scratch_cxt);
- fmgr_info_copy(&fpmeta->eq_opr_finfo[i], &entry->eq_opr_finfo,
- fpmeta->scratch_cxt);
- fpmeta->regops[i] = get_opcode(eq_opr);
-
- get_op_opfamily_properties(eq_opr,
- idx_rel->rd_opfamily[idx_col],
- false,
- &fpmeta->strats[i],
- &lefttype,
- &fpmeta->subtypes[i]);
- }
- MemoryContextSetParent(context, TopMemoryContext);
- riinfo->fpmeta = fpmeta;
+ if (generation != riinfo->cache_generation || !riinfo->valid)
+ {
+ Oid constraint_id = riinfo->constraint_id;
+
+ MemoryContextDelete(context);
+ riinfo = ri_LoadConstraintInfo(constraint_id);
+ continue;
+ }
+
+ /* Nested RI may have populated the same entry while we built ours. */
+ if (riinfo->fpmeta != NULL)
+ {
+ MemoryContextDelete(context);
+ return;
+ }
+
+ MemoryContextSetParent(context, TopMemoryContext);
+ riinfo->fpmeta = fpmeta;
+ return;
+ }
}
/*
@@ -4371,7 +4403,7 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
* If not already initialized, build a new generation of call information.
* Use a separate context so invalidation cannot affect active callers.
*/
- if (entry->info == NULL)
+ while (entry->info == NULL)
{
Oid lefttype,
righttype,
@@ -4379,6 +4411,7 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
CoercionPathType pathtype;
MemoryContext context;
RI_CompareInfo *info;
+ uint64 generation = ri_compare_cache_generation;
/*
* If we chose to use a cast from FK to PK type, we may have to apply
@@ -4435,6 +4468,17 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
fmgr_info_cxt(castfunc, &info->cast_func_finfo, context);
else
info->cast_func_finfo.fn_oid = InvalidOid;
+
+ /*
+ * Discard this copy if invalidation was processed or a nested call
+ * populated the same entry while we were constructing it.
+ */
+ if (generation != ri_compare_cache_generation || entry->info != NULL)
+ {
+ MemoryContextDelete(context);
+ continue;
+ }
+
MemoryContextSetParent(context, TopMemoryContext);
entry->info = info;
}
