From 5b7e8f936536917bdef730e45ef2de405b8391ab Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Mon, 5 Oct 2026 15:49:24 +0900 Subject: [PATCH v1 2/3] Remove RI fast-path code for calling cast functions The previous commit stopped using the fast path when FK values need a cast function, which leaves its code for calling them unused. Remove it: the cached FmgrInfos for the cast and equality functions and the memory context they used, the cast step in build_index_scankeys(), and the EXECUTE privilege check on the cast function. Also revert the changes made to ri_HashCompareOp() for the fast path's cross-type operators and domain-typed keys in commits 2da86c1ef9 and 68ace967c1. Its remaining caller, ri_KeysEqual(), passes only same-type operators. Discussion: https://postgr.es/m/ --- src/backend/utils/adt/ri_triggers.c | 140 +++++++++------------------- 1 file changed, 42 insertions(+), 98 deletions(-) diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index a029dd6195d..32feea9b1df 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -164,25 +164,11 @@ typedef struct RI_CompareHashEntry RI_CompareHashEntry; /* Fast-path metadata for RI checks on foreign key referencing tables */ typedef struct FastPathMeta { - FmgrInfo eq_opr_finfo[RI_MAX_NUMKEYS]; - FmgrInfo cast_func_finfo[RI_MAX_NUMKEYS]; RegProcedure regops[RI_MAX_NUMKEYS]; Oid subtypes[RI_MAX_NUMKEYS]; int strats[RI_MAX_NUMKEYS]; AttrNumber index_attnos[RI_MAX_NUMKEYS]; /* index column positions */ - /* - * fn_mcxt for the cached FmgrInfos above. Cast and equality functions - * (e.g. record_eq()) use fn_mcxt as scratch space, caching state there - * and keeping a pointer to it in FmgrInfo.fn_extra. Give them a context - * of their own, created with this struct and destroyed with it in - * AtEOXact_RI(). - * - * Note this context must not be reset while the FmgrInfos remain in use, - * since that would free the state fn_extra still points at. - */ - MemoryContext scratch_cxt; - /* Link in ri_fpmeta_dead_list while awaiting deferred release */ struct FastPathMeta *next_dead; } FastPathMeta; @@ -311,11 +297,11 @@ static bool recheck_matched_pk_tuple(Relation idxrel, ScanKeyData *skeys, static void ri_CheckFunctionPermissions(const RI_ConstraintInfo *riinfo, const FastPathMeta *fpmeta); static void build_index_scankeys(const RI_ConstraintInfo *riinfo, - FastPathMeta *fpmeta, - Relation idx_rel, Datum *pk_vals, - char *pk_nulls, ScanKey skeys); + const FastPathMeta *fpmeta, + Relation idx_rel, const Datum *pk_vals, + ScanKey skeys); static void ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo, - Relation fk_rel, Relation idx_rel); + Relation idx_rel); static void ri_ExtractValues(Relation rel, TupleTableSlot *slot, const RI_ConstraintInfo *riinfo, bool rel_is_pk, Datum *vals, char *nulls); @@ -2558,13 +2544,12 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid, /* * Detach any fast-path metadata so that the next check * repopulates it, but do not free it here. ri_FastPathCheck() - * copies riinfo->fpmeta into a local (and takes FmgrInfo pointers - * into it) and then runs index scans, tuple locking, and - * user-supplied cast and equality functions, all of which can - * accept invalidation messages and reach this callback. Freeing - * now would leave those callers reading freed memory. Queue it - * instead; AtEOXact_RI() releases it once no RI check can be - * running. + * copies riinfo->fpmeta into a local and then does catalog + * lookups, index scans, tuple locking, and calls equality + * functions, all of which can accept invalidation messages and + * reach this callback. Freeing now would leave it reading freed + * memory. Queue it instead; AtEOXact_RI() releases it once no RI + * check can be running. */ if (riinfo->fpmeta) { @@ -2829,10 +2814,10 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo, * InvalidSnapshot, so SPI takes the snapshot after the * referenced-relation lock has been acquired. * - * Make this snapshot active too, as SPI does. STABLE cast and equality - * functions use the active snapshot, so leaving the outer query's - * snapshot active could hide changes made by earlier triggers even though - * the index scan can see them. + * Make this snapshot active too, as SPI does. STABLE equality functions + * use the active snapshot, so leaving the outer query's snapshot active + * could hide changes made by earlier triggers even though the index scan + * can see them. */ snapshot = RegisterSnapshot(GetTransactionSnapshot()); PushActiveSnapshot(snapshot); @@ -2861,7 +2846,7 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo, { /* Reload to ensure it's valid. */ riinfo = ri_LoadConstraintInfo(riinfo->constraint_id); - ri_populate_fastpath_metadata(riinfo, fk_rel, idx_rel); + ri_populate_fastpath_metadata(riinfo, idx_rel); } /* @@ -2875,7 +2860,7 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo, Assert(fpmeta); ri_CheckFunctionPermissions(riinfo, fpmeta); ri_ExtractValues(fk_rel, newslot, riinfo, false, pk_vals, pk_nulls); - build_index_scankeys(riinfo, fpmeta, idx_rel, pk_vals, pk_nulls, skey); + build_index_scankeys(riinfo, fpmeta, idx_rel, pk_vals, skey); found = ri_FastPathProbeOne(pk_rel, idx_rel, scandesc, slot, snapshot, riinfo, skey, riinfo->nkeys); SetUserIdAndSecContext(saved_userid, saved_sec_context); @@ -3250,8 +3235,9 @@ recheck_matched_pk_tuple(Relation idxrel, ScanKeyData *skeys, int nkeys, * * This parallels the checks ExecInitFunc() performs when the SPI path * initializes its generated query, where the equality operator's function - * appears in the WHERE clause and the cast function, if any, in the cast - * applied to the parameter. Call with the user id already switched to the + * appears in the WHERE clause. The fast path is not used when the FK values + * need a coercion function (see ri_check_fastpath_index()), so that is the + * only function to check. Call with the user id already switched to the * referenced table's owner. */ static void @@ -3260,21 +3246,15 @@ ri_CheckFunctionPermissions(const RI_ConstraintInfo *riinfo, { for (int i = 0; i < riinfo->nkeys; i++) { - Oid funcs[2] = {fpmeta->regops[i], fpmeta->cast_func_finfo[i].fn_oid}; + Oid funcid = fpmeta->regops[i]; + AclResult aclresult; - for (int j = 0; j < lengthof(funcs); j++) - { - AclResult aclresult; - - if (!OidIsValid(funcs[j])) - continue; - aclresult = object_aclcheck(ProcedureRelationId, funcs[j], - GetUserId(), ACL_EXECUTE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, OBJECT_FUNCTION, - get_func_name(funcs[j])); - InvokeFunctionExecuteHook(funcs[j]); - } + aclresult = object_aclcheck(ProcedureRelationId, funcid, + GetUserId(), ACL_EXECUTE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, OBJECT_FUNCTION, + get_func_name(funcid)); + InvokeFunctionExecuteHook(funcid); } } @@ -3282,33 +3262,19 @@ ri_CheckFunctionPermissions(const RI_ConstraintInfo *riinfo, * build_index_scankeys * Build ScanKeys for a direct index probe of the PK's unique index. * - * Uses cached compare entries, operator procedures, and strategy numbers - * from ri_populate_fastpath_metadata() rather than looking them up on - * each invocation. Casts FK values to the operator's expected input - * type if needed. + * Uses operator procedures and strategy numbers cached by + * ri_populate_fastpath_metadata() rather than looking them up on each + * invocation. The FK values are used as they are: the fast path is only + * used when they need no coercion to the operator's input type. */ static void build_index_scankeys(const RI_ConstraintInfo *riinfo, - FastPathMeta *fpmeta, - Relation idx_rel, Datum *pk_vals, - char *pk_nulls, ScanKey skeys) + const FastPathMeta *fpmeta, + Relation idx_rel, const Datum *pk_vals, + ScanKey skeys) { Assert(fpmeta); - /* - * May need to cast each of the individual values of the foreign key to - * the corresponding PK column's type if the equality operator demands it. - */ - for (int i = 0; i < riinfo->nkeys; i++) - { - if (pk_nulls[i] != 'n' && - OidIsValid(fpmeta->cast_func_finfo[i].fn_oid)) - pk_vals[i] = FunctionCall3(&fpmeta->cast_func_finfo[i], - pk_vals[i], - Int32GetDatum(-1), /* typmod */ - BoolGetDatum(false)); /* implicit coercion */ - } - /* * Set up ScanKeys for the index scan. This is essentially how * ExecIndexBuildScanKeys() sets them up. Use the cached index_attnos and @@ -3333,13 +3299,12 @@ build_index_scankeys(const RI_ConstraintInfo *riinfo, * ri_populate_fastpath_metadata * Cache per-key metadata needed by build_index_scankeys(). * - * Looks up the compare hash entry, operator procedure OID, and index - * strategy/subtype for each key column. Called lazily on first use - * and persists for the lifetime of the RI_ConstraintInfo entry. + * Looks up the operator procedure OID and index strategy/subtype for each + * key column. Called lazily on first use and persists for the lifetime of + * the RI_ConstraintInfo entry. */ static void -ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo, - Relation fk_rel, Relation idx_rel) +ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo, Relation idx_rel) { FastPathMeta *fpmeta; MemoryContext oldcxt = MemoryContextSwitchTo(TopMemoryContext); @@ -3350,16 +3315,10 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo, fpmeta = palloc_object(FastPathMeta); fpmeta->next_dead = NULL; - /* Scratch context for the cached FmgrInfos' fn_mcxt; see FastPathMeta. */ - fpmeta->scratch_cxt = AllocSetContextCreate(TopMemoryContext, - "RI fast-path finfo scratch", - ALLOCSET_SMALL_SIZES); for (int i = 0; i < riinfo->nkeys; i++) { Oid eq_opr = riinfo->pf_eq_oprs[i]; - Oid typeid = RIAttType(fk_rel, riinfo->fk_attnums[i]); Oid lefttype; - RI_CompareHashEntry *entry = ri_HashCompareOp(eq_opr, typeid); int idx_col; /* @@ -3378,10 +3337,6 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo, /* 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, @@ -3897,11 +3852,8 @@ ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid, /* * ri_HashCompareOp - * - * Look up or create a cache entry for the given equality operator and - * the caller's value type (typeid). The entry holds the operator's - * FmgrInfo and, if typeid doesn't match what the operator expects as - * its right-hand input, a cast function to coerce the value before - * comparison. + * See if we know how to compare two values, and create a new hash entry + * if not. */ static RI_CompareHashEntry * ri_HashCompareOp(Oid eq_opr, Oid typeid) @@ -3957,15 +3909,8 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid) * moment since that will never be generated for implicit coercions. */ op_input_types(eq_opr, &lefttype, &righttype); - - /* - * pf_eq_oprs (used by the fast path) can be cross-type when the FK - * and PK columns differ in type, e.g. int48eq for int4 PK / int8 FK. - * If the FK column's type, or the base type of a domain over it, - * already matches what the operator expects as its right-hand input, - * no cast is needed. - */ - if (getBaseType(typeid) == righttype) + Assert(lefttype == righttype); + if (typeid == lefttype) castfunc = InvalidOid; /* simplest case */ else { @@ -4055,7 +4000,6 @@ AtEOXact_RI(bool isCommit) FastPathMeta *dead = ri_fpmeta_dead_list; ri_fpmeta_dead_list = dead->next_dead; - MemoryContextDelete(dead->scratch_cxt); pfree(dead); } } -- 2.47.3