agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedPG19: two RI fast-path issues found while testing the batching revert
16+ messages / 4 participants
[nested] [flat]
* PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-10 16:02 Nikolay Samokhvalov <nik@postgres.ai>
0 siblings, 1 reply; 16+ messages in thread
From: Nikolay Samokhvalov @ 2026-09-10 16:02 UTC (permalink / raw)
To: pgsql-hackers <pgsql-hackers@lists.postgresql.org>; +Cc: Andrey Borodin <amborodin@acm.org>; Kirk Wolak <wolakk@gmail.com>; amitlangote09@gmail.com
Hi hackers,
After talking to Andrey Borodin yesterday, we thought it would be
useful to check for remaining issues after the RI batching revert. I
used the harness I'm building for general testing of new Postgres
features. It found no issues caused by the revert itself, but found
these two apparently pre-existing bugs that I think should be fixed.
I haven't had time to verify the findings myself or fully read the
output. This is the first time I'm sending a report without doing
that. I still think it's useful given the circumstances, and my
confidence is fairly high: the harness is designed to look for false
positives and try to refute its findings.
Both reproducers were run against compiled REL_19_STABLE at
5dec175fb4, with and without the v5 batching-removal series.
Column-level SELECT rejected by the FK fast path
Run as superuser:
begin;
create role fk_owner;
create schema fk_test authorization fk_owner;
set role fk_owner;
set search_path = fk_test, pg_catalog;
create table p (id int primary key, payload text);
insert into p values (1, 'x');
create table f (id int references p);
revoke select on p from fk_owner;
grant select (id) on p to fk_owner;
select 1 from p where id = 1 for key share; -- succeeds
insert into f values (1);
-- ERROR: permission denied for table p
rollback;
The referenced-table owner has SELECT on the key column. The
partitioned-parent SPI path accepts the same grants, but
ri_CheckPermissions() checks only table-level SELECT.
FK insert uses a dropped cast function
Run in one session:
begin;
create schema cast_test;
set local search_path = cast_test, pg_catalog;
create type k as (v int);
create function cast1(k) returns int
language sql immutable strict as 'select $1.v';
create cast (k as int) with function cast1(k) as implicit;
create table p (id int primary key);
create table f (id k references p);
insert into p values (1);
insert into f values (row(1)::k);
drop cast (k as int);
create function cast2(k) returns int
language sql immutable strict as 'select $1.v';
create cast (k as int) with function cast2(k) as implicit;
drop function cast1(k);
select row(1)::k::int; -- returns 1
insert into f values (row(1)::k);
-- ERROR: cache lookup failed for function <old cast1 OID>
rollback;
Both cast functions have identical behavior, and the DDL succeeds
without cascade. Looks like the RI cast cache isn't invalidated.
Nik
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-10 23:41 Amit Langote <amitlangote09@gmail.com>
parent: Nikolay Samokhvalov <nik@postgres.ai>
0 siblings, 1 reply; 16+ messages in thread
From: Amit Langote @ 2026-09-10 23:41 UTC (permalink / raw)
To: Nikolay Samokhvalov <nik@postgres.ai>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Andrey Borodin <amborodin@acm.org>; Kirk Wolak <wolakk@gmail.com>
Hi Nik,
On Fri, Sep 11, 2026 at 1:02 AM Nikolay Samokhvalov <nik@postgres.ai> wrote:
>
> Hi hackers,
>
> After talking to Andrey Borodin yesterday, we thought it would be
> useful to check for remaining issues after the RI batching revert. I
> used the harness I'm building for general testing of new Postgres
> features. It found no issues caused by the revert itself, but found
> these two apparently pre-existing bugs that I think should be fixed.
>
> I haven't had time to verify the findings myself or fully read the
> output. This is the first time I'm sending a report without doing
> that. I still think it's useful given the circumstances, and my
> confidence is fairly high: the harness is designed to look for false
> positives and try to refute its findings.
Thanks for doing this.
> Both reproducers were run against compiled REL_19_STABLE at
> 5dec175fb4, with and without the v5 batching-removal series.
>
> Column-level SELECT rejected by the FK fast path
>
> Run as superuser:
>
> begin;
> create role fk_owner;
> create schema fk_test authorization fk_owner;
> set role fk_owner;
> set search_path = fk_test, pg_catalog;
>
> create table p (id int primary key, payload text);
> insert into p values (1, 'x');
> create table f (id int references p);
>
> revoke select on p from fk_owner;
> grant select (id) on p to fk_owner;
>
> select 1 from p where id = 1 for key share; -- succeeds
> insert into f values (1);
> -- ERROR: permission denied for table p
> rollback;
>
> The referenced-table owner has SELECT on the key column. The
> partitioned-parent SPI path accepts the same grants, but
> ri_CheckPermissions() checks only table-level SELECT.
>
> FK insert uses a dropped cast function
>
> Run in one session:
>
> begin;
> create schema cast_test;
> set local search_path = cast_test, pg_catalog;
>
> create type k as (v int);
> create function cast1(k) returns int
> language sql immutable strict as 'select $1.v';
> create cast (k as int) with function cast1(k) as implicit;
>
> create table p (id int primary key);
> create table f (id k references p);
> insert into p values (1);
> insert into f values (row(1)::k);
>
> drop cast (k as int);
> create function cast2(k) returns int
> language sql immutable strict as 'select $1.v';
> create cast (k as int) with function cast2(k) as implicit;
> drop function cast1(k);
>
> select row(1)::k::int; -- returns 1
> insert into f values (row(1)::k);
> -- ERROR: cache lookup failed for function <old cast1 OID>
> rollback;
>
> Both cast functions have identical behavior, and the DDL succeeds
> without cascade. Looks like the RI cast cache isn't invalidated.
Looking at these now. The first issue is clearly a fast-path code
problem. The 2nd one interacts with the existing non-fast-path code so
I'll need to check if the bug predates fast-path.
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-11 00:33 Nikolay Samokhvalov <nik@postgres.ai>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Nikolay Samokhvalov @ 2026-09-11 00:33 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Andrey Borodin <amborodin@acm.org>; Kirk Wolak <wolakk@gmail.com>
On Thu, Sep 10, 2026 at 4:41 PM Amit Langote wrote:
> Looking at these now. The first issue is clearly a fast-path code
> problem. The 2nd one interacts with the existing non-fast-path code so
> I'll need to check if the bug predates fast-path.
Thanks Amit. In case helpful, here are two proposed fixes, with
regression tests.
Built and tested with assertions; regression and isolation suites pass.
An independent agent reviewed and tested both, catching a cleanup issue
that's now fixed. I didn't have time to fully study the patches manually,
but my harness tested them thoroughly.
Nik
Attachments:
[application/octet-stream] 0001-ri-column-select.patch (7.3K, ../../CAM527d87ebV6ES2m_HTmWS_8j3TjuEYokic1=UOkWHUB9OY2zg@mail.gmail.com/2-0001-ri-column-select.patch)
download | inline diff:
From 92f9b9f75e73b14ebff1330baefdee0d4bec9427 Mon Sep 17 00:00:00 2001
From: Nik Samokhvalov <nik@postgres.ai>
Date: Thu, 10 Sep 2026 15:42:23 -0700
Subject: [PATCH 1/2] Honor column-level SELECT privileges in RI fast-path
checks
The referenced table's owner can have SELECT on all referenced columns
without having table-level SELECT. Accept these column privileges in the
fast path, as the SPI query does, instead of rejecting a valid FK check.
Check only the referenced columns, not other index or table attributes.
Keep the schema check and the table-level privilege fast path unchanged.
Extend the foreign_key ACL tests to cover single and composite keys,
partial and unrelated column grants, and revocation after a successful
check. The composite case also covers differing attribute/index order
and an INCLUDE column without SELECT privilege.
---
src/backend/utils/adt/ri_triggers.c | 25 +++++++++++++-----
src/test/regress/expected/foreign_key.out | 31 +++++++++++++++++++++++
src/test/regress/sql/foreign_key.sql | 30 ++++++++++++++++++++++
3 files changed, 79 insertions(+), 7 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 6958f99..8c8edc1 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -290,7 +290,8 @@ static bool ri_FastPathProbeOne(Relation pk_rel, Relation idx_rel,
static bool ri_LockPKTuple(Relation pk_rel, TupleTableSlot *slot, Snapshot snap,
bool *concurrently_updated);
static bool ri_fastpath_is_applicable(const RI_ConstraintInfo *riinfo);
-static void ri_CheckPermissions(Relation query_rel);
+static void ri_CheckPermissions(Relation query_rel,
+ const RI_ConstraintInfo *riinfo);
static bool recheck_matched_pk_tuple(Relation idxrel, ScanKeyData *skeys,
int nkeys, TupleTableSlot *new_slot);
static void build_index_scankeys(const RI_ConstraintInfo *riinfo,
@@ -2802,7 +2803,7 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo,
saved_sec_context |
SECURITY_LOCAL_USERID_CHANGE |
SECURITY_NOFORCE_RLS);
- ri_CheckPermissions(pk_rel);
+ ri_CheckPermissions(pk_rel, riinfo);
/*
* Begin the scan under the switched user id, so that any access method
@@ -2989,10 +2990,10 @@ ri_fastpath_is_applicable(const RI_ConstraintInfo *riinfo)
/*
* ri_CheckPermissions
* Check that the current user has permissions to look into the schema of
- * and SELECT from 'query_rel'
+ * and SELECT the referenced columns of 'query_rel'
*/
static void
-ri_CheckPermissions(Relation query_rel)
+ri_CheckPermissions(Relation query_rel, const RI_ConstraintInfo *riinfo)
{
AclResult aclresult;
@@ -3007,9 +3008,19 @@ ri_CheckPermissions(Relation query_rel)
/* SELECT on relation. */
aclresult = pg_class_aclcheck(RelationGetRelid(query_rel), GetUserId(),
ACL_SELECT);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, OBJECT_TABLE,
- RelationGetRelationName(query_rel));
+ if (aclresult == ACLCHECK_OK)
+ return;
+
+ /* Otherwise, require SELECT on each referenced column, as SPI does. */
+ for (int i = 0; i < riinfo->nkeys; i++)
+ {
+ aclresult = pg_attribute_aclcheck(RelationGetRelid(query_rel),
+ riinfo->pk_attnums[i], GetUserId(),
+ ACL_SELECT);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, OBJECT_TABLE,
+ RelationGetRelationName(query_rel));
+ }
}
/*
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 9cea669..3386a17 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -413,6 +413,37 @@ REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
-- Inserting into FKTABLE should fail
INSERT INTO FKTABLE VALUES (2, 6);
ERROR: permission denied for table pktable
+-- Column-level SELECT on the referenced key should suffice.
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+DROP TABLE FKTABLE;
+DROP TABLE PKTABLE;
+-- Check column-level privileges with differing key and attribute orders.
+CREATE TABLE PKTABLE (payload text, ptest1 int, ptest2 int,
+ PRIMARY KEY (ptest2, ptest1) INCLUDE (payload));
+CREATE TABLE FKTABLE (ftest1 int, ftest2 int,
+ FOREIGN KEY (ftest1, ftest2) REFERENCES PKTABLE (ptest1, ptest2));
+INSERT INTO PKTABLE VALUES ('test', 1, 2);
+ALTER TABLE PKTABLE OWNER TO regress_foreign_key_user;
+REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
+-- An unrelated column grant is not enough.
+GRANT SELECT (payload) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (1, 2);
+ERROR: permission denied for table pktable
+-- Nor is a grant on only one of the referenced columns.
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (1, 2);
+ERROR: permission denied for table pktable
+-- Both key columns suffice, without access to the included column.
+REVOKE SELECT (payload) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (1, 2);
+-- Recheck privileges even after a successful check.
+REVOKE SELECT (ptest1) ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (1, 2);
+ERROR: permission denied for table pktable
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (1, 2);
DROP TABLE FKTABLE;
DROP TABLE PKTABLE;
DROP USER regress_foreign_key_user;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 07d8921..2f857ce 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -301,6 +301,36 @@ REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
-- Inserting into FKTABLE should fail
INSERT INTO FKTABLE VALUES (2, 6);
+-- Column-level SELECT on the referenced key should suffice.
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+
+DROP TABLE FKTABLE;
+DROP TABLE PKTABLE;
+
+-- Check column-level privileges with differing key and attribute orders.
+CREATE TABLE PKTABLE (payload text, ptest1 int, ptest2 int,
+ PRIMARY KEY (ptest2, ptest1) INCLUDE (payload));
+CREATE TABLE FKTABLE (ftest1 int, ftest2 int,
+ FOREIGN KEY (ftest1, ftest2) REFERENCES PKTABLE (ptest1, ptest2));
+INSERT INTO PKTABLE VALUES ('test', 1, 2);
+ALTER TABLE PKTABLE OWNER TO regress_foreign_key_user;
+REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
+-- An unrelated column grant is not enough.
+GRANT SELECT (payload) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (1, 2);
+-- Nor is a grant on only one of the referenced columns.
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (1, 2);
+-- Both key columns suffice, without access to the included column.
+REVOKE SELECT (payload) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (1, 2);
+-- Recheck privileges even after a successful check.
+REVOKE SELECT (ptest1) ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (1, 2);
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (1, 2);
DROP TABLE FKTABLE;
DROP TABLE PKTABLE;
--
2.50.1 (Apple Git-155)
[application/octet-stream] 0002-ri-cast-cache.patch (19.3K, ../../CAM527d87ebV6ES2m_HTmWS_8j3TjuEYokic1=UOkWHUB9OY2zg@mail.gmail.com/3-0002-ri-cast-cache.patch)
download | inline diff:
From 94bbd92222c2aac3c07fb84e769eb41435356916 Mon Sep 17 00:00:00 2001
From: Nik Samokhvalov <nik@postgres.ai>
Date: Thu, 10 Sep 2026 15:44:08 -0700
Subject: [PATCH 2/2] Invalidate RI call information when casts change
Cast replacement need not modify pg_constraint, so the constraint-cache
callback does not refresh the cast functions cached by RI comparisons and
fast-path foreign-key checks. Dropping the old function after an equivalent
replacement can make a valid insert fail with a stale function OID.
Invalidate both caches on CASTSOURCETARGET changes. Keep comparison call
information separate from its hash entry and defer releasing invalidated
objects until transaction end, as already done for fast-path metadata.
A cast can run DDL and reenter RI checks, so invalidation must not free or
overwrite call information still used by an outer comparison. This retains
function-local caching without adding per-row function-info copies.
Keep unfinished fast-path metadata in a transaction-owned context until
construction succeeds, so failed rebuilds do not leak backend memory.
Add regression coverage for replacement after INSERT/UPDATE cache warmup,
invalid-key rejection, rollback restoring the old cast, and invalidation
with a nested RI check during a comparison. Check that failed rebuilds
release their provisional contexts on subtransaction abort.
---
src/backend/utils/adt/ri_triggers.c | 137 +++++++++++++++-------
src/test/regress/expected/foreign_key.out | 100 ++++++++++++++++
src/test/regress/sql/foreign_key.sql | 85 ++++++++++++++
src/tools/pgindent/typedefs.list | 1 +
4 files changed, 281 insertions(+), 42 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 8c8edc1..6abd381 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -164,7 +164,7 @@ typedef struct FastPathMeta
* 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
+ * of their own, which also owns this struct and is destroyed by
* AtEOXact_RI().
*
* Note this context must not be reset while the FmgrInfos remain in use,
@@ -207,15 +207,25 @@ typedef struct RI_CompareKey
Oid typeid; /* the data type to apply it to */
} RI_CompareKey;
+/*
+ * Cached call information is detached on invalidation, but kept until the end
+ * of the transaction in case an active comparison still references it.
+ */
+typedef struct RI_CompareInfo
+{
+ FmgrInfo eq_opr_finfo; /* call info for equality fn */
+ FmgrInfo cast_func_finfo; /* in case we must coerce input */
+ MemoryContext context;
+ struct RI_CompareInfo *next_dead;
+} RI_CompareInfo;
+
/*
* RI_CompareHashEntry
*/
typedef struct RI_CompareHashEntry
{
RI_CompareKey key;
- bool valid; /* successfully initialized? */
- FmgrInfo eq_opr_finfo; /* call info for equality fn */
- FmgrInfo cast_func_finfo; /* in case we must coerce input */
+ RI_CompareInfo *info; /* NULL if invalid */
} RI_CompareHashEntry;
/*
@@ -233,6 +243,7 @@ static dclist_head ri_constraint_cache_valid_list;
* InvalidateConstraintCacheCallBack().
*/
static FastPathMeta *ri_fpmeta_dead_list = NULL;
+static RI_CompareInfo *ri_compare_dead_list = NULL;
/*
* Local function prototypes
@@ -261,11 +272,13 @@ static bool ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid,
Datum lhs, Datum rhs);
static void ri_InitHashTables(void);
+static void InvalidateCastCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
+ uint32 hashvalue);
static void InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
uint32 hashvalue);
static SPIPlanPtr ri_FetchPreparedPlan(RI_QueryKey *key);
static void ri_HashPreparedPlan(RI_QueryKey *key, SPIPlanPtr plan);
-static RI_CompareHashEntry *ri_HashCompareOp(Oid eq_opr, Oid typeid);
+static RI_CompareInfo *ri_HashCompareOp(Oid eq_opr, Oid typeid);
static void ri_CheckTrigger(FunctionCallInfo fcinfo, const char *funcname,
int tgkind);
@@ -2548,6 +2561,34 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
}
+/*
+ * Cast changes can affect any comparison or fast-path entry. Do not free or
+ * overwrite call information here: a cast can execute DDL and reenter RI checks
+ * while an outer call is still using it. AtEOXact_RI() releases detached data.
+ */
+static void
+InvalidateCastCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
+ uint32 hashvalue)
+{
+ HASH_SEQ_STATUS status;
+ RI_CompareHashEntry *entry;
+
+ hash_seq_init(&status, ri_compare_cache);
+ while ((entry = hash_seq_search(&status)) != NULL)
+ {
+ if (entry->info != NULL)
+ {
+ entry->info->next_dead = ri_compare_dead_list;
+ ri_compare_dead_list = entry->info;
+ entry->info = NULL;
+ }
+ }
+
+ /* Fast-path metadata contains copies of the cached call information. */
+ InvalidateConstraintCacheCallBack(arg, cacheid, 0);
+}
+
+
/*
* Prepare execution plan for a query to enforce an RI restriction
*/
@@ -3143,24 +3184,23 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo,
Relation fk_rel, Relation idx_rel)
{
FastPathMeta *fpmeta;
- MemoryContext oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+ MemoryContext context;
Assert(riinfo != NULL && riinfo->valid);
Assert(riinfo->fpmeta == NULL);
- 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);
+ /* 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++)
{
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);
+ RI_CompareInfo *entry = ri_HashCompareOp(eq_opr, typeid);
int idx_col;
/*
@@ -3193,8 +3233,8 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo,
&fpmeta->subtypes[i]);
}
+ MemoryContextSetParent(context, TopMemoryContext);
riinfo->fpmeta = fpmeta;
- MemoryContextSwitchTo(oldcxt);
}
/*
@@ -3465,6 +3505,10 @@ ri_InitHashTables(void)
ri_compare_cache = hash_create("RI compare cache",
RI_INIT_QUERYHASHSIZE,
&ctl, HASH_ELEM | HASH_BLOBS);
+
+ CacheRegisterSyscacheCallback(CASTSOURCETARGET,
+ InvalidateCastCacheCallBack,
+ (Datum) 0);
}
@@ -3653,7 +3697,7 @@ static bool
ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid,
Datum lhs, Datum rhs)
{
- RI_CompareHashEntry *entry = ri_HashCompareOp(eq_opr, typeid);
+ RI_CompareInfo *entry = ri_HashCompareOp(eq_opr, typeid);
/* Do we need to cast the values? */
if (OidIsValid(entry->cast_func_finfo.fn_oid))
@@ -3698,7 +3742,7 @@ ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid,
* its right-hand input, a cast function to coerce the value before
* comparison.
*/
-static RI_CompareHashEntry *
+static RI_CompareInfo *
ri_HashCompareOp(Oid eq_opr, Oid typeid)
{
RI_CompareKey key;
@@ -3721,23 +3765,20 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
&key,
HASH_ENTER, &found);
if (!found)
- entry->valid = false;
+ entry->info = NULL;
/*
- * If not already initialized, do so. Since we'll keep this hash entry
- * for the life of the backend, put any subsidiary info for the function
- * cache structs into TopMemoryContext.
+ * If not already initialized, build a new generation of call information.
+ * Use a separate context so invalidation cannot affect active callers.
*/
- if (!entry->valid)
+ if (entry->info == NULL)
{
Oid lefttype,
righttype,
castfunc;
CoercionPathType pathtype;
-
- /* We always need to know how to call the equality operator */
- fmgr_info_cxt(get_opcode(eq_opr), &entry->eq_opr_finfo,
- TopMemoryContext);
+ MemoryContext context;
+ RI_CompareInfo *info;
/*
* If we chose to use a cast from FK to PK type, we may have to apply
@@ -3782,15 +3823,22 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
format_type_be(lefttype));
}
}
+ /* Leave incomplete entries subject to normal error cleanup. */
+ context = AllocSetContextCreate(CurTransactionContext,
+ "RI compare info",
+ ALLOCSET_SMALL_SIZES);
+ info = MemoryContextAllocZero(context, sizeof(RI_CompareInfo));
+ info->context = context;
+ fmgr_info_cxt(get_opcode(eq_opr), &info->eq_opr_finfo, context);
if (OidIsValid(castfunc))
- fmgr_info_cxt(castfunc, &entry->cast_func_finfo,
- TopMemoryContext);
+ fmgr_info_cxt(castfunc, &info->cast_func_finfo, context);
else
- entry->cast_func_finfo.fn_oid = InvalidOid;
- entry->valid = true;
+ info->cast_func_finfo.fn_oid = InvalidOid;
+ MemoryContextSetParent(context, TopMemoryContext);
+ entry->info = info;
}
- return entry;
+ return entry->info;
}
@@ -3827,30 +3875,35 @@ RI_FKey_trigger_type(Oid tgfoid)
* AtEOXact_RI
* End-of-transaction cleanup for referential integrity.
*
- * Currently this only releases fast-path metadata detached during the
- * transaction. InvalidateConstraintCacheCallBack() cannot free a
- * FastPathMeta when it detaches one, because an RI check further up the
- * stack may still hold a pointer into it. It queues them on
- * ri_fpmeta_dead_list instead, and we release them here, where no such
- * reference can exist. isCommit is accepted for consistency with the
- * other AtEOXact_* routines but is not used: the release is the same on
- * the commit and the abort path.
+ * Release comparison and fast-path call information detached during the
+ * transaction. Invalidation callbacks cannot free it immediately, because
+ * an RI check further up the stack may still hold a pointer into it.
+ * We release the queued objects here, where no such reference can exist.
+ * isCommit is accepted for consistency with the other AtEOXact_* routines
+ * but is not used: the release is the same on the commit and the abort path.
*
* There is no AtEOSubXact_RI() counterpart. Nothing here is scoped to a
- * subtransaction: a detached FastPathMeta stays reachable from the dead
- * list whichever subtransaction detached it, and a check holding a pointer
+ * subtransaction: detached call information stays reachable from the dead
+ * lists whichever subtransaction detached it, and a check holding a pointer
* into one may be running at an outer level, so releasing at subtransaction
* end would be unsafe as well as unnecessary.
*/
void
AtEOXact_RI(bool isCommit)
{
+ while (ri_compare_dead_list != NULL)
+ {
+ RI_CompareInfo *dead = ri_compare_dead_list;
+
+ ri_compare_dead_list = dead->next_dead;
+ MemoryContextDelete(dead->context);
+ }
+
while (ri_fpmeta_dead_list != NULL)
{
FastPathMeta *dead = ri_fpmeta_dead_list;
ri_fpmeta_dead_list = dead->next_dead;
MemoryContextDelete(dead->scratch_cxt);
- pfree(dead);
}
}
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 3386a17..70c05d9 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -3948,3 +3948,103 @@ DROP TYPE fkint CASCADE;
NOTICE: drop cascades to 2 other objects
DETAIL: drop cascades to function fkint_in(cstring)
drop cascades to function fkint_out(fkint)
+-- Replacing a cast must invalidate both comparison and fast-path caches.
+BEGIN;
+CREATE TYPE fk_cast_type AS (v int);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type;
+SAVEPOINT original_cast;
+DROP CAST (fk_cast_type AS int);
+CREATE FUNCTION fk_cast2(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast2(fk_cast_type) AS IMPLICIT;
+DROP FUNCTION fk_cast1(fk_cast_type);
+INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type;
+SAVEPOINT invalid_key;
+INSERT INTO fk_cast_fk VALUES (ROW(2)::fk_cast_type); -- must fail
+ERROR: insert or update on table "fk_cast_fk" violates foreign key constraint "fk_cast_fk_id_fkey"
+DETAIL: Key (id)=((2)) is not present in table "fk_cast_pk".
+ROLLBACK TO invalid_key;
+-- Restoring the old cast must invalidate the replacement's cache entries too.
+ROLLBACK TO original_cast;
+-- Failed rebuilds must not accumulate metadata across subtransaction aborts.
+SAVEPOINT missing_cast;
+DROP CAST (fk_cast_type AS int);
+DO $$
+DECLARE
+ before_count bigint;
+ after_count bigint;
+BEGIN
+ SELECT count(*) INTO before_count FROM pg_backend_memory_contexts
+ WHERE name LIKE 'RI %';
+ FOR i IN 1..10 LOOP
+ BEGIN
+ INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type);
+ RAISE EXCEPTION 'missing cast was not detected';
+ EXCEPTION WHEN internal_error THEN
+ IF SQLERRM NOT LIKE 'no conversion function%' THEN
+ RAISE;
+ END IF;
+ END;
+ END LOOP;
+ SELECT count(*) INTO after_count FROM pg_backend_memory_contexts
+ WHERE name LIKE 'RI %';
+ IF after_count > before_count THEN
+ RAISE EXCEPTION 'RI contexts leaked across failed checks';
+ END IF;
+END $$;
+ROLLBACK TO missing_cast;
+INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type;
+SELECT count(*) FROM fk_cast_fk;
+ count
+-------
+ 2
+(1 row)
+
+ROLLBACK;
+-- A cast invalidation and nested RI check must not overwrite call information
+-- still being used by the outer comparison.
+BEGIN;
+CREATE TYPE fk_cast_type AS (v int);
+CREATE TABLE fk_cast_guard (armed bool);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int LANGUAGE plpgsql AS $$
+BEGIN
+ IF EXISTS (SELECT FROM fk_cast_guard) THEN
+ DELETE FROM fk_cast_guard;
+ EXECUTE 'DROP CAST (fk_cast_type AS bigint)';
+ INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type);
+ END IF;
+ RETURN $1.v;
+END $$;
+CREATE FUNCTION fk_cast_aux(fk_cast_type) RETURNS bigint
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v::bigint';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE CAST (fk_cast_type AS bigint) WITH FUNCTION fk_cast_aux(fk_cast_type);
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type;
+INSERT INTO fk_cast_guard VALUES (true);
+UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type;
+SELECT count(*) FROM fk_cast_fk;
+ count
+-------
+ 2
+(1 row)
+
+SELECT count(*) FROM fk_cast_guard;
+ count
+-------
+ 0
+(1 row)
+
+ROLLBACK;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 2f857ce..ec46ec5 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -2900,3 +2900,88 @@ DROP TABLE pktable_inval;
DROP CAST (fkint AS int4);
DROP FUNCTION fkint_to_int4(fkint);
DROP TYPE fkint CASCADE;
+
+-- Replacing a cast must invalidate both comparison and fast-path caches.
+BEGIN;
+CREATE TYPE fk_cast_type AS (v int);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type;
+SAVEPOINT original_cast;
+DROP CAST (fk_cast_type AS int);
+CREATE FUNCTION fk_cast2(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast2(fk_cast_type) AS IMPLICIT;
+DROP FUNCTION fk_cast1(fk_cast_type);
+INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type;
+SAVEPOINT invalid_key;
+INSERT INTO fk_cast_fk VALUES (ROW(2)::fk_cast_type); -- must fail
+ROLLBACK TO invalid_key;
+-- Restoring the old cast must invalidate the replacement's cache entries too.
+ROLLBACK TO original_cast;
+-- Failed rebuilds must not accumulate metadata across subtransaction aborts.
+SAVEPOINT missing_cast;
+DROP CAST (fk_cast_type AS int);
+DO $$
+DECLARE
+ before_count bigint;
+ after_count bigint;
+BEGIN
+ SELECT count(*) INTO before_count FROM pg_backend_memory_contexts
+ WHERE name LIKE 'RI %';
+ FOR i IN 1..10 LOOP
+ BEGIN
+ INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type);
+ RAISE EXCEPTION 'missing cast was not detected';
+ EXCEPTION WHEN internal_error THEN
+ IF SQLERRM NOT LIKE 'no conversion function%' THEN
+ RAISE;
+ END IF;
+ END;
+ END LOOP;
+ SELECT count(*) INTO after_count FROM pg_backend_memory_contexts
+ WHERE name LIKE 'RI %';
+ IF after_count > before_count THEN
+ RAISE EXCEPTION 'RI contexts leaked across failed checks';
+ END IF;
+END $$;
+ROLLBACK TO missing_cast;
+INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type;
+SELECT count(*) FROM fk_cast_fk;
+ROLLBACK;
+
+-- A cast invalidation and nested RI check must not overwrite call information
+-- still being used by the outer comparison.
+BEGIN;
+CREATE TYPE fk_cast_type AS (v int);
+CREATE TABLE fk_cast_guard (armed bool);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int LANGUAGE plpgsql AS $$
+BEGIN
+ IF EXISTS (SELECT FROM fk_cast_guard) THEN
+ DELETE FROM fk_cast_guard;
+ EXECUTE 'DROP CAST (fk_cast_type AS bigint)';
+ INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type);
+ END IF;
+ RETURN $1.v;
+END $$;
+CREATE FUNCTION fk_cast_aux(fk_cast_type) RETURNS bigint
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v::bigint';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE CAST (fk_cast_type AS bigint) WITH FUNCTION fk_cast_aux(fk_cast_type);
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES (ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type;
+INSERT INTO fk_cast_guard VALUES (true);
+UPDATE fk_cast_fk SET id = ROW(1)::fk_cast_type;
+SELECT count(*) FROM fk_cast_fk;
+SELECT count(*) FROM fk_cast_guard;
+ROLLBACK;
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index e11d3ae..58a395d 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2491,6 +2491,7 @@ RBTreeIterator
REPARSE_JUNCTION_DATA_BUFFER
RIX
RI_CompareHashEntry
+RI_CompareInfo
RI_CompareKey
RI_ConstraintInfo
RI_QueryHashEntry
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-11 09:25 Amit Langote <amitlangote09@gmail.com>
parent: Nikolay Samokhvalov <nik@postgres.ai>
0 siblings, 3 replies; 16+ messages in thread
From: Amit Langote @ 2026-09-11 09:25 UTC (permalink / raw)
To: Nikolay Samokhvalov <nik@postgres.ai>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Andrey Borodin <amborodin@acm.org>; Kirk Wolak <wolakk@gmail.com>
On Fri, Sep 11, 2026 at 9:33 AM Nikolay Samokhvalov <nik@postgres.ai> wrote:
>
> On Thu, Sep 10, 2026 at 4:41 PM Amit Langote wrote:
> > Looking at these now. The first issue is clearly a fast-path code
> > problem. The 2nd one interacts with the existing non-fast-path code so
> > I'll need to check if the bug predates fast-path.
>
> Thanks Amit. In case helpful, here are two proposed fixes, with
> regression tests.
>
> Built and tested with assertions; regression and isolation suites pass.
> An independent agent reviewed and tested both, catching a cleanup issue
> that's now fixed. I didn't have time to fully study the patches manually,
> but my harness tested them thoroughly.
Thanks, Nik. Attached are updated patches incorporating your fixes.
For 0001, SPI's FOR KEY SHARE also requires UPDATE privilege on at
least one column. I've used ExecCheckOneRelPerms() to cover that along
with column-level SELECT. The tests exercise both per-row and batched
checks, including rejection without UPDATE and acceptance with UPDATE
on an unrelated column.
For #2, I reproduced the stale cast cache on 18.6 by warming it with
an UPDATE of a committed row before replacing the cast. I've adjusted
the tests to use committed rows, since same-transaction rows bypass
the key comparison. The nested case now also uses UPDATE to exercise
the comparison cache on older branches.
The cleanup strategy in 0002 deserves some discussion. It retains
invalidated call information until transaction end because a cast can
invalidate the cache and re-enter RI checks while an outer comparison
still uses it. I've carried that approach into the backpatch, but this
means introducing AtEOXact_RI() on pre-19 branches. I'd welcome closer
review before settling on that strategy. Could we replace the dead
list and explicit cleanup with reparenting to TopTransactionContext
when an entry is invalidated? That would avoid the new hook, but needs
checking against invalidation timing.
There are separate versions of 0001 and 0002 for master and
REL_19_STABLE. The two versions of 0002 contain the same fix and
tests, adapted to each branch's surrounding code. A shared version of
0002 applies to branches 14 through 18, which have no fast-path code.
--
Thanks, Amit Langote
Attachments:
[application/octet-stream] master-v2-0001-Fix-RI-fast-path-permission-checks.patch (12.0K, ../../CA+HiwqGAq2fqXDSOUzE-uv4-LNMDcu6zHCF8co6=aRBZaTnoQg@mail.gmail.com/2-master-v2-0001-Fix-RI-fast-path-permission-checks.patch)
download | inline diff:
From 14197072f607d53d79e73a41dba76f83445bc78a Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlangote09@gmail.com>
Date: Fri, 11 Sep 2026 12:14:20 +0900
Subject: [PATCH v2 1/2] Fix RI fast-path permission checks
The fast path required table-level SELECT on the referenced table,
rejecting checks that the SPI path allows with column-level grants.
It also omitted the UPDATE privilege required by FOR KEY SHARE.
When table privileges do not suffice, use ExecCheckOneRelPerms() with
the referenced key columns as selectedCols and an empty updatedCols.
This accepts SELECT on all referenced columns and UPDATE on any column,
matching the SPI query. Keep the table-privilege check as a shortcut
that avoids constructing a column bitmap in the usual case.
Extend the ACL regression tests to cover column grants, privilege
revocation, and per-row validation of a composite foreign key.
Co-authored-by: Nikolay Samokhvalov <nik@postgres.ai>
Backpatch-through: 19
---
src/backend/utils/adt/ri_triggers.c | 47 +++++++++++++-----
src/test/regress/expected/foreign_key.out | 55 ++++++++++++++++++++-
src/test/regress/sql/foreign_key.sql | 59 ++++++++++++++++++++++-
3 files changed, 146 insertions(+), 15 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index ea94b84ffc9..c62f7dc2d06 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -384,7 +384,8 @@ static bool ri_FastPathProbeOne(Relation pk_rel, Relation idx_rel,
static bool ri_LockPKTuple(Relation pk_rel, TupleTableSlot *slot, Snapshot snap,
bool *concurrently_updated);
static bool ri_fastpath_is_applicable(const RI_ConstraintInfo *riinfo);
-static void ri_CheckPermissions(Relation query_rel);
+static void ri_CheckPermissions(const RI_ConstraintInfo *riinfo,
+ Relation query_rel);
static bool recheck_matched_pk_tuple(Relation idxrel, ScanKeyData *skeys,
int nkeys, TupleTableSlot *new_slot);
static void build_index_scankeys(const RI_ConstraintInfo *riinfo,
@@ -2920,7 +2921,7 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo,
saved_sec_context |
SECURITY_LOCAL_USERID_CHANGE |
SECURITY_NOFORCE_RLS);
- ri_CheckPermissions(pk_rel);
+ ri_CheckPermissions(riinfo, pk_rel);
/*
* Begin the scan under the switched user id, so that any access method
@@ -3082,7 +3083,7 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel,
* albeit checked once per flush rather than once per row, like in
* ri_FastPathCheck().
*/
- ri_CheckPermissions(pk_rel);
+ ri_CheckPermissions(riinfo, pk_rel);
/*
* Begin the scan under the switched user id, so that any access method
@@ -3505,13 +3506,16 @@ ri_fastpath_is_applicable(const RI_ConstraintInfo *riinfo)
/*
* ri_CheckPermissions
- * Check that the current user has permissions to look into the schema of
- * and SELECT from 'query_rel'
+ * Check permissions for the SELECT ... FOR KEY SHARE used by the SPI
+ * path, as the referenced table's owner.
*/
static void
-ri_CheckPermissions(Relation query_rel)
+ri_CheckPermissions(const RI_ConstraintInfo *riinfo, Relation query_rel)
{
AclResult aclresult;
+ AclMode requiredPerms = ACL_SELECT | ACL_SELECT_FOR_UPDATE;
+ RTEPermissionInfo *perminfo;
+ bool result;
/* USAGE on schema. */
aclresult = object_aclcheck(NamespaceRelationId,
@@ -3521,11 +3525,32 @@ ri_CheckPermissions(Relation query_rel)
aclcheck_error(aclresult, OBJECT_SCHEMA,
get_namespace_name(RelationGetNamespace(query_rel)));
- /* SELECT on relation. */
- aclresult = pg_class_aclcheck(RelationGetRelid(query_rel), GetUserId(),
- ACL_SELECT);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, OBJECT_TABLE,
+ /* Avoid building the column bitmap when table privileges suffice. */
+ if (pg_class_aclmask(RelationGetRelid(query_rel), GetUserId(),
+ requiredPerms, ACLMASK_ALL) == requiredPerms)
+ return;
+
+ /*
+ * SELECT is needed only on the referenced key columns. FOR KEY SHARE
+ * also needs UPDATE privilege, which may be granted on any column. Use
+ * the executor's checks for both, leaving updatedCols empty as the SPI
+ * query does.
+ */
+ perminfo = makeNode(RTEPermissionInfo);
+ perminfo->relid = RelationGetRelid(query_rel);
+ perminfo->requiredPerms = requiredPerms;
+ for (int i = 0; i < riinfo->nkeys; i++)
+ {
+ int attno = riinfo->pk_attnums[i] - FirstLowInvalidHeapAttributeNumber;
+
+ perminfo->selectedCols = bms_add_member(perminfo->selectedCols, attno);
+ }
+
+ result = ExecCheckOneRelPerms(perminfo);
+ bms_free(perminfo->selectedCols);
+ pfree(perminfo);
+ if (!result)
+ aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLE,
RelationGetRelationName(query_rel));
}
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 8d81240f1c6..c54a9895b55 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -402,17 +402,68 @@ CREATE TABLE FKTABLE ( ftest1 int REFERENCES PKTABLE, ftest2 int );
INSERT INTO PKTABLE VALUES (1, 'Test1');
INSERT INTO PKTABLE VALUES (2, 'Test2');
INSERT INTO PKTABLE VALUES (3, 'Test3');
--- Grant usage on PKTABLE to user regress_foreign_key_user
+-- Grant SELECT on PKTABLE to user regress_foreign_key_user
CREATE USER regress_foreign_key_user NOLOGIN;
GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
ALTER TABLE PKTABLE OWNER to regress_foreign_key_user;
-- Inserting into FKTABLE should work
INSERT INTO FKTABLE VALUES (3, 5);
--- Revoke usage on PKTABLE from user regress_foreign_key_user
+-- Revoke SELECT on PKTABLE from user regress_foreign_key_user
REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
-- Inserting into FKTABLE should fail
INSERT INTO FKTABLE VALUES (2, 6);
ERROR: permission denied for table pktable
+-- SELECT on the referenced key column is enough, without SELECT on ptest2.
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+-- SELECT on an unrelated column does not suffice.
+REVOKE SELECT (ptest1) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+ERROR: permission denied for table pktable
+REVOKE SELECT (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+-- FOR KEY SHARE also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+ERROR: permission denied for table pktable
+-- UPDATE on any column suffices, even one that the check does not read.
+GRANT UPDATE (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+-- Table-level SELECT can be combined with column-level UPDATE.
+GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+REVOKE UPDATE (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+ERROR: permission denied for table pktable
+DROP TABLE FKTABLE;
+DROP TABLE PKTABLE;
+-- Check all referenced columns, including when index and FK order differ.
+CREATE TABLE PKTABLE ( ptest0 text, ptest1 int, ptest2 int,
+ PRIMARY KEY (ptest2, ptest1) );
+CREATE TABLE FKTABLE ( ftest1 int, ftest2 int );
+INSERT INTO PKTABLE VALUES ('unused', 1, 2);
+INSERT INTO FKTABLE VALUES (1, 2);
+ALTER TABLE FKTABLE ADD CONSTRAINT fktable_fk
+ FOREIGN KEY (ftest1, ftest2) REFERENCES PKTABLE (ptest1, ptest2) NOT VALID;
+ALTER TABLE PKTABLE OWNER TO regress_foreign_key_user;
+ALTER TABLE FKTABLE OWNER TO regress_foreign_key_user;
+REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+-- Lack of SELECT on FKTABLE forces validation to check each row.
+REVOKE SELECT ON FKTABLE FROM regress_foreign_key_user;
+SET ROLE regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+ERROR: permission denied for table pktable
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+-- Per-row validation also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+ERROR: permission denied for table pktable
+-- UPDATE on the unrelated column is enough.
+GRANT UPDATE (ptest0) ON PKTABLE TO regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk;
+RESET ROLE;
DROP TABLE FKTABLE;
DROP TABLE PKTABLE;
DROP USER regress_foreign_key_user;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 184d9efdc97..c013d1f8834 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -286,7 +286,7 @@ INSERT INTO PKTABLE VALUES (1, 'Test1');
INSERT INTO PKTABLE VALUES (2, 'Test2');
INSERT INTO PKTABLE VALUES (3, 'Test3');
--- Grant usage on PKTABLE to user regress_foreign_key_user
+-- Grant SELECT on PKTABLE to user regress_foreign_key_user
CREATE USER regress_foreign_key_user NOLOGIN;
GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
@@ -295,12 +295,67 @@ ALTER TABLE PKTABLE OWNER to regress_foreign_key_user;
-- Inserting into FKTABLE should work
INSERT INTO FKTABLE VALUES (3, 5);
--- Revoke usage on PKTABLE from user regress_foreign_key_user
+-- Revoke SELECT on PKTABLE from user regress_foreign_key_user
REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
-- Inserting into FKTABLE should fail
INSERT INTO FKTABLE VALUES (2, 6);
+-- SELECT on the referenced key column is enough, without SELECT on ptest2.
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+
+-- SELECT on an unrelated column does not suffice.
+REVOKE SELECT (ptest1) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+REVOKE SELECT (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+
+-- FOR KEY SHARE also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+
+-- UPDATE on any column suffices, even one that the check does not read.
+GRANT UPDATE (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+
+-- Table-level SELECT can be combined with column-level UPDATE.
+GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+REVOKE UPDATE (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+
+DROP TABLE FKTABLE;
+DROP TABLE PKTABLE;
+
+-- Check all referenced columns, including when index and FK order differ.
+CREATE TABLE PKTABLE ( ptest0 text, ptest1 int, ptest2 int,
+ PRIMARY KEY (ptest2, ptest1) );
+CREATE TABLE FKTABLE ( ftest1 int, ftest2 int );
+INSERT INTO PKTABLE VALUES ('unused', 1, 2);
+INSERT INTO FKTABLE VALUES (1, 2);
+ALTER TABLE FKTABLE ADD CONSTRAINT fktable_fk
+ FOREIGN KEY (ftest1, ftest2) REFERENCES PKTABLE (ptest1, ptest2) NOT VALID;
+ALTER TABLE PKTABLE OWNER TO regress_foreign_key_user;
+ALTER TABLE FKTABLE OWNER TO regress_foreign_key_user;
+REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+
+-- Lack of SELECT on FKTABLE forces validation to check each row.
+REVOKE SELECT ON FKTABLE FROM regress_foreign_key_user;
+SET ROLE regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+
+-- Per-row validation also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+-- UPDATE on the unrelated column is enough.
+GRANT UPDATE (ptest0) ON PKTABLE TO regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk;
+RESET ROLE;
+
DROP TABLE FKTABLE;
DROP TABLE PKTABLE;
--
2.47.3
[application/octet-stream] master-v2-0002-Invalidate-RI-call-information-when-casts-change.patch (21.0K, ../../CA+HiwqGAq2fqXDSOUzE-uv4-LNMDcu6zHCF8co6=aRBZaTnoQg@mail.gmail.com/3-master-v2-0002-Invalidate-RI-call-information-when-casts-change.patch)
download | inline diff:
From 5d26d70e9d84a41899e4704335ac75e685f619f4 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Fri, 11 Sep 2026 17:01:08 +0900
Subject: [PATCH v2 2/2] Invalidate RI call information when casts change
Changing a cast need not modify pg_constraint, so the RI comparison
cache can keep using the old cast function after its replacement.
Dropping that function can then make an UPDATE fail with a cache lookup
error.
The fast-path foreign key checks added in PostgreSQL 19 also use this
cache and copy its call information into their own metadata, exposing
the problem on INSERT as well.
Invalidate both caches on CASTSOURCETARGET changes. Keep comparison call
information separate from its hash entry and defer releasing invalidated
objects until transaction end, as already done for fast-path metadata.
A cast can run DDL and reenter RI checks, so invalidation must not free
or overwrite call information still used by an outer comparison.
Build new comparison and fast-path call information in transaction-owned
contexts and reparent them only when construction succeeds, so failed
rebuilds do not leak backend memory.
Test replacement after cache warmup using a committed row, invalid-key
rejection, rollback restoring the old cast, and invalidation with a
nested comparison of another committed row. Also check cleanup after
failed fast-path rebuilds.
Author: Nikolay Samokhvalov <nik@postgres.ai>
Co-authored-by: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CAM527d9BgPjeOOYmbCBTd57R145qHCk-dzw9qNq+nOrDq1j__A@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/adt/ri_triggers.c | 130 ++++++++++++++++------
src/test/regress/expected/foreign_key.out | 117 +++++++++++++++++++
src/test/regress/sql/foreign_key.sql | 110 ++++++++++++++++++
src/tools/pgindent/typedefs.list | 1 +
4 files changed, 322 insertions(+), 36 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index c62f7dc2d06..1811a3c2ae4 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -165,7 +165,7 @@ typedef struct FastPathMeta
* 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
+ * of their own, which also owns this struct and is destroyed by
* AtEOXact_RI().
*
* Note this context must not be reset while the FmgrInfos remain in use,
@@ -208,15 +208,25 @@ typedef struct RI_CompareKey
Oid typeid; /* the data type to apply it to */
} RI_CompareKey;
+/*
+ * Cached call information is detached on invalidation, but kept until the end
+ * of the transaction in case an active comparison still references it.
+ */
+typedef struct RI_CompareInfo
+{
+ FmgrInfo eq_opr_finfo; /* call info for equality fn */
+ FmgrInfo cast_func_finfo; /* in case we must coerce input */
+ MemoryContext context;
+ struct RI_CompareInfo *next_dead;
+} RI_CompareInfo;
+
/*
* RI_CompareHashEntry
*/
typedef struct RI_CompareHashEntry
{
RI_CompareKey key;
- bool valid; /* successfully initialized? */
- FmgrInfo eq_opr_finfo; /* call info for equality fn */
- FmgrInfo cast_func_finfo; /* in case we must coerce input */
+ RI_CompareInfo *info; /* NULL if invalid */
} RI_CompareHashEntry;
/*
@@ -316,6 +326,9 @@ static bool ri_fastpath_flushing = false;
*/
static FastPathMeta *ri_fpmeta_dead_list = NULL;
+/* Comparison call information detached by InvalidateCastCacheCallBack(). */
+static RI_CompareInfo *ri_compare_dead_list = NULL;
+
/*
* Local function prototypes
*/
@@ -343,11 +356,13 @@ static bool ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid,
Datum lhs, Datum rhs);
static void ri_InitHashTables(void);
+static void InvalidateCastCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
+ uint32 hashvalue);
static void InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
uint32 hashvalue);
static SPIPlanPtr ri_FetchPreparedPlan(RI_QueryKey *key);
static void ri_HashPreparedPlan(RI_QueryKey *key, SPIPlanPtr plan);
-static RI_CompareHashEntry *ri_HashCompareOp(Oid eq_opr, Oid typeid);
+static RI_CompareInfo *ri_HashCompareOp(Oid eq_opr, Oid typeid);
static void ri_CheckTrigger(FunctionCallInfo fcinfo, const char *funcname,
int tgkind);
@@ -2666,6 +2681,34 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
}
+/*
+ * Cast changes can affect any comparison or fast-path entry. Do not free or
+ * overwrite call information here: a cast can execute DDL and reenter RI checks
+ * while an outer call is still using it. AtEOXact_RI() releases detached data.
+ */
+static void
+InvalidateCastCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
+ uint32 hashvalue)
+{
+ HASH_SEQ_STATUS status;
+ RI_CompareHashEntry *entry;
+
+ hash_seq_init(&status, ri_compare_cache);
+ while ((entry = hash_seq_search(&status)) != NULL)
+ {
+ if (entry->info != NULL)
+ {
+ entry->info->next_dead = ri_compare_dead_list;
+ ri_compare_dead_list = entry->info;
+ entry->info = NULL;
+ }
+ }
+
+ /* Fast-path metadata contains copies of the cached call information. */
+ InvalidateConstraintCacheCallBack(arg, cacheid, 0);
+}
+
+
/*
* Prepare execution plan for a query to enforce an RI restriction
*/
@@ -3674,24 +3717,23 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo,
Relation fk_rel, Relation idx_rel)
{
FastPathMeta *fpmeta;
- MemoryContext oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+ MemoryContext context;
Assert(riinfo != NULL && riinfo->valid);
Assert(riinfo->fpmeta == NULL);
- 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);
+ /* 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++)
{
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);
+ RI_CompareInfo *entry = ri_HashCompareOp(eq_opr, typeid);
int idx_col;
/*
@@ -3724,8 +3766,8 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo,
&fpmeta->subtypes[i]);
}
+ MemoryContextSetParent(context, TopMemoryContext);
riinfo->fpmeta = fpmeta;
- MemoryContextSwitchTo(oldcxt);
}
/*
@@ -3996,6 +4038,10 @@ ri_InitHashTables(void)
ri_compare_cache = hash_create("RI compare cache",
RI_INIT_QUERYHASHSIZE,
&ctl, HASH_ELEM | HASH_BLOBS);
+
+ CacheRegisterSyscacheCallback(CASTSOURCETARGET,
+ InvalidateCastCacheCallBack,
+ (Datum) 0);
}
@@ -4184,7 +4230,7 @@ static bool
ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid,
Datum lhs, Datum rhs)
{
- RI_CompareHashEntry *entry = ri_HashCompareOp(eq_opr, typeid);
+ RI_CompareInfo *entry = ri_HashCompareOp(eq_opr, typeid);
/* Do we need to cast the values? */
if (OidIsValid(entry->cast_func_finfo.fn_oid))
@@ -4229,7 +4275,7 @@ ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid,
* its right-hand input, a cast function to coerce the value before
* comparison.
*/
-static RI_CompareHashEntry *
+static RI_CompareInfo *
ri_HashCompareOp(Oid eq_opr, Oid typeid)
{
RI_CompareKey key;
@@ -4252,23 +4298,20 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
&key,
HASH_ENTER, &found);
if (!found)
- entry->valid = false;
+ entry->info = NULL;
/*
- * If not already initialized, do so. Since we'll keep this hash entry
- * for the life of the backend, put any subsidiary info for the function
- * cache structs into TopMemoryContext.
+ * If not already initialized, build a new generation of call information.
+ * Use a separate context so invalidation cannot affect active callers.
*/
- if (!entry->valid)
+ if (entry->info == NULL)
{
Oid lefttype,
righttype,
castfunc;
CoercionPathType pathtype;
-
- /* We always need to know how to call the equality operator */
- fmgr_info_cxt(get_opcode(eq_opr), &entry->eq_opr_finfo,
- TopMemoryContext);
+ MemoryContext context;
+ RI_CompareInfo *info;
/*
* If we chose to use a cast from FK to PK type, we may have to apply
@@ -4313,15 +4356,23 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
format_type_be(lefttype));
}
}
+
+ /* Leave incomplete entries subject to normal error cleanup. */
+ context = AllocSetContextCreate(CurTransactionContext,
+ "RI compare info",
+ ALLOCSET_SMALL_SIZES);
+ info = MemoryContextAllocZero(context, sizeof(RI_CompareInfo));
+ info->context = context;
+ fmgr_info_cxt(get_opcode(eq_opr), &info->eq_opr_finfo, context);
if (OidIsValid(castfunc))
- fmgr_info_cxt(castfunc, &entry->cast_func_finfo,
- TopMemoryContext);
+ fmgr_info_cxt(castfunc, &info->cast_func_finfo, context);
else
- entry->cast_func_finfo.fn_oid = InvalidOid;
- entry->valid = true;
+ info->cast_func_finfo.fn_oid = InvalidOid;
+ MemoryContextSetParent(context, TopMemoryContext);
+ entry->info = info;
}
- return entry;
+ return entry->info;
}
@@ -4519,18 +4570,25 @@ AtEOXact_RI(bool isCommit)
ri_fastpath_flushing = false;
/*
- * Release fast-path metadata detached during this transaction by
- * InvalidateConstraintCacheCallBack(). We are past every RI check that
- * could still hold a pointer into one of these, so freeing here is safe
- * on both the commit and the abort path.
+ * Release comparison and fast-path call information detached by
+ * invalidation callbacks. We are past every RI check that could still
+ * hold a pointer into one of these, so freeing here is safe on both the
+ * commit and the abort path.
*/
+ while (ri_compare_dead_list != NULL)
+ {
+ RI_CompareInfo *dead = ri_compare_dead_list;
+
+ ri_compare_dead_list = dead->next_dead;
+ MemoryContextDelete(dead->context);
+ }
+
while (ri_fpmeta_dead_list != NULL)
{
FastPathMeta *dead = ri_fpmeta_dead_list;
ri_fpmeta_dead_list = dead->next_dead;
MemoryContextDelete(dead->scratch_cxt);
- pfree(dead);
}
}
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index c54a9895b55..d551f8531ac 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1081,6 +1081,123 @@ CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY
ptest3) REFERENCES pktable);
ERROR: foreign key constraint "pktable_ptest4_ptest3_fkey" cannot be implemented
DETAIL: Key columns "ptest4" of the referencing table and "ptest1" of the referenced table are of incompatible types: inet and integer.
+-- Replacing a cast must invalidate cached RI comparison call information.
+CREATE TYPE fk_cast_type AS (v int);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES ('original', ROW(1)::fk_cast_type);
+-- With autocommit, this compares a committed row and commits the updated row.
+-- Updating a row inserted in the same transaction would skip the comparison.
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+BEGIN;
+SAVEPOINT original_cast;
+DROP CAST (fk_cast_type AS int);
+CREATE FUNCTION fk_cast2(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast2(fk_cast_type) AS IMPLICIT;
+DROP FUNCTION fk_cast1(fk_cast_type);
+-- Exercise the comparison cache before a new INSERT can rebuild other caches.
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+INSERT INTO fk_cast_fk VALUES ('replacement', ROW(1)::fk_cast_type);
+SAVEPOINT invalid_key;
+INSERT INTO fk_cast_fk VALUES ('invalid', ROW(2)::fk_cast_type); -- must fail
+ERROR: insert or update on table "fk_cast_fk" violates foreign key constraint "fk_cast_fk_id_fkey"
+DETAIL: Key (id)=((2)) is not present in table "fk_cast_pk".
+ROLLBACK TO invalid_key;
+-- Exercise restoration immediately, before any further DDL invalidates caches.
+ROLLBACK TO original_cast;
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+INSERT INTO fk_cast_fk VALUES ('restored', ROW(1)::fk_cast_type);
+SELECT count(*) FROM fk_cast_fk;
+ count
+-------
+ 2
+(1 row)
+
+ROLLBACK;
+-- Failed fast-path rebuilds must not accumulate metadata on subxact abort.
+BEGIN;
+SAVEPOINT missing_cast;
+DROP CAST (fk_cast_type AS int);
+DO $$
+DECLARE
+ before_count bigint;
+ after_count bigint;
+BEGIN
+ SELECT count(*) INTO before_count FROM pg_backend_memory_contexts
+ WHERE name LIKE 'RI %';
+ FOR i IN 1..10 LOOP
+ BEGIN
+ INSERT INTO fk_cast_fk VALUES ('failed', ROW(1)::fk_cast_type);
+ RAISE EXCEPTION 'missing cast was not detected';
+ EXCEPTION WHEN internal_error THEN
+ IF SQLERRM NOT LIKE 'no conversion function%' THEN
+ RAISE;
+ END IF;
+ END;
+ END LOOP;
+ SELECT count(*) INTO after_count FROM pg_backend_memory_contexts
+ WHERE name LIKE 'RI %';
+ IF after_count > before_count THEN
+ RAISE EXCEPTION 'RI contexts leaked across failed checks';
+ END IF;
+END $$;
+ROLLBACK TO missing_cast;
+INSERT INTO fk_cast_fk VALUES ('after_error', ROW(1)::fk_cast_type);
+ROLLBACK;
+DROP TABLE fk_cast_fk, fk_cast_pk;
+DROP CAST (fk_cast_type AS int);
+DROP FUNCTION fk_cast1(fk_cast_type);
+DROP TYPE fk_cast_type;
+-- Invalidation during a comparison must not overwrite its call information.
+CREATE TYPE fk_cast_type AS (v int);
+CREATE TABLE fk_cast_guard (armed bool);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int LANGUAGE plpgsql AS $$
+BEGIN
+ IF EXISTS (SELECT FROM fk_cast_guard) THEN
+ DELETE FROM fk_cast_guard;
+ EXECUTE 'DROP CAST (fk_cast_type AS bigint)';
+ -- Compare a different committed row, rebuilding the same cache entry.
+ UPDATE fk_cast_fk SET id = id WHERE label = 'nested';
+ END IF;
+ RETURN $1.v;
+END $$;
+CREATE FUNCTION fk_cast_aux(fk_cast_type) RETURNS bigint
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v::bigint';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE CAST (fk_cast_type AS bigint) WITH FUNCTION fk_cast_aux(fk_cast_type);
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES ('outer', ROW(1)::fk_cast_type),
+ ('nested', ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = id WHERE label = 'outer';
+BEGIN;
+INSERT INTO fk_cast_guard VALUES (true);
+-- Both the outer and nested UPDATE compare rows from earlier transactions.
+UPDATE fk_cast_fk SET id = id WHERE label = 'outer';
+SELECT count(*) FROM fk_cast_guard;
+ count
+-------
+ 0
+(1 row)
+
+SELECT count(*) FROM fk_cast_fk;
+ count
+-------
+ 2
+(1 row)
+
+ROLLBACK;
+DROP TABLE fk_cast_fk, fk_cast_pk, fk_cast_guard;
+DROP CAST (fk_cast_type AS int);
+DROP CAST (fk_cast_type AS bigint);
+DROP FUNCTION fk_cast1(fk_cast_type), fk_cast_aux(fk_cast_type);
+DROP TYPE fk_cast_type;
--
-- Now some cases with inheritance
-- Basic 2 table case: 1 column of matching types.
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index c013d1f8834..8986b93a5ed 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -747,6 +747,116 @@ ptest3) REFERENCES pktable(ptest1, ptest2));
CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY KEY(ptest1, ptest2), FOREIGN KEY(ptest4,
ptest3) REFERENCES pktable);
+-- Replacing a cast must invalidate cached RI comparison call information.
+CREATE TYPE fk_cast_type AS (v int);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES ('original', ROW(1)::fk_cast_type);
+
+-- With autocommit, this compares a committed row and commits the updated row.
+-- Updating a row inserted in the same transaction would skip the comparison.
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+
+BEGIN;
+SAVEPOINT original_cast;
+DROP CAST (fk_cast_type AS int);
+CREATE FUNCTION fk_cast2(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast2(fk_cast_type) AS IMPLICIT;
+DROP FUNCTION fk_cast1(fk_cast_type);
+
+-- Exercise the comparison cache before a new INSERT can rebuild other caches.
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+INSERT INTO fk_cast_fk VALUES ('replacement', ROW(1)::fk_cast_type);
+SAVEPOINT invalid_key;
+INSERT INTO fk_cast_fk VALUES ('invalid', ROW(2)::fk_cast_type); -- must fail
+ROLLBACK TO invalid_key;
+
+-- Exercise restoration immediately, before any further DDL invalidates caches.
+ROLLBACK TO original_cast;
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+INSERT INTO fk_cast_fk VALUES ('restored', ROW(1)::fk_cast_type);
+SELECT count(*) FROM fk_cast_fk;
+ROLLBACK;
+
+-- Failed fast-path rebuilds must not accumulate metadata on subxact abort.
+BEGIN;
+SAVEPOINT missing_cast;
+DROP CAST (fk_cast_type AS int);
+DO $$
+DECLARE
+ before_count bigint;
+ after_count bigint;
+BEGIN
+ SELECT count(*) INTO before_count FROM pg_backend_memory_contexts
+ WHERE name LIKE 'RI %';
+ FOR i IN 1..10 LOOP
+ BEGIN
+ INSERT INTO fk_cast_fk VALUES ('failed', ROW(1)::fk_cast_type);
+ RAISE EXCEPTION 'missing cast was not detected';
+ EXCEPTION WHEN internal_error THEN
+ IF SQLERRM NOT LIKE 'no conversion function%' THEN
+ RAISE;
+ END IF;
+ END;
+ END LOOP;
+ SELECT count(*) INTO after_count FROM pg_backend_memory_contexts
+ WHERE name LIKE 'RI %';
+ IF after_count > before_count THEN
+ RAISE EXCEPTION 'RI contexts leaked across failed checks';
+ END IF;
+END $$;
+ROLLBACK TO missing_cast;
+INSERT INTO fk_cast_fk VALUES ('after_error', ROW(1)::fk_cast_type);
+ROLLBACK;
+
+DROP TABLE fk_cast_fk, fk_cast_pk;
+DROP CAST (fk_cast_type AS int);
+DROP FUNCTION fk_cast1(fk_cast_type);
+DROP TYPE fk_cast_type;
+
+-- Invalidation during a comparison must not overwrite its call information.
+CREATE TYPE fk_cast_type AS (v int);
+CREATE TABLE fk_cast_guard (armed bool);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int LANGUAGE plpgsql AS $$
+BEGIN
+ IF EXISTS (SELECT FROM fk_cast_guard) THEN
+ DELETE FROM fk_cast_guard;
+ EXECUTE 'DROP CAST (fk_cast_type AS bigint)';
+ -- Compare a different committed row, rebuilding the same cache entry.
+ UPDATE fk_cast_fk SET id = id WHERE label = 'nested';
+ END IF;
+ RETURN $1.v;
+END $$;
+CREATE FUNCTION fk_cast_aux(fk_cast_type) RETURNS bigint
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v::bigint';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE CAST (fk_cast_type AS bigint) WITH FUNCTION fk_cast_aux(fk_cast_type);
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES ('outer', ROW(1)::fk_cast_type),
+ ('nested', ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = id WHERE label = 'outer';
+
+BEGIN;
+INSERT INTO fk_cast_guard VALUES (true);
+-- Both the outer and nested UPDATE compare rows from earlier transactions.
+UPDATE fk_cast_fk SET id = id WHERE label = 'outer';
+SELECT count(*) FROM fk_cast_guard;
+SELECT count(*) FROM fk_cast_fk;
+ROLLBACK;
+
+DROP TABLE fk_cast_fk, fk_cast_pk, fk_cast_guard;
+DROP CAST (fk_cast_type AS int);
+DROP CAST (fk_cast_type AS bigint);
+DROP FUNCTION fk_cast1(fk_cast_type), fk_cast_aux(fk_cast_type);
+DROP TYPE fk_cast_type;
+
--
-- Now some cases with inheritance
-- Basic 2 table case: 1 column of matching types.
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 7aedaafab90..5bc98422258 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2501,6 +2501,7 @@ RBTreeIterator
REPARSE_JUNCTION_DATA_BUFFER
RIX
RI_CompareHashEntry
+RI_CompareInfo
RI_CompareKey
RI_ConstraintInfo
RI_FastPathEntry
--
2.47.3
[application/octet-stream] REL_19_STABLE-v2-0001-Fix-RI-fast-path-permission-checks.patch (11.6K, ../../CA+HiwqGAq2fqXDSOUzE-uv4-LNMDcu6zHCF8co6=aRBZaTnoQg@mail.gmail.com/4-REL_19_STABLE-v2-0001-Fix-RI-fast-path-permission-checks.patch)
download | inline diff:
From d4c75315e59593d2308e5fe924f2648dda468709 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlangote09@gmail.com>
Date: Fri, 11 Sep 2026 12:14:20 +0900
Subject: [PATCH v2 1/2] Fix RI fast-path permission checks
The fast path required table-level SELECT on the referenced table,
rejecting checks that the SPI path allows with column-level grants.
It also omitted the UPDATE privilege required by FOR KEY SHARE.
When table privileges do not suffice, use ExecCheckOneRelPerms() with
the referenced key columns as selectedCols and an empty updatedCols.
This accepts SELECT on all referenced columns and UPDATE on any column,
matching the SPI query. Keep the table-privilege check as a shortcut
that avoids constructing a column bitmap in the usual case.
Extend the ACL regression tests to cover column grants, privilege
revocation, and per-row validation of a composite foreign key.
Co-authored-by: Nikolay Samokhvalov <nik@postgres.ai>
Backpatch-through: 19
---
src/backend/utils/adt/ri_triggers.c | 45 +++++++++++++----
src/test/regress/expected/foreign_key.out | 55 ++++++++++++++++++++-
src/test/regress/sql/foreign_key.sql | 59 ++++++++++++++++++++++-
3 files changed, 145 insertions(+), 14 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 6958f991604..4342daef4ef 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -290,7 +290,8 @@ static bool ri_FastPathProbeOne(Relation pk_rel, Relation idx_rel,
static bool ri_LockPKTuple(Relation pk_rel, TupleTableSlot *slot, Snapshot snap,
bool *concurrently_updated);
static bool ri_fastpath_is_applicable(const RI_ConstraintInfo *riinfo);
-static void ri_CheckPermissions(Relation query_rel);
+static void ri_CheckPermissions(const RI_ConstraintInfo *riinfo,
+ Relation query_rel);
static bool recheck_matched_pk_tuple(Relation idxrel, ScanKeyData *skeys,
int nkeys, TupleTableSlot *new_slot);
static void build_index_scankeys(const RI_ConstraintInfo *riinfo,
@@ -2802,7 +2803,7 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo,
saved_sec_context |
SECURITY_LOCAL_USERID_CHANGE |
SECURITY_NOFORCE_RLS);
- ri_CheckPermissions(pk_rel);
+ ri_CheckPermissions(riinfo, pk_rel);
/*
* Begin the scan under the switched user id, so that any access method
@@ -2988,13 +2989,16 @@ ri_fastpath_is_applicable(const RI_ConstraintInfo *riinfo)
/*
* ri_CheckPermissions
- * Check that the current user has permissions to look into the schema of
- * and SELECT from 'query_rel'
+ * Check permissions for the SELECT ... FOR KEY SHARE used by the SPI
+ * path, as the referenced table's owner.
*/
static void
-ri_CheckPermissions(Relation query_rel)
+ri_CheckPermissions(const RI_ConstraintInfo *riinfo, Relation query_rel)
{
AclResult aclresult;
+ AclMode requiredPerms = ACL_SELECT | ACL_SELECT_FOR_UPDATE;
+ RTEPermissionInfo *perminfo;
+ bool result;
/* USAGE on schema. */
aclresult = object_aclcheck(NamespaceRelationId,
@@ -3004,11 +3008,32 @@ ri_CheckPermissions(Relation query_rel)
aclcheck_error(aclresult, OBJECT_SCHEMA,
get_namespace_name(RelationGetNamespace(query_rel)));
- /* SELECT on relation. */
- aclresult = pg_class_aclcheck(RelationGetRelid(query_rel), GetUserId(),
- ACL_SELECT);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, OBJECT_TABLE,
+ /* Avoid building the column bitmap when table privileges suffice. */
+ if (pg_class_aclmask(RelationGetRelid(query_rel), GetUserId(),
+ requiredPerms, ACLMASK_ALL) == requiredPerms)
+ return;
+
+ /*
+ * SELECT is needed only on the referenced key columns. FOR KEY SHARE
+ * also needs UPDATE privilege, which may be granted on any column. Use
+ * the executor's checks for both, leaving updatedCols empty as the SPI
+ * query does.
+ */
+ perminfo = makeNode(RTEPermissionInfo);
+ perminfo->relid = RelationGetRelid(query_rel);
+ perminfo->requiredPerms = requiredPerms;
+ for (int i = 0; i < riinfo->nkeys; i++)
+ {
+ int attno = riinfo->pk_attnums[i] - FirstLowInvalidHeapAttributeNumber;
+
+ perminfo->selectedCols = bms_add_member(perminfo->selectedCols, attno);
+ }
+
+ result = ExecCheckOneRelPerms(perminfo);
+ bms_free(perminfo->selectedCols);
+ pfree(perminfo);
+ if (!result)
+ aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLE,
RelationGetRelationName(query_rel));
}
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 9cea669b6fd..d0c93ac8744 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -402,17 +402,68 @@ CREATE TABLE FKTABLE ( ftest1 int REFERENCES PKTABLE, ftest2 int );
INSERT INTO PKTABLE VALUES (1, 'Test1');
INSERT INTO PKTABLE VALUES (2, 'Test2');
INSERT INTO PKTABLE VALUES (3, 'Test3');
--- Grant usage on PKTABLE to user regress_foreign_key_user
+-- Grant SELECT on PKTABLE to user regress_foreign_key_user
CREATE USER regress_foreign_key_user NOLOGIN;
GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
ALTER TABLE PKTABLE OWNER to regress_foreign_key_user;
-- Inserting into FKTABLE should work
INSERT INTO FKTABLE VALUES (3, 5);
--- Revoke usage on PKTABLE from user regress_foreign_key_user
+-- Revoke SELECT on PKTABLE from user regress_foreign_key_user
REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
-- Inserting into FKTABLE should fail
INSERT INTO FKTABLE VALUES (2, 6);
ERROR: permission denied for table pktable
+-- SELECT on the referenced key column is enough, without SELECT on ptest2.
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+-- SELECT on an unrelated column does not suffice.
+REVOKE SELECT (ptest1) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+ERROR: permission denied for table pktable
+REVOKE SELECT (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+-- FOR KEY SHARE also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+ERROR: permission denied for table pktable
+-- UPDATE on any column suffices, even one that the check does not read.
+GRANT UPDATE (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+-- Table-level SELECT can be combined with column-level UPDATE.
+GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+REVOKE UPDATE (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+ERROR: permission denied for table pktable
+DROP TABLE FKTABLE;
+DROP TABLE PKTABLE;
+-- Check all referenced columns, including when index and FK order differ.
+CREATE TABLE PKTABLE ( ptest0 text, ptest1 int, ptest2 int,
+ PRIMARY KEY (ptest2, ptest1) );
+CREATE TABLE FKTABLE ( ftest1 int, ftest2 int );
+INSERT INTO PKTABLE VALUES ('unused', 1, 2);
+INSERT INTO FKTABLE VALUES (1, 2);
+ALTER TABLE FKTABLE ADD CONSTRAINT fktable_fk
+ FOREIGN KEY (ftest1, ftest2) REFERENCES PKTABLE (ptest1, ptest2) NOT VALID;
+ALTER TABLE PKTABLE OWNER TO regress_foreign_key_user;
+ALTER TABLE FKTABLE OWNER TO regress_foreign_key_user;
+REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+-- Lack of SELECT on FKTABLE forces validation to check each row.
+REVOKE SELECT ON FKTABLE FROM regress_foreign_key_user;
+SET ROLE regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+ERROR: permission denied for table pktable
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+-- Per-row validation also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+ERROR: permission denied for table pktable
+-- UPDATE on the unrelated column is enough.
+GRANT UPDATE (ptest0) ON PKTABLE TO regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk;
+RESET ROLE;
DROP TABLE FKTABLE;
DROP TABLE PKTABLE;
DROP USER regress_foreign_key_user;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 07d89219318..5e27b9b2d24 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -286,7 +286,7 @@ INSERT INTO PKTABLE VALUES (1, 'Test1');
INSERT INTO PKTABLE VALUES (2, 'Test2');
INSERT INTO PKTABLE VALUES (3, 'Test3');
--- Grant usage on PKTABLE to user regress_foreign_key_user
+-- Grant SELECT on PKTABLE to user regress_foreign_key_user
CREATE USER regress_foreign_key_user NOLOGIN;
GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
@@ -295,12 +295,67 @@ ALTER TABLE PKTABLE OWNER to regress_foreign_key_user;
-- Inserting into FKTABLE should work
INSERT INTO FKTABLE VALUES (3, 5);
--- Revoke usage on PKTABLE from user regress_foreign_key_user
+-- Revoke SELECT on PKTABLE from user regress_foreign_key_user
REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
-- Inserting into FKTABLE should fail
INSERT INTO FKTABLE VALUES (2, 6);
+-- SELECT on the referenced key column is enough, without SELECT on ptest2.
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+
+-- SELECT on an unrelated column does not suffice.
+REVOKE SELECT (ptest1) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+REVOKE SELECT (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+
+-- FOR KEY SHARE also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+
+-- UPDATE on any column suffices, even one that the check does not read.
+GRANT UPDATE (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+
+-- Table-level SELECT can be combined with column-level UPDATE.
+GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+REVOKE UPDATE (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+
+DROP TABLE FKTABLE;
+DROP TABLE PKTABLE;
+
+-- Check all referenced columns, including when index and FK order differ.
+CREATE TABLE PKTABLE ( ptest0 text, ptest1 int, ptest2 int,
+ PRIMARY KEY (ptest2, ptest1) );
+CREATE TABLE FKTABLE ( ftest1 int, ftest2 int );
+INSERT INTO PKTABLE VALUES ('unused', 1, 2);
+INSERT INTO FKTABLE VALUES (1, 2);
+ALTER TABLE FKTABLE ADD CONSTRAINT fktable_fk
+ FOREIGN KEY (ftest1, ftest2) REFERENCES PKTABLE (ptest1, ptest2) NOT VALID;
+ALTER TABLE PKTABLE OWNER TO regress_foreign_key_user;
+ALTER TABLE FKTABLE OWNER TO regress_foreign_key_user;
+REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+
+-- Lack of SELECT on FKTABLE forces validation to check each row.
+REVOKE SELECT ON FKTABLE FROM regress_foreign_key_user;
+SET ROLE regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+
+-- Per-row validation also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+-- UPDATE on the unrelated column is enough.
+GRANT UPDATE (ptest0) ON PKTABLE TO regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk;
+RESET ROLE;
+
DROP TABLE FKTABLE;
DROP TABLE PKTABLE;
--
2.47.3
[application/octet-stream] REL_19_STABLE-v2-0002-Invalidate-RI-call-information-when-casts-change.patch (22.0K, ../../CA+HiwqGAq2fqXDSOUzE-uv4-LNMDcu6zHCF8co6=aRBZaTnoQg@mail.gmail.com/5-REL_19_STABLE-v2-0002-Invalidate-RI-call-information-when-casts-change.patch)
download | inline diff:
From d79a2dc111c3026bb4b5c56be487a81e26cc3e97 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Fri, 11 Sep 2026 17:06:06 +0900
Subject: [PATCH v2 2/2] Invalidate RI call information when casts change
Changing a cast need not modify pg_constraint, so the RI comparison
cache can keep using the old cast function after its replacement.
Dropping that function can then make an UPDATE fail with a cache lookup
error.
The fast-path foreign key checks added in PostgreSQL 19 also use this
cache and copy its call information into their own metadata, exposing
the problem on INSERT as well.
Invalidate both caches on CASTSOURCETARGET changes. Keep comparison call
information separate from its hash entry and defer releasing invalidated
objects until transaction end, as already done for fast-path metadata.
A cast can run DDL and reenter RI checks, so invalidation must not free
or overwrite call information still used by an outer comparison.
Build new comparison and fast-path call information in transaction-owned
contexts and reparent them only when construction succeeds, so failed
rebuilds do not leak backend memory.
Test replacement after cache warmup using a committed row, invalid-key
rejection, rollback restoring the old cast, and invalidation with a
nested comparison of another committed row. Also check cleanup after
failed fast-path rebuilds.
Author: Nikolay Samokhvalov <nik@postgres.ai>
Co-authored-by: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CAM527d9BgPjeOOYmbCBTd57R145qHCk-dzw9qNq+nOrDq1j__A@mail.gmail.com
Backpatch-through: 14
---
src/backend/utils/adt/ri_triggers.c | 140 +++++++++++++++-------
src/test/regress/expected/foreign_key.out | 117 ++++++++++++++++++
src/test/regress/sql/foreign_key.sql | 110 +++++++++++++++++
src/tools/pgindent/typedefs.list | 1 +
4 files changed, 326 insertions(+), 42 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 4342daef4ef..b8d00af98d8 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -164,7 +164,7 @@ typedef struct FastPathMeta
* 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
+ * of their own, which also owns this struct and is destroyed by
* AtEOXact_RI().
*
* Note this context must not be reset while the FmgrInfos remain in use,
@@ -207,15 +207,25 @@ typedef struct RI_CompareKey
Oid typeid; /* the data type to apply it to */
} RI_CompareKey;
+/*
+ * Cached call information is detached on invalidation, but kept until the end
+ * of the transaction in case an active comparison still references it.
+ */
+typedef struct RI_CompareInfo
+{
+ FmgrInfo eq_opr_finfo; /* call info for equality fn */
+ FmgrInfo cast_func_finfo; /* in case we must coerce input */
+ MemoryContext context;
+ struct RI_CompareInfo *next_dead;
+} RI_CompareInfo;
+
/*
* RI_CompareHashEntry
*/
typedef struct RI_CompareHashEntry
{
RI_CompareKey key;
- bool valid; /* successfully initialized? */
- FmgrInfo eq_opr_finfo; /* call info for equality fn */
- FmgrInfo cast_func_finfo; /* in case we must coerce input */
+ RI_CompareInfo *info; /* NULL if invalid */
} RI_CompareHashEntry;
/*
@@ -234,6 +244,9 @@ static dclist_head ri_constraint_cache_valid_list;
*/
static FastPathMeta *ri_fpmeta_dead_list = NULL;
+/* Comparison call information detached by InvalidateCastCacheCallBack(). */
+static RI_CompareInfo *ri_compare_dead_list = NULL;
+
/*
* Local function prototypes
*/
@@ -261,11 +274,13 @@ static bool ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid,
Datum lhs, Datum rhs);
static void ri_InitHashTables(void);
+static void InvalidateCastCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
+ uint32 hashvalue);
static void InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
uint32 hashvalue);
static SPIPlanPtr ri_FetchPreparedPlan(RI_QueryKey *key);
static void ri_HashPreparedPlan(RI_QueryKey *key, SPIPlanPtr plan);
-static RI_CompareHashEntry *ri_HashCompareOp(Oid eq_opr, Oid typeid);
+static RI_CompareInfo *ri_HashCompareOp(Oid eq_opr, Oid typeid);
static void ri_CheckTrigger(FunctionCallInfo fcinfo, const char *funcname,
int tgkind);
@@ -2548,6 +2563,34 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
}
+/*
+ * Cast changes can affect any comparison or fast-path entry. Do not free or
+ * overwrite call information here: a cast can execute DDL and reenter RI checks
+ * while an outer call is still using it. AtEOXact_RI() releases detached data.
+ */
+static void
+InvalidateCastCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
+ uint32 hashvalue)
+{
+ HASH_SEQ_STATUS status;
+ RI_CompareHashEntry *entry;
+
+ hash_seq_init(&status, ri_compare_cache);
+ while ((entry = hash_seq_search(&status)) != NULL)
+ {
+ if (entry->info != NULL)
+ {
+ entry->info->next_dead = ri_compare_dead_list;
+ ri_compare_dead_list = entry->info;
+ entry->info = NULL;
+ }
+ }
+
+ /* Fast-path metadata contains copies of the cached call information. */
+ InvalidateConstraintCacheCallBack(arg, cacheid, 0);
+}
+
+
/*
* Prepare execution plan for a query to enforce an RI restriction
*/
@@ -3157,24 +3200,23 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo,
Relation fk_rel, Relation idx_rel)
{
FastPathMeta *fpmeta;
- MemoryContext oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+ MemoryContext context;
Assert(riinfo != NULL && riinfo->valid);
Assert(riinfo->fpmeta == NULL);
- 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);
+ /* 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++)
{
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);
+ RI_CompareInfo *entry = ri_HashCompareOp(eq_opr, typeid);
int idx_col;
/*
@@ -3207,8 +3249,8 @@ ri_populate_fastpath_metadata(RI_ConstraintInfo *riinfo,
&fpmeta->subtypes[i]);
}
+ MemoryContextSetParent(context, TopMemoryContext);
riinfo->fpmeta = fpmeta;
- MemoryContextSwitchTo(oldcxt);
}
/*
@@ -3479,6 +3521,10 @@ ri_InitHashTables(void)
ri_compare_cache = hash_create("RI compare cache",
RI_INIT_QUERYHASHSIZE,
&ctl, HASH_ELEM | HASH_BLOBS);
+
+ CacheRegisterSyscacheCallback(CASTSOURCETARGET,
+ InvalidateCastCacheCallBack,
+ (Datum) 0);
}
@@ -3667,7 +3713,7 @@ static bool
ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid,
Datum lhs, Datum rhs)
{
- RI_CompareHashEntry *entry = ri_HashCompareOp(eq_opr, typeid);
+ RI_CompareInfo *entry = ri_HashCompareOp(eq_opr, typeid);
/* Do we need to cast the values? */
if (OidIsValid(entry->cast_func_finfo.fn_oid))
@@ -3712,7 +3758,7 @@ ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid,
* its right-hand input, a cast function to coerce the value before
* comparison.
*/
-static RI_CompareHashEntry *
+static RI_CompareInfo *
ri_HashCompareOp(Oid eq_opr, Oid typeid)
{
RI_CompareKey key;
@@ -3735,23 +3781,20 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
&key,
HASH_ENTER, &found);
if (!found)
- entry->valid = false;
+ entry->info = NULL;
/*
- * If not already initialized, do so. Since we'll keep this hash entry
- * for the life of the backend, put any subsidiary info for the function
- * cache structs into TopMemoryContext.
+ * If not already initialized, build a new generation of call information.
+ * Use a separate context so invalidation cannot affect active callers.
*/
- if (!entry->valid)
+ if (entry->info == NULL)
{
Oid lefttype,
righttype,
castfunc;
CoercionPathType pathtype;
-
- /* We always need to know how to call the equality operator */
- fmgr_info_cxt(get_opcode(eq_opr), &entry->eq_opr_finfo,
- TopMemoryContext);
+ MemoryContext context;
+ RI_CompareInfo *info;
/*
* If we chose to use a cast from FK to PK type, we may have to apply
@@ -3796,15 +3839,23 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
format_type_be(lefttype));
}
}
+
+ /* Leave incomplete entries subject to normal error cleanup. */
+ context = AllocSetContextCreate(CurTransactionContext,
+ "RI compare info",
+ ALLOCSET_SMALL_SIZES);
+ info = MemoryContextAllocZero(context, sizeof(RI_CompareInfo));
+ info->context = context;
+ fmgr_info_cxt(get_opcode(eq_opr), &info->eq_opr_finfo, context);
if (OidIsValid(castfunc))
- fmgr_info_cxt(castfunc, &entry->cast_func_finfo,
- TopMemoryContext);
+ fmgr_info_cxt(castfunc, &info->cast_func_finfo, context);
else
- entry->cast_func_finfo.fn_oid = InvalidOid;
- entry->valid = true;
+ info->cast_func_finfo.fn_oid = InvalidOid;
+ MemoryContextSetParent(context, TopMemoryContext);
+ entry->info = info;
}
- return entry;
+ return entry->info;
}
@@ -3841,30 +3892,35 @@ RI_FKey_trigger_type(Oid tgfoid)
* AtEOXact_RI
* End-of-transaction cleanup for referential integrity.
*
- * Currently this only releases fast-path metadata detached during the
- * transaction. InvalidateConstraintCacheCallBack() cannot free a
- * FastPathMeta when it detaches one, because an RI check further up the
- * stack may still hold a pointer into it. It queues them on
- * ri_fpmeta_dead_list instead, and we release them here, where no such
- * reference can exist. isCommit is accepted for consistency with the
- * other AtEOXact_* routines but is not used: the release is the same on
- * the commit and the abort path.
+ * Release comparison and fast-path call information detached during the
+ * transaction. Invalidation callbacks cannot free it immediately, because
+ * an RI check further up the stack may still hold a pointer into it.
+ * We release the queued objects here, where no such reference can exist.
+ * isCommit is accepted for consistency with the other AtEOXact_* routines
+ * but is not used: the release is the same on the commit and the abort path.
*
* There is no AtEOSubXact_RI() counterpart. Nothing here is scoped to a
- * subtransaction: a detached FastPathMeta stays reachable from the dead
- * list whichever subtransaction detached it, and a check holding a pointer
+ * subtransaction: detached call information stays reachable from the dead
+ * lists whichever subtransaction detached it, and a check holding a pointer
* into one may be running at an outer level, so releasing at subtransaction
* end would be unsafe as well as unnecessary.
*/
void
AtEOXact_RI(bool isCommit)
{
+ while (ri_compare_dead_list != NULL)
+ {
+ RI_CompareInfo *dead = ri_compare_dead_list;
+
+ ri_compare_dead_list = dead->next_dead;
+ MemoryContextDelete(dead->context);
+ }
+
while (ri_fpmeta_dead_list != NULL)
{
FastPathMeta *dead = ri_fpmeta_dead_list;
ri_fpmeta_dead_list = dead->next_dead;
MemoryContextDelete(dead->scratch_cxt);
- pfree(dead);
}
}
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index d0c93ac8744..5afcec5e41f 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1081,6 +1081,123 @@ CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY
ptest3) REFERENCES pktable);
ERROR: foreign key constraint "pktable_ptest4_ptest3_fkey" cannot be implemented
DETAIL: Key columns "ptest4" of the referencing table and "ptest1" of the referenced table are of incompatible types: inet and integer.
+-- Replacing a cast must invalidate cached RI comparison call information.
+CREATE TYPE fk_cast_type AS (v int);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES ('original', ROW(1)::fk_cast_type);
+-- With autocommit, this compares a committed row and commits the updated row.
+-- Updating a row inserted in the same transaction would skip the comparison.
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+BEGIN;
+SAVEPOINT original_cast;
+DROP CAST (fk_cast_type AS int);
+CREATE FUNCTION fk_cast2(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast2(fk_cast_type) AS IMPLICIT;
+DROP FUNCTION fk_cast1(fk_cast_type);
+-- Exercise the comparison cache before a new INSERT can rebuild other caches.
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+INSERT INTO fk_cast_fk VALUES ('replacement', ROW(1)::fk_cast_type);
+SAVEPOINT invalid_key;
+INSERT INTO fk_cast_fk VALUES ('invalid', ROW(2)::fk_cast_type); -- must fail
+ERROR: insert or update on table "fk_cast_fk" violates foreign key constraint "fk_cast_fk_id_fkey"
+DETAIL: Key (id)=((2)) is not present in table "fk_cast_pk".
+ROLLBACK TO invalid_key;
+-- Exercise restoration immediately, before any further DDL invalidates caches.
+ROLLBACK TO original_cast;
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+INSERT INTO fk_cast_fk VALUES ('restored', ROW(1)::fk_cast_type);
+SELECT count(*) FROM fk_cast_fk;
+ count
+-------
+ 2
+(1 row)
+
+ROLLBACK;
+-- Failed fast-path rebuilds must not accumulate metadata on subxact abort.
+BEGIN;
+SAVEPOINT missing_cast;
+DROP CAST (fk_cast_type AS int);
+DO $$
+DECLARE
+ before_count bigint;
+ after_count bigint;
+BEGIN
+ SELECT count(*) INTO before_count FROM pg_backend_memory_contexts
+ WHERE name LIKE 'RI %';
+ FOR i IN 1..10 LOOP
+ BEGIN
+ INSERT INTO fk_cast_fk VALUES ('failed', ROW(1)::fk_cast_type);
+ RAISE EXCEPTION 'missing cast was not detected';
+ EXCEPTION WHEN internal_error THEN
+ IF SQLERRM NOT LIKE 'no conversion function%' THEN
+ RAISE;
+ END IF;
+ END;
+ END LOOP;
+ SELECT count(*) INTO after_count FROM pg_backend_memory_contexts
+ WHERE name LIKE 'RI %';
+ IF after_count > before_count THEN
+ RAISE EXCEPTION 'RI contexts leaked across failed checks';
+ END IF;
+END $$;
+ROLLBACK TO missing_cast;
+INSERT INTO fk_cast_fk VALUES ('after_error', ROW(1)::fk_cast_type);
+ROLLBACK;
+DROP TABLE fk_cast_fk, fk_cast_pk;
+DROP CAST (fk_cast_type AS int);
+DROP FUNCTION fk_cast1(fk_cast_type);
+DROP TYPE fk_cast_type;
+-- Invalidation during a comparison must not overwrite its call information.
+CREATE TYPE fk_cast_type AS (v int);
+CREATE TABLE fk_cast_guard (armed bool);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int LANGUAGE plpgsql AS $$
+BEGIN
+ IF EXISTS (SELECT FROM fk_cast_guard) THEN
+ DELETE FROM fk_cast_guard;
+ EXECUTE 'DROP CAST (fk_cast_type AS bigint)';
+ -- Compare a different committed row, rebuilding the same cache entry.
+ UPDATE fk_cast_fk SET id = id WHERE label = 'nested';
+ END IF;
+ RETURN $1.v;
+END $$;
+CREATE FUNCTION fk_cast_aux(fk_cast_type) RETURNS bigint
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v::bigint';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE CAST (fk_cast_type AS bigint) WITH FUNCTION fk_cast_aux(fk_cast_type);
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES ('outer', ROW(1)::fk_cast_type),
+ ('nested', ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = id WHERE label = 'outer';
+BEGIN;
+INSERT INTO fk_cast_guard VALUES (true);
+-- Both the outer and nested UPDATE compare rows from earlier transactions.
+UPDATE fk_cast_fk SET id = id WHERE label = 'outer';
+SELECT count(*) FROM fk_cast_guard;
+ count
+-------
+ 0
+(1 row)
+
+SELECT count(*) FROM fk_cast_fk;
+ count
+-------
+ 2
+(1 row)
+
+ROLLBACK;
+DROP TABLE fk_cast_fk, fk_cast_pk, fk_cast_guard;
+DROP CAST (fk_cast_type AS int);
+DROP CAST (fk_cast_type AS bigint);
+DROP FUNCTION fk_cast1(fk_cast_type), fk_cast_aux(fk_cast_type);
+DROP TYPE fk_cast_type;
--
-- Now some cases with inheritance
-- Basic 2 table case: 1 column of matching types.
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 5e27b9b2d24..60cb739cfa3 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -747,6 +747,116 @@ ptest3) REFERENCES pktable(ptest1, ptest2));
CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY KEY(ptest1, ptest2), FOREIGN KEY(ptest4,
ptest3) REFERENCES pktable);
+-- Replacing a cast must invalidate cached RI comparison call information.
+CREATE TYPE fk_cast_type AS (v int);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES ('original', ROW(1)::fk_cast_type);
+
+-- With autocommit, this compares a committed row and commits the updated row.
+-- Updating a row inserted in the same transaction would skip the comparison.
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+
+BEGIN;
+SAVEPOINT original_cast;
+DROP CAST (fk_cast_type AS int);
+CREATE FUNCTION fk_cast2(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast2(fk_cast_type) AS IMPLICIT;
+DROP FUNCTION fk_cast1(fk_cast_type);
+
+-- Exercise the comparison cache before a new INSERT can rebuild other caches.
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+INSERT INTO fk_cast_fk VALUES ('replacement', ROW(1)::fk_cast_type);
+SAVEPOINT invalid_key;
+INSERT INTO fk_cast_fk VALUES ('invalid', ROW(2)::fk_cast_type); -- must fail
+ROLLBACK TO invalid_key;
+
+-- Exercise restoration immediately, before any further DDL invalidates caches.
+ROLLBACK TO original_cast;
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+INSERT INTO fk_cast_fk VALUES ('restored', ROW(1)::fk_cast_type);
+SELECT count(*) FROM fk_cast_fk;
+ROLLBACK;
+
+-- Failed fast-path rebuilds must not accumulate metadata on subxact abort.
+BEGIN;
+SAVEPOINT missing_cast;
+DROP CAST (fk_cast_type AS int);
+DO $$
+DECLARE
+ before_count bigint;
+ after_count bigint;
+BEGIN
+ SELECT count(*) INTO before_count FROM pg_backend_memory_contexts
+ WHERE name LIKE 'RI %';
+ FOR i IN 1..10 LOOP
+ BEGIN
+ INSERT INTO fk_cast_fk VALUES ('failed', ROW(1)::fk_cast_type);
+ RAISE EXCEPTION 'missing cast was not detected';
+ EXCEPTION WHEN internal_error THEN
+ IF SQLERRM NOT LIKE 'no conversion function%' THEN
+ RAISE;
+ END IF;
+ END;
+ END LOOP;
+ SELECT count(*) INTO after_count FROM pg_backend_memory_contexts
+ WHERE name LIKE 'RI %';
+ IF after_count > before_count THEN
+ RAISE EXCEPTION 'RI contexts leaked across failed checks';
+ END IF;
+END $$;
+ROLLBACK TO missing_cast;
+INSERT INTO fk_cast_fk VALUES ('after_error', ROW(1)::fk_cast_type);
+ROLLBACK;
+
+DROP TABLE fk_cast_fk, fk_cast_pk;
+DROP CAST (fk_cast_type AS int);
+DROP FUNCTION fk_cast1(fk_cast_type);
+DROP TYPE fk_cast_type;
+
+-- Invalidation during a comparison must not overwrite its call information.
+CREATE TYPE fk_cast_type AS (v int);
+CREATE TABLE fk_cast_guard (armed bool);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int LANGUAGE plpgsql AS $$
+BEGIN
+ IF EXISTS (SELECT FROM fk_cast_guard) THEN
+ DELETE FROM fk_cast_guard;
+ EXECUTE 'DROP CAST (fk_cast_type AS bigint)';
+ -- Compare a different committed row, rebuilding the same cache entry.
+ UPDATE fk_cast_fk SET id = id WHERE label = 'nested';
+ END IF;
+ RETURN $1.v;
+END $$;
+CREATE FUNCTION fk_cast_aux(fk_cast_type) RETURNS bigint
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v::bigint';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE CAST (fk_cast_type AS bigint) WITH FUNCTION fk_cast_aux(fk_cast_type);
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES ('outer', ROW(1)::fk_cast_type),
+ ('nested', ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = id WHERE label = 'outer';
+
+BEGIN;
+INSERT INTO fk_cast_guard VALUES (true);
+-- Both the outer and nested UPDATE compare rows from earlier transactions.
+UPDATE fk_cast_fk SET id = id WHERE label = 'outer';
+SELECT count(*) FROM fk_cast_guard;
+SELECT count(*) FROM fk_cast_fk;
+ROLLBACK;
+
+DROP TABLE fk_cast_fk, fk_cast_pk, fk_cast_guard;
+DROP CAST (fk_cast_type AS int);
+DROP CAST (fk_cast_type AS bigint);
+DROP FUNCTION fk_cast1(fk_cast_type), fk_cast_aux(fk_cast_type);
+DROP TYPE fk_cast_type;
+
--
-- Now some cases with inheritance
-- Basic 2 table case: 1 column of matching types.
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index e11d3ae290b..58a395da88e 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2491,6 +2491,7 @@ RBTreeIterator
REPARSE_JUNCTION_DATA_BUFFER
RIX
RI_CompareHashEntry
+RI_CompareInfo
RI_CompareKey
RI_ConstraintInfo
RI_QueryHashEntry
--
2.47.3
[application/octet-stream] PG14-18-v2-0001-Invalidate-RI-call-information-when-casts-change.patch (17.9K, ../../CA+HiwqGAq2fqXDSOUzE-uv4-LNMDcu6zHCF8co6=aRBZaTnoQg@mail.gmail.com/6-PG14-18-v2-0001-Invalidate-RI-call-information-when-casts-change.patch)
download | inline diff:
From 1ce6f7ce4000ce9318d0dea00259e4c7352b91cd Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Fri, 11 Sep 2026 17:06:50 +0900
Subject: [PATCH v2] Invalidate RI call information when casts change
Changing a cast need not modify pg_constraint, so the RI comparison
cache can keep using the old cast function after its replacement.
Dropping that function can then make an UPDATE fail with a cache lookup
error.
Invalidate comparison call information on CASTSOURCETARGET changes.
Keep it separate from its hash entry and defer releasing invalidated
objects until transaction end. A cast can run DDL and reenter RI checks,
so invalidation must not free or overwrite call information still used
by an outer comparison.
Build new call information in a transaction-owned context and reparent
it only when construction succeeds. Add AtEOXact_RI() calls on commit,
abort, and prepare to release the detached objects. Subtransaction end
is too early, since an outer comparison may still be using them.
Test replacement after cache warmup using a committed row, invalid-key
rejection, rollback restoring the old cast, and invalidation with a
nested comparison of another committed row.
Author: Nikolay Samokhvalov <nik@postgres.ai>
Co-authored-by: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CAM527d9BgPjeOOYmbCBTd57R145qHCk-dzw9qNq+nOrDq1j__A@mail.gmail.com
Backpatch-through: 14
---
src/backend/access/transam/xact.c | 3 +
src/backend/utils/adt/ri_triggers.c | 106 ++++++++++++++++++----
src/include/commands/trigger.h | 2 +
src/test/regress/expected/foreign_key.out | 87 ++++++++++++++++++
src/test/regress/sql/foreign_key.sql | 79 ++++++++++++++++
src/tools/pgindent/typedefs.list | 1 +
6 files changed, 258 insertions(+), 20 deletions(-)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index b885513f765..3f3f5bc1a3b 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2472,6 +2472,7 @@ CommitTransaction(void)
AtEOXact_Files(true);
AtEOXact_ComboCid();
AtEOXact_HashTables(true);
+ AtEOXact_RI(true);
AtEOXact_PgStat(true, is_parallel_worker);
AtEOXact_Snapshot(true, false);
AtEOXact_ApplyLauncher(true);
@@ -2766,6 +2767,7 @@ PrepareTransaction(void)
AtEOXact_Files(true);
AtEOXact_ComboCid();
AtEOXact_HashTables(true);
+ AtEOXact_RI(true);
/* don't call AtEOXact_PgStat here; we fixed pgstat state above */
AtEOXact_Snapshot(true, true);
/* we treat PREPARE as ROLLBACK so far as waking workers goes */
@@ -2990,6 +2992,7 @@ AbortTransaction(void)
AtEOXact_Files(false);
AtEOXact_ComboCid();
AtEOXact_HashTables(false);
+ AtEOXact_RI(false);
AtEOXact_PgStat(false, is_parallel_worker);
AtEOXact_ApplyLauncher(false);
AtEOXact_LogicalRepWorkers(false);
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 6239900fa28..5226f535f6f 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -165,21 +165,34 @@ typedef struct RI_CompareKey
Oid typeid; /* the data type to apply it to */
} RI_CompareKey;
+/*
+ * Cached call information is detached on invalidation, but kept until the end
+ * of the transaction in case an active comparison still references it.
+ */
+typedef struct RI_CompareInfo
+{
+ FmgrInfo eq_opr_finfo; /* call info for equality fn */
+ FmgrInfo cast_func_finfo; /* in case we must coerce input */
+ MemoryContext context;
+ struct RI_CompareInfo *next_dead;
+} RI_CompareInfo;
+
/*
* RI_CompareHashEntry
*/
typedef struct RI_CompareHashEntry
{
RI_CompareKey key;
- bool valid; /* successfully initialized? */
- FmgrInfo eq_opr_finfo; /* call info for equality fn */
- FmgrInfo cast_func_finfo; /* in case we must coerce input */
+ RI_CompareInfo *info; /* NULL if invalid */
} RI_CompareHashEntry;
/*
* Local data
*/
+/* Invalidated call information retained until AtEOXact_RI(). */
+static RI_CompareInfo *ri_compare_dead_list = NULL;
+
static HTAB *ri_constraint_cache = NULL;
static HTAB *ri_query_cache = NULL;
static HTAB *ri_compare_cache = NULL;
@@ -213,10 +226,11 @@ static bool ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid,
Datum lhs, Datum rhs);
static void ri_InitHashTables(void);
+static void InvalidateCastCacheCallBack(Datum arg, int cacheid, uint32 hashvalue);
static void InvalidateConstraintCacheCallBack(Datum arg, int cacheid, uint32 hashvalue);
static SPIPlanPtr ri_FetchPreparedPlan(RI_QueryKey *key);
static void ri_HashPreparedPlan(RI_QueryKey *key, SPIPlanPtr plan);
-static RI_CompareHashEntry *ri_HashCompareOp(Oid eq_opr, Oid typeid);
+static RI_CompareInfo *ri_HashCompareOp(Oid eq_opr, Oid typeid);
static void ri_CheckTrigger(FunctionCallInfo fcinfo, const char *funcname,
int tgkind);
@@ -2434,6 +2448,30 @@ InvalidateConstraintCacheCallBack(Datum arg, int cacheid, uint32 hashvalue)
}
+/*
+ * Cast changes can affect any comparison entry. Do not free or overwrite
+ * call information here: a cast can execute DDL and reenter RI checks while an
+ * outer comparison is still using it. AtEOXact_RI() releases detached data.
+ */
+static void
+InvalidateCastCacheCallBack(Datum arg, int cacheid, uint32 hashvalue)
+{
+ HASH_SEQ_STATUS status;
+ RI_CompareHashEntry *entry;
+
+ hash_seq_init(&status, ri_compare_cache);
+ while ((entry = hash_seq_search(&status)) != NULL)
+ {
+ if (entry->info != NULL)
+ {
+ entry->info->next_dead = ri_compare_dead_list;
+ ri_compare_dead_list = entry->info;
+ entry->info = NULL;
+ }
+ }
+}
+
+
/*
* Prepare execution plan for a query to enforce an RI restriction
*/
@@ -2883,6 +2921,10 @@ ri_InitHashTables(void)
ri_compare_cache = hash_create("RI compare cache",
RI_INIT_QUERYHASHSIZE,
&ctl, HASH_ELEM | HASH_BLOBS);
+
+ CacheRegisterSyscacheCallback(CASTSOURCETARGET,
+ InvalidateCastCacheCallBack,
+ (Datum) 0);
}
@@ -3071,7 +3113,7 @@ static bool
ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid,
Datum lhs, Datum rhs)
{
- RI_CompareHashEntry *entry = ri_HashCompareOp(eq_opr, typeid);
+ RI_CompareInfo *entry = ri_HashCompareOp(eq_opr, typeid);
/* Do we need to cast the values? */
if (OidIsValid(entry->cast_func_finfo.fn_oid))
@@ -3113,7 +3155,7 @@ ri_CompareWithCast(Oid eq_opr, Oid typeid, Oid collid,
* See if we know how to compare two values, and create a new hash entry
* if not.
*/
-static RI_CompareHashEntry *
+static RI_CompareInfo *
ri_HashCompareOp(Oid eq_opr, Oid typeid)
{
RI_CompareKey key;
@@ -3136,23 +3178,20 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
&key,
HASH_ENTER, &found);
if (!found)
- entry->valid = false;
+ entry->info = NULL;
/*
- * If not already initialized, do so. Since we'll keep this hash entry
- * for the life of the backend, put any subsidiary info for the function
- * cache structs into TopMemoryContext.
+ * If not already initialized, build a new generation of call information.
+ * Use a separate context so invalidation cannot affect active callers.
*/
- if (!entry->valid)
+ if (entry->info == NULL)
{
Oid lefttype,
righttype,
castfunc;
CoercionPathType pathtype;
-
- /* We always need to know how to call the equality operator */
- fmgr_info_cxt(get_opcode(eq_opr), &entry->eq_opr_finfo,
- TopMemoryContext);
+ MemoryContext context;
+ RI_CompareInfo *info;
/*
* If we chose to use a cast from FK to PK type, we may have to apply
@@ -3190,15 +3229,23 @@ ri_HashCompareOp(Oid eq_opr, Oid typeid)
format_type_be(lefttype));
}
}
+
+ /* Leave incomplete entries subject to normal error cleanup. */
+ context = AllocSetContextCreate(CurTransactionContext,
+ "RI compare info",
+ ALLOCSET_SMALL_SIZES);
+ info = MemoryContextAllocZero(context, sizeof(RI_CompareInfo));
+ info->context = context;
+ fmgr_info_cxt(get_opcode(eq_opr), &info->eq_opr_finfo, context);
if (OidIsValid(castfunc))
- fmgr_info_cxt(castfunc, &entry->cast_func_finfo,
- TopMemoryContext);
+ fmgr_info_cxt(castfunc, &info->cast_func_finfo, context);
else
- entry->cast_func_finfo.fn_oid = InvalidOid;
- entry->valid = true;
+ info->cast_func_finfo.fn_oid = InvalidOid;
+ MemoryContextSetParent(context, TopMemoryContext);
+ entry->info = info;
}
- return entry;
+ return entry->info;
}
@@ -3230,3 +3277,22 @@ RI_FKey_trigger_type(Oid tgfoid)
return RI_TRIGGER_NONE;
}
+
+
+/*
+ * Release comparison call information detached by invalidation callbacks.
+ * No RI comparison can still reference it at transaction end, on commit,
+ * abort, or prepare. Do not release it at subtransaction end: an outer
+ * comparison may still be using an object invalidated by a nested call.
+ */
+void
+AtEOXact_RI(bool isCommit)
+{
+ while (ri_compare_dead_list != NULL)
+ {
+ RI_CompareInfo *dead = ri_compare_dead_list;
+
+ ri_compare_dead_list = dead->next_dead;
+ MemoryContextDelete(dead->context);
+ }
+}
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index 54be07c9e42..2de516ca11d 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -289,4 +289,6 @@ extern void RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel,
extern int RI_FKey_trigger_type(Oid tgfoid);
+extern void AtEOXact_RI(bool isCommit);
+
#endif /* TRIGGER_H */
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 427a55cfee0..1c81705113d 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -983,6 +983,93 @@ CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY
ptest3) REFERENCES pktable);
ERROR: foreign key constraint "pktable_ptest4_ptest3_fkey" cannot be implemented
DETAIL: Key columns "ptest4" of the referencing table and "ptest1" of the referenced table are of incompatible types: inet and integer.
+-- Replacing a cast must invalidate cached RI comparison call information.
+CREATE TYPE fk_cast_type AS (v int);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES ('original', ROW(1)::fk_cast_type);
+-- With autocommit, this compares a committed row and commits the updated row.
+-- Updating a row inserted in the same transaction would skip the comparison.
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+BEGIN;
+SAVEPOINT original_cast;
+DROP CAST (fk_cast_type AS int);
+CREATE FUNCTION fk_cast2(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast2(fk_cast_type) AS IMPLICIT;
+DROP FUNCTION fk_cast1(fk_cast_type);
+-- Exercise the comparison cache before a new INSERT can rebuild other caches.
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+INSERT INTO fk_cast_fk VALUES ('replacement', ROW(1)::fk_cast_type);
+SAVEPOINT invalid_key;
+INSERT INTO fk_cast_fk VALUES ('invalid', ROW(2)::fk_cast_type); -- must fail
+ERROR: insert or update on table "fk_cast_fk" violates foreign key constraint "fk_cast_fk_id_fkey"
+DETAIL: Key (id)=((2)) is not present in table "fk_cast_pk".
+ROLLBACK TO invalid_key;
+-- Exercise restoration immediately, before any further DDL invalidates caches.
+ROLLBACK TO original_cast;
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+INSERT INTO fk_cast_fk VALUES ('restored', ROW(1)::fk_cast_type);
+SELECT count(*) FROM fk_cast_fk;
+ count
+-------
+ 2
+(1 row)
+
+ROLLBACK;
+DROP TABLE fk_cast_fk, fk_cast_pk;
+DROP CAST (fk_cast_type AS int);
+DROP FUNCTION fk_cast1(fk_cast_type);
+DROP TYPE fk_cast_type;
+-- Invalidation during a comparison must not overwrite its call information.
+CREATE TYPE fk_cast_type AS (v int);
+CREATE TABLE fk_cast_guard (armed bool);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int LANGUAGE plpgsql AS $$
+BEGIN
+ IF EXISTS (SELECT FROM fk_cast_guard) THEN
+ DELETE FROM fk_cast_guard;
+ EXECUTE 'DROP CAST (fk_cast_type AS bigint)';
+ -- Compare a different committed row, rebuilding the same cache entry.
+ UPDATE fk_cast_fk SET id = id WHERE label = 'nested';
+ END IF;
+ RETURN $1.v;
+END $$;
+CREATE FUNCTION fk_cast_aux(fk_cast_type) RETURNS bigint
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v::bigint';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE CAST (fk_cast_type AS bigint) WITH FUNCTION fk_cast_aux(fk_cast_type);
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES ('outer', ROW(1)::fk_cast_type),
+ ('nested', ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = id WHERE label = 'outer';
+BEGIN;
+INSERT INTO fk_cast_guard VALUES (true);
+-- Both the outer and nested UPDATE compare rows from earlier transactions.
+UPDATE fk_cast_fk SET id = id WHERE label = 'outer';
+SELECT count(*) FROM fk_cast_guard;
+ count
+-------
+ 0
+(1 row)
+
+SELECT count(*) FROM fk_cast_fk;
+ count
+-------
+ 2
+(1 row)
+
+ROLLBACK;
+DROP TABLE fk_cast_fk, fk_cast_pk, fk_cast_guard;
+DROP CAST (fk_cast_type AS int);
+DROP CAST (fk_cast_type AS bigint);
+DROP FUNCTION fk_cast1(fk_cast_type), fk_cast_aux(fk_cast_type);
+DROP TYPE fk_cast_type;
--
-- Now some cases with inheritance
-- Basic 2 table case: 1 column of matching types.
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 983bd856eeb..ba3d5a2ed3a 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -628,6 +628,85 @@ ptest3) REFERENCES pktable(ptest1, ptest2));
CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY KEY(ptest1, ptest2), FOREIGN KEY(ptest4,
ptest3) REFERENCES pktable);
+-- Replacing a cast must invalidate cached RI comparison call information.
+CREATE TYPE fk_cast_type AS (v int);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES ('original', ROW(1)::fk_cast_type);
+
+-- With autocommit, this compares a committed row and commits the updated row.
+-- Updating a row inserted in the same transaction would skip the comparison.
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+
+BEGIN;
+SAVEPOINT original_cast;
+DROP CAST (fk_cast_type AS int);
+CREATE FUNCTION fk_cast2(fk_cast_type) RETURNS int
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast2(fk_cast_type) AS IMPLICIT;
+DROP FUNCTION fk_cast1(fk_cast_type);
+
+-- Exercise the comparison cache before a new INSERT can rebuild other caches.
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+INSERT INTO fk_cast_fk VALUES ('replacement', ROW(1)::fk_cast_type);
+SAVEPOINT invalid_key;
+INSERT INTO fk_cast_fk VALUES ('invalid', ROW(2)::fk_cast_type); -- must fail
+ROLLBACK TO invalid_key;
+
+-- Exercise restoration immediately, before any further DDL invalidates caches.
+ROLLBACK TO original_cast;
+UPDATE fk_cast_fk SET id = id WHERE label = 'original';
+INSERT INTO fk_cast_fk VALUES ('restored', ROW(1)::fk_cast_type);
+SELECT count(*) FROM fk_cast_fk;
+ROLLBACK;
+
+DROP TABLE fk_cast_fk, fk_cast_pk;
+DROP CAST (fk_cast_type AS int);
+DROP FUNCTION fk_cast1(fk_cast_type);
+DROP TYPE fk_cast_type;
+
+-- Invalidation during a comparison must not overwrite its call information.
+CREATE TYPE fk_cast_type AS (v int);
+CREATE TABLE fk_cast_guard (armed bool);
+CREATE FUNCTION fk_cast1(fk_cast_type) RETURNS int LANGUAGE plpgsql AS $$
+BEGIN
+ IF EXISTS (SELECT FROM fk_cast_guard) THEN
+ DELETE FROM fk_cast_guard;
+ EXECUTE 'DROP CAST (fk_cast_type AS bigint)';
+ -- Compare a different committed row, rebuilding the same cache entry.
+ UPDATE fk_cast_fk SET id = id WHERE label = 'nested';
+ END IF;
+ RETURN $1.v;
+END $$;
+CREATE FUNCTION fk_cast_aux(fk_cast_type) RETURNS bigint
+ LANGUAGE sql IMMUTABLE STRICT AS 'SELECT $1.v::bigint';
+CREATE CAST (fk_cast_type AS int) WITH FUNCTION fk_cast1(fk_cast_type) AS IMPLICIT;
+CREATE CAST (fk_cast_type AS bigint) WITH FUNCTION fk_cast_aux(fk_cast_type);
+CREATE TABLE fk_cast_pk (id int PRIMARY KEY);
+CREATE TABLE fk_cast_fk (label text PRIMARY KEY, id fk_cast_type REFERENCES fk_cast_pk);
+INSERT INTO fk_cast_pk VALUES (1);
+INSERT INTO fk_cast_fk VALUES ('outer', ROW(1)::fk_cast_type),
+ ('nested', ROW(1)::fk_cast_type);
+UPDATE fk_cast_fk SET id = id WHERE label = 'outer';
+
+BEGIN;
+INSERT INTO fk_cast_guard VALUES (true);
+-- Both the outer and nested UPDATE compare rows from earlier transactions.
+UPDATE fk_cast_fk SET id = id WHERE label = 'outer';
+SELECT count(*) FROM fk_cast_guard;
+SELECT count(*) FROM fk_cast_fk;
+ROLLBACK;
+
+DROP TABLE fk_cast_fk, fk_cast_pk, fk_cast_guard;
+DROP CAST (fk_cast_type AS int);
+DROP CAST (fk_cast_type AS bigint);
+DROP FUNCTION fk_cast1(fk_cast_type), fk_cast_aux(fk_cast_type);
+DROP TYPE fk_cast_type;
+
--
-- Now some cases with inheritance
-- Basic 2 table case: 1 column of matching types.
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index b6f0897dc84..7205feed9b1 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2399,6 +2399,7 @@ RBTreeIterator
REPARSE_JUNCTION_DATA_BUFFER
RIX
RI_CompareHashEntry
+RI_CompareInfo
RI_CompareKey
RI_ConstraintInfo
RI_QueryHashEntry
--
2.47.3
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-11 10:08 Amit Langote <amitlangote09@gmail.com>
parent: Amit Langote <amitlangote09@gmail.com>
2 siblings, 0 replies; 16+ messages in thread
From: Amit Langote @ 2026-09-11 10:08 UTC (permalink / raw)
To: Nikolay Samokhvalov <nik@postgres.ai>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Andrey Borodin <amborodin@acm.org>; Kirk Wolak <wolakk@gmail.com>
On Fri, Sep 11, 2026 at 6:25 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Fri, Sep 11, 2026 at 9:33 AM Nikolay Samokhvalov <nik@postgres.ai> wrote:
> >
> > On Thu, Sep 10, 2026 at 4:41 PM Amit Langote wrote:
> > > Looking at these now. The first issue is clearly a fast-path code
> > > problem. The 2nd one interacts with the existing non-fast-path code so
> > > I'll need to check if the bug predates fast-path.
> >
> > Thanks Amit. In case helpful, here are two proposed fixes, with
> > regression tests.
> >
> > Built and tested with assertions; regression and isolation suites pass.
> > An independent agent reviewed and tested both, catching a cleanup issue
> > that's now fixed. I didn't have time to fully study the patches manually,
> > but my harness tested them thoroughly.
>
> Thanks, Nik. Attached are updated patches incorporating your fixes.
>
> For 0001, SPI's FOR KEY SHARE also requires UPDATE privilege on at
> least one column. I've used ExecCheckOneRelPerms() to cover that along
> with column-level SELECT. The tests exercise both per-row and batched
> checks, including rejection without UPDATE and acceptance with UPDATE
> on an unrelated column.
>
> For #2, I reproduced the stale cast cache on 18.6 by warming it with
> an UPDATE of a committed row before replacing the cast. I've adjusted
> the tests to use committed rows, since same-transaction rows bypass
> the key comparison. The nested case now also uses UPDATE to exercise
> the comparison cache on older branches.
>
> The cleanup strategy in 0002 deserves some discussion. It retains
> invalidated call information until transaction end because a cast can
> invalidate the cache and re-enter RI checks while an outer comparison
> still uses it. I've carried that approach into the backpatch, but this
> means introducing AtEOXact_RI() on pre-19 branches. I'd welcome closer
> review before settling on that strategy. Could we replace the dead
> list and explicit cleanup with reparenting to TopTransactionContext
> when an entry is invalidated? That would avoid the new hook, but needs
> checking against invalidation timing.
>
> There are separate versions of 0001 and 0002 for master and
> REL_19_STABLE. The two versions of 0002 contain the same fix and
> tests, adapted to each branch's surrounding code. A shared version of
> 0002 applies to branches 14 through 18, which have no fast-path code.
Added an open item for #1:
RI fastpath handles permissions incorrectly
Commit: 2da86c1ef9b
Owner: Amit Langote
And a "live issue" for #2:
Foreign key cast cache not invalidated properly
Commit: N/A This is an old bug predating the fast path added in 19 but
found during its testing.
Owner: Amit Langote
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-12 15:04 Nikolay Samokhvalov <nik@postgres.ai>
parent: Amit Langote <amitlangote09@gmail.com>
2 siblings, 1 reply; 16+ messages in thread
From: Nikolay Samokhvalov @ 2026-09-12 15:04 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Andrey Borodin <amborodin@acm.org>; Kirk Wolak <wolakk@gmail.com>
Hi Amit,
Thanks. Tested v2 with assertions. PG19 and PG18 regression/isolation
suites pass, as do the reentry and cleanup probes.
Two things:
- On master, `LIKE 'RI %'` catches batch flush contexts retained until
xact end. Restricting it to `RI compare info` and `RI fast-path finfo
scratch` fixes the test; all suites then pass. Checked that it still
catches the actual leak.
- The shared backpatch doesn't apply cleanly with `git apply` on
PG14–17. Adapted PG14 passes too.
I'd keep the dead list for now. No runtime issue found in v2.
Thanks,
Nik
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-13 08:20 Nikolay Samokhvalov <nik@postgres.ai>
parent: Nikolay Samokhvalov <nik@postgres.ai>
0 siblings, 0 replies; 16+ messages in thread
From: Nikolay Samokhvalov @ 2026-09-13 08:20 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>; Andrey Borodin <amborodin@acm.org>; Kirk Wolak <wolakk@gmail.com>
On Sat, Sep 12, 2026 at 8:04 AM Nikolay Samokhvalov <nik@postgres.ai> 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
Attachments:
[application/x-patch] v2-construction-fix.patch (6.8K, ../../CAM527d8az+PKmmchgb-0_9VZzn7bDGqh938ezZzKGkCTZT3NDg@mail.gmail.com/2-v2-construction-fix.patch)
download | inline diff:
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;
}
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-15 02:13 Nikolay Samokhvalov <nik@postgres.ai>
parent: Amit Langote <amitlangote09@gmail.com>
2 siblings, 2 replies; 16+ messages in thread
From: Nikolay Samokhvalov @ 2026-09-15 02:13 UTC (permalink / raw)
To: pgsql-hackers <pgsql-hackers@lists.postgresql.org>; +Cc: Amit Langote <amitlangote09@gmail.com>
On Fri, Sep 11, 2026 at 2:25 AM Amit Langote
<amitlangote09@gmail.com> wrote:
> For 0001, SPI's FOR KEY SHARE also requires UPDATE privilege on at
> least one column. I've used ExecCheckOneRelPerms() to cover that along
> with column-level SELECT. The tests exercise both per-row and batched
> checks, including rejection without UPDATE and acceptance with UPDATE
> on an unrelated column.
I kept testing with my AI harness and found another case on master and
PG19. Replacing a loose cross-type equality member leaves the FK's stored
operator unchanged. An uncached fast-path check then errors; warmed
metadata is not invalidated by the pg_amop change. The replacement calls
the same int48eq function, so the family semantics are unchanged and SPI
continues to enforce the FK normally.
Attached are standalone fixes for master a625fc57 and PG19 f4b511ae.
They invalidate the metadata on pg_amop changes and use SPI unless the
stored operator is still an equality member. On master, buffered rows
also need the SPI fallback if the family changes between AFTER triggers.
Both assertion builds pass the native foreign_key test, full regression
and isolation suites, and the injection-point suites. The tests cover
warmed and uncached metadata and missing keys on both sides of the DDL.
Thanks,
Nik
Attachments:
[application/x-patch] master-v1-0001-Check-RI-fast-path-operator-family-membership.patch (23.2K, ../../CAM527d9PzFzagr67N0=Ex2ng1p5HzrcAszy3j5OoZKHXQMARXA@mail.gmail.com/2-master-v1-0001-Check-RI-fast-path-operator-family-membership.patch)
download | inline diff:
From 2020efd6cca71b90916e4bc3625b485fabba453f Mon Sep 17 00:00:00 2001
From: Nik Samokhvalov <nik@postgres.ai>
Date: Mon, 14 Sep 2026 17:17:55 -0700
Subject: [PATCH] Check RI fast-path operator family membership
A loose cross-type equality member can be replaced without changing an
existing foreign key's recorded operator. Invalidate fast-path metadata
on pg_amop changes and fall back to SPI unless that operator is still an
equality member of the referenced index's operator family.
Operator family DDL can also occur between AFTER triggers while a batch
has buffered rows. Recheck eligibility when flushing and use the same SPI
plan as the per-row path for any rows that can no longer be checked by a
direct index probe.
Add regression coverage for warmed and uncached metadata, and for valid
and missing keys on both sides of a mid-batch operator family change.
---
src/backend/utils/adt/ri_triggers.c | 294 +++++++++++++++-------
src/test/regress/expected/foreign_key.out | 141 +++++++++++
src/test/regress/sql/foreign_key.sql | 88 +++++++
3 files changed, 428 insertions(+), 95 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index c46f789..66bfcfe 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -355,6 +355,9 @@ static void ri_InitHashTables(void);
static void InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
uint32 hashvalue);
static SPIPlanPtr ri_FetchPreparedPlan(RI_QueryKey *key);
+static SPIPlanPtr ri_FetchPreparedCheckPlan(RI_QueryKey *key,
+ const RI_ConstraintInfo *riinfo,
+ Relation fk_rel, Relation pk_rel);
static void ri_HashPreparedPlan(RI_QueryKey *key, SPIPlanPtr plan);
static RI_CompareHashEntry *ri_HashCompareOp(Oid eq_opr, Oid typeid);
@@ -378,6 +381,9 @@ static bool ri_FastPathBatchAdd(RI_ConstraintInfo *riinfo,
Relation fk_rel, TupleTableSlot *newslot);
static void ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel,
RI_ConstraintInfo *riinfo);
+static void ri_FastPathBatchFlushSPI(RI_FastPathEntry *fpentry,
+ Relation fk_rel,
+ RI_ConstraintInfo *riinfo);
static int ri_FastPathFlushArray(RI_FastPathEntry *fpentry, TupleTableSlot *fk_slot,
const RI_ConstraintInfo *riinfo,
FastPathMeta *fpmeta, Relation fk_rel,
@@ -565,92 +571,7 @@ RI_FKey_check(TriggerData *trigdata)
/* Fetch or prepare a saved plan for the real check */
ri_BuildQueryKey(&qkey, riinfo, RI_PLAN_CHECK_LOOKUPPK);
-
- if ((qplan = ri_FetchPreparedPlan(&qkey)) == NULL)
- {
- StringInfoData querybuf;
- char pkrelname[MAX_QUOTED_REL_NAME_LEN];
- char attname[MAX_QUOTED_NAME_LEN];
- char paramname[16];
- const char *querysep;
- Oid queryoids[RI_MAX_NUMKEYS];
- const char *pk_only;
-
- /* ----------
- * The query string built is
- * SELECT 1 FROM [ONLY] <pktable> x WHERE pkatt1 = $1 [AND ...]
- * FOR KEY SHARE OF x
- * The type id's for the $ parameters are those of the
- * corresponding FK attributes.
- *
- * But for temporal FKs we need to make sure
- * the FK's range is completely covered.
- * So we use this query instead:
- * SELECT 1
- * FROM (
- * SELECT pkperiodatt AS r
- * FROM [ONLY] pktable x
- * WHERE pkatt1 = $1 [AND ...]
- * AND pkperiodatt && $n
- * FOR KEY SHARE OF x
- * ) x1
- * HAVING $n <@ range_agg(x1.r)
- * Note if FOR KEY SHARE ever allows GROUP BY and HAVING
- * we can make this a bit simpler.
- * ----------
- */
- initStringInfo(&querybuf);
- pk_only = pk_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ?
- "" : "ONLY ";
- quoteRelationName(pkrelname, pk_rel);
- if (riinfo->hasperiod)
- {
- quoteOneName(attname,
- RIAttName(pk_rel, riinfo->pk_attnums[riinfo->nkeys - 1]));
-
- appendStringInfo(&querybuf,
- "SELECT 1 FROM (SELECT %s AS r FROM %s%s x",
- attname, pk_only, pkrelname);
- }
- else
- {
- appendStringInfo(&querybuf, "SELECT 1 FROM %s%s x",
- pk_only, pkrelname);
- }
- querysep = "WHERE";
- for (int i = 0; i < riinfo->nkeys; i++)
- {
- Oid pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
- Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
-
- quoteOneName(attname,
- RIAttName(pk_rel, riinfo->pk_attnums[i]));
- sprintf(paramname, "$%d", i + 1);
- ri_GenerateQual(&querybuf, querysep,
- attname, pk_type,
- riinfo->pf_eq_oprs[i],
- paramname, fk_type);
- querysep = "AND";
- queryoids[i] = fk_type;
- }
- appendStringInfoString(&querybuf, " FOR KEY SHARE OF x");
- if (riinfo->hasperiod)
- {
- Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[riinfo->nkeys - 1]);
-
- appendStringInfoString(&querybuf, ") x1 HAVING ");
- sprintf(paramname, "$%d", riinfo->nkeys);
- ri_GenerateQual(&querybuf, "",
- paramname, fk_type,
- riinfo->agged_period_contained_by_oper,
- "pg_catalog.range_agg", ANYMULTIRANGEOID);
- appendStringInfoString(&querybuf, "(x1.r)");
- }
-
- /* Prepare and save the plan */
- qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
- &qkey, fk_rel, pk_rel);
- }
+ qplan = ri_FetchPreparedCheckPlan(&qkey, riinfo, fk_rel, pk_rel);
/*
* Now check that foreign key exists in PK table
@@ -2601,7 +2522,7 @@ get_ri_constraint_root(Oid constrOid)
}
/*
- * Callback for pg_constraint inval events
+ * Callback for pg_constraint and pg_amop inval events
*
* While most syscache callbacks just flush all their entries, pg_constraint
* gets enough update traffic that it's probably worth being smarter.
@@ -2626,6 +2547,10 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
Assert(ri_constraint_cache != NULL);
+ /* pg_amop changes can affect any constraint's fast-path metadata. */
+ if (cacheid == AMOPOPID)
+ hashvalue = 0;
+
/*
* If the list of currently valid entries gets excessively large, we mark
* them all invalid so we can empty the list. This arrangement avoids
@@ -2719,6 +2644,105 @@ ri_PlanCheck(const char *querystr, int nargs, const Oid *argtypes,
return qplan;
}
+/*
+ * Fetch or prepare the plan used to check a foreign key row via SPI.
+ */
+static SPIPlanPtr
+ri_FetchPreparedCheckPlan(RI_QueryKey *qkey,
+ const RI_ConstraintInfo *riinfo,
+ Relation fk_rel, Relation pk_rel)
+{
+ SPIPlanPtr qplan;
+
+ if ((qplan = ri_FetchPreparedPlan(qkey)) == NULL)
+ {
+ StringInfoData querybuf;
+ char pkrelname[MAX_QUOTED_REL_NAME_LEN];
+ char attname[MAX_QUOTED_NAME_LEN];
+ char paramname[16];
+ const char *querysep;
+ Oid queryoids[RI_MAX_NUMKEYS];
+ const char *pk_only;
+
+ /* ----------
+ * The query string built is
+ * SELECT 1 FROM [ONLY] <pktable> x WHERE pkatt1 = $1 [AND ...]
+ * FOR KEY SHARE OF x
+ * The type id's for the $ parameters are those of the
+ * corresponding FK attributes.
+ *
+ * But for temporal FKs we need to make sure
+ * the FK's range is completely covered.
+ * So we use this query instead:
+ * SELECT 1
+ * FROM (
+ * SELECT pkperiodatt AS r
+ * FROM [ONLY] pktable x
+ * WHERE pkatt1 = $1 [AND ...]
+ * AND pkperiodatt && $n
+ * FOR KEY SHARE OF x
+ * ) x1
+ * HAVING $n <@ range_agg(x1.r)
+ * Note if FOR KEY SHARE ever allows GROUP BY and HAVING
+ * we can make this a bit simpler.
+ * ----------
+ */
+ initStringInfo(&querybuf);
+ pk_only = pk_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ?
+ "" : "ONLY ";
+ quoteRelationName(pkrelname, pk_rel);
+ if (riinfo->hasperiod)
+ {
+ quoteOneName(attname,
+ RIAttName(pk_rel, riinfo->pk_attnums[riinfo->nkeys - 1]));
+
+ appendStringInfo(&querybuf,
+ "SELECT 1 FROM (SELECT %s AS r FROM %s%s x",
+ attname, pk_only, pkrelname);
+ }
+ else
+ {
+ appendStringInfo(&querybuf, "SELECT 1 FROM %s%s x",
+ pk_only, pkrelname);
+ }
+ querysep = "WHERE";
+ for (int i = 0; i < riinfo->nkeys; i++)
+ {
+ Oid pk_type = RIAttType(pk_rel, riinfo->pk_attnums[i]);
+ Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[i]);
+
+ quoteOneName(attname,
+ RIAttName(pk_rel, riinfo->pk_attnums[i]));
+ sprintf(paramname, "$%d", i + 1);
+ ri_GenerateQual(&querybuf, querysep,
+ attname, pk_type,
+ riinfo->pf_eq_oprs[i],
+ paramname, fk_type);
+ querysep = "AND";
+ queryoids[i] = fk_type;
+ }
+ appendStringInfoString(&querybuf, " FOR KEY SHARE OF x");
+ if (riinfo->hasperiod)
+ {
+ Oid fk_type = RIAttType(fk_rel, riinfo->fk_attnums[riinfo->nkeys - 1]);
+
+ appendStringInfoString(&querybuf, ") x1 HAVING ");
+ sprintf(paramname, "$%d", riinfo->nkeys);
+ ri_GenerateQual(&querybuf, "",
+ paramname, fk_type,
+ riinfo->agged_period_contained_by_oper,
+ "pg_catalog.range_agg", ANYMULTIRANGEOID);
+ appendStringInfoString(&querybuf, "(x1.r)");
+ }
+
+ /* Prepare and save the plan */
+ qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids,
+ qkey, fk_rel, pk_rel);
+ }
+
+ return qplan;
+}
+
/*
* Perform a query to enforce an RI restriction
*/
@@ -3073,6 +3097,13 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel,
if (fpentry->batch_count == 0)
return;
+ /* The operator family may have changed since these rows were buffered. */
+ if (!ri_check_fastpath_index(riinfo, pk_rel, idx_rel))
+ {
+ ri_FastPathBatchFlushSPI(fpentry, fk_rel, riinfo);
+ return;
+ }
+
/*
* CCI and security context switch are done once for the entire batch.
* Per-row CCI is unnecessary because by the time a flush runs, all AFTER
@@ -3177,6 +3208,53 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel,
MemoryContextSwitchTo(oldcxt);
}
+/*
+ * Check buffered rows through SPI after the index becomes unsuitable for
+ * direct probing. This can happen if user code changes the operator family
+ * between AFTER triggers in the same firing cycle.
+ */
+static void
+ri_FastPathBatchFlushSPI(RI_FastPathEntry *fpentry, Relation fk_rel,
+ RI_ConstraintInfo *riinfo)
+{
+ RI_QueryKey qkey;
+ SPIPlanPtr qplan;
+
+ if (fpentry->batch_count == 0)
+ return;
+
+ /* Protect the batch array from reentrant checks, as in the direct path. */
+ Assert(!fpentry->flushing);
+ fpentry->flushing = true;
+ PG_TRY();
+ {
+ SPI_connect();
+ ri_BuildQueryKey(&qkey, riinfo, RI_PLAN_CHECK_LOOKUPPK);
+ qplan = ri_FetchPreparedCheckPlan(&qkey, riinfo, fk_rel,
+ fpentry->pk_rel);
+
+ for (int i = 0; i < fpentry->batch_count; i++)
+ {
+ ExecStoreHeapTuple(fpentry->batch[i], fpentry->fk_slot, false);
+ ri_PerformCheck(riinfo, &qkey, qplan,
+ fk_rel, fpentry->pk_rel,
+ NULL, fpentry->fk_slot,
+ false, false, SPI_OK_SELECT);
+ }
+
+ if (SPI_finish() != SPI_OK_FINISH)
+ elog(ERROR, "SPI_finish failed");
+ }
+ PG_FINALLY();
+ {
+ fpentry->flushing = false;
+ fpentry->batch_count = 0;
+ }
+ PG_END_TRY();
+
+ MemoryContextReset(fpentry->flush_cxt);
+}
+
/*
* ri_FastPathFlushLoop
* Multi-column fallback: probe the index once per buffered row.
@@ -3566,6 +3644,32 @@ ri_check_fastpath_index(RI_ConstraintInfo *riinfo,
}
}
+ /*
+ * The equality operator stored in pg_constraint must still be an equality
+ * member of the index opfamily. A loose cross-type member can be
+ * replaced without changing the constraint itself; leave that case to
+ * SPI, which continues to use the operator recorded by the constraint.
+ */
+ for (int i = 0; i < riinfo->nkeys; i++)
+ {
+ int idx_col;
+
+ for (idx_col = 0; idx_col < idx_rel->rd_index->indnkeyatts; idx_col++)
+ {
+ if (idx_rel->rd_index->indkey.values[idx_col] ==
+ riinfo->pk_attnums[i])
+ break;
+ }
+ Assert(idx_col < idx_rel->rd_index->indnkeyatts);
+
+ if (get_op_opfamily_strategy(riinfo->pf_eq_oprs[i],
+ idx_rel->rd_opfamily[idx_col]) != BTEqualStrategyNumber)
+ {
+ riinfo->fastpath_state = RI_FASTPATH_UNUSABLE;
+ return false;
+ }
+ }
+
riinfo->fastpath_state = RI_FASTPATH_USABLE;
return true;
}
@@ -4022,10 +4126,13 @@ ri_InitHashTables(void)
RI_INIT_CONSTRAINTHASHSIZE,
&ctl, HASH_ELEM | HASH_BLOBS);
- /* Arrange to flush cache on pg_constraint changes */
+ /* Arrange to flush cache on pg_constraint or pg_amop changes */
CacheRegisterSyscacheCallback(CONSTROID,
InvalidateConstraintCacheCallBack,
(Datum) 0);
+ CacheRegisterSyscacheCallback(AMOPOPID,
+ InvalidateConstraintCacheCallBack,
+ (Datum) 0);
ctl.keysize = sizeof(RI_QueryKey);
ctl.entrysize = sizeof(RI_QueryHashEntry);
@@ -4794,14 +4901,11 @@ ri_FastPathGetEntry(RI_ConstraintInfo *riinfo, Relation fk_rel)
{
/*
* Invalidation can reset the cached eligibility while an entry is
- * still in use. Its held index remains usable, even if REINDEX
- * CONCURRENTLY has replaced it with an equivalent new index.
+ * still in use. REINDEX CONCURRENTLY leaves its held index usable,
+ * but an operator family change can require falling back to SPI.
+ * Leave any buffered rows for the end-of-batch callback to check.
*/
- bool usable;
-
- usable = ri_check_fastpath_index(riinfo, entry->pk_rel, entry->idx_rel);
- Assert(usable);
- if (!usable)
+ if (!ri_check_fastpath_index(riinfo, entry->pk_rel, entry->idx_rel))
return NULL;
}
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 8d81240..a5a428f 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1030,6 +1030,147 @@ CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY
ptest3) REFERENCES pktable);
ERROR: foreign key constraint "pktable_ptest4_ptest3_fkey" cannot be implemented
DETAIL: Key columns "ptest4" of the referencing table and "ptest1" of the referenced table are of incompatible types: inet and integer.
+-- Replacing a loose cross-type operator family member must invalidate the
+-- fast-path metadata. The FK continues using its stored equality operator,
+-- which need no longer be a member of the index family. Use the very same
+-- implementation for the replacement, keeping the family semantics unchanged.
+create schema fk_opfamily;
+set search_path = fk_opfamily, pg_catalog;
+create operator family fam using btree;
+create operator class int_ops for type integer using btree family fam as
+ operator 1 <(integer,integer), operator 2 <=(integer,integer),
+ operator 3 =(integer,integer), operator 4 >=(integer,integer),
+ operator 5 >(integer,integer), function 1 btint4cmp(integer,integer);
+alter operator family fam using btree add
+ operator 1 <(integer,bigint), operator 2 <=(integer,bigint),
+ operator 3 =(integer,bigint), operator 4 >=(integer,bigint),
+ operator 5 >(integer,bigint),
+ operator 1 <(bigint,integer), operator 2 <=(bigint,integer),
+ operator 3 =(bigint,integer), operator 4 >=(bigint,integer),
+ operator 5 >(bigint,integer),
+ operator 1 <(bigint,bigint), operator 2 <=(bigint,bigint),
+ operator 3 =(bigint,bigint), operator 4 >=(bigint,bigint),
+ operator 5 >(bigint,bigint),
+ function 1 (integer,bigint) btint48cmp(integer,bigint),
+ function 1 (bigint,integer) btint84cmp(bigint,integer),
+ function 1 (bigint,bigint) btint8cmp(bigint,bigint);
+create operator =#= (leftarg=integer, rightarg=bigint, function=int48eq);
+create table p(k integer);
+create unique index p_idx on p(k int_ops);
+create table warm(k bigint references p(k));
+create table cold(k bigint references p(k));
+insert into p values (1), (2);
+insert into warm values (1);
+select amvalidate(oid) from pg_opclass
+where opcnamespace = 'fk_opfamily'::regnamespace;
+ amvalidate
+------------
+ t
+(1 row)
+
+select exists (select from pg_backend_memory_contexts
+ where name = 'RI fast-path finfo scratch') as metadata_cached;
+ metadata_cached
+-----------------
+ t
+(1 row)
+
+-- Change only pg_amop after warming the cache.
+begin;
+alter operator family fam using btree drop operator 3(integer,bigint);
+alter operator family fam using btree add operator 3 =#=(integer,bigint);
+commit;
+select amvalidate(oid) from pg_opclass
+where opcnamespace = 'fk_opfamily'::regnamespace;
+ amvalidate
+------------
+ t
+(1 row)
+
+select exists (select from pg_backend_memory_contexts
+ where name = 'RI fast-path finfo scratch') as metadata_cached;
+ metadata_cached
+-----------------
+ f
+(1 row)
+
+insert into warm values (2);
+insert into warm values (99);
+ERROR: insert or update on table "warm" violates foreign key constraint "warm_k_fkey"
+DETAIL: Key (k)=(99) is not present in table "p".
+-- This constraint has no cached fast-path metadata yet.
+insert into cold values (2);
+insert into cold values (99);
+ERROR: insert or update on table "cold" violates foreign key constraint "cold_k_fkey"
+DETAIL: Key (k)=(99) is not present in table "p".
+select * from warm order by k;
+ k
+---
+ 1
+ 2
+(2 rows)
+
+select * from cold order by k;
+ k
+---
+ 2
+(1 row)
+
+-- Change the family between AFTER triggers. The RI trigger sorts first, so
+-- rows buffered before the DDL must also be checked through SPI at batch end.
+alter operator family fam using btree drop operator 3(integer,bigint);
+alter operator family fam using btree add operator 3 =(integer,bigint);
+create table batch(k bigint references p(k));
+create function change_family() returns trigger language plpgsql as $$
+begin
+ if new.k = 1 then
+ alter operator family fam using btree drop operator 3(integer,bigint);
+ alter operator family fam using btree add operator 3 =#=(integer,bigint);
+ end if;
+ return null;
+end
+$$;
+create trigger zzz_change_family after insert on batch
+ for each row execute function change_family();
+begin;
+insert into batch values (1), (2);
+select * from batch order by k;
+ k
+---
+ 1
+ 2
+(2 rows)
+
+rollback;
+-- No later RI trigger is needed to notice that the index became unsuitable.
+begin;
+insert into batch values (1);
+select * from batch;
+ k
+---
+ 1
+(1 row)
+
+rollback;
+-- Reject missing keys both after and before the DDL boundary.
+insert into batch values (1), (99);
+ERROR: insert or update on table "batch" violates foreign key constraint "batch_k_fkey"
+DETAIL: Key (k)=(99) is not present in table "p".
+insert into batch values (99), (1);
+ERROR: insert or update on table "batch" violates foreign key constraint "batch_k_fkey"
+DETAIL: Key (k)=(99) is not present in table "p".
+select * from batch;
+ k
+---
+(0 rows)
+
+reset search_path;
+drop table fk_opfamily.warm, fk_opfamily.cold, fk_opfamily.batch, fk_opfamily.p;
+drop function fk_opfamily.change_family();
+drop operator class fk_opfamily.int_ops using btree;
+drop operator family fk_opfamily.fam using btree;
+drop operator fk_opfamily.=#=(integer,bigint);
+drop schema fk_opfamily;
--
-- Now some cases with inheritance
-- Basic 2 table case: 1 column of matching types.
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 184d9ef..ba4c408 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -692,6 +692,94 @@ ptest3) REFERENCES pktable(ptest1, ptest2));
CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY KEY(ptest1, ptest2), FOREIGN KEY(ptest4,
ptest3) REFERENCES pktable);
+-- Replacing a loose cross-type operator family member must invalidate the
+-- fast-path metadata. The FK continues using its stored equality operator,
+-- which need no longer be a member of the index family. Use the very same
+-- implementation for the replacement, keeping the family semantics unchanged.
+create schema fk_opfamily;
+set search_path = fk_opfamily, pg_catalog;
+create operator family fam using btree;
+create operator class int_ops for type integer using btree family fam as
+ operator 1 <(integer,integer), operator 2 <=(integer,integer),
+ operator 3 =(integer,integer), operator 4 >=(integer,integer),
+ operator 5 >(integer,integer), function 1 btint4cmp(integer,integer);
+alter operator family fam using btree add
+ operator 1 <(integer,bigint), operator 2 <=(integer,bigint),
+ operator 3 =(integer,bigint), operator 4 >=(integer,bigint),
+ operator 5 >(integer,bigint),
+ operator 1 <(bigint,integer), operator 2 <=(bigint,integer),
+ operator 3 =(bigint,integer), operator 4 >=(bigint,integer),
+ operator 5 >(bigint,integer),
+ operator 1 <(bigint,bigint), operator 2 <=(bigint,bigint),
+ operator 3 =(bigint,bigint), operator 4 >=(bigint,bigint),
+ operator 5 >(bigint,bigint),
+ function 1 (integer,bigint) btint48cmp(integer,bigint),
+ function 1 (bigint,integer) btint84cmp(bigint,integer),
+ function 1 (bigint,bigint) btint8cmp(bigint,bigint);
+create operator =#= (leftarg=integer, rightarg=bigint, function=int48eq);
+create table p(k integer);
+create unique index p_idx on p(k int_ops);
+create table warm(k bigint references p(k));
+create table cold(k bigint references p(k));
+insert into p values (1), (2);
+insert into warm values (1);
+select amvalidate(oid) from pg_opclass
+where opcnamespace = 'fk_opfamily'::regnamespace;
+select exists (select from pg_backend_memory_contexts
+ where name = 'RI fast-path finfo scratch') as metadata_cached;
+-- Change only pg_amop after warming the cache.
+begin;
+alter operator family fam using btree drop operator 3(integer,bigint);
+alter operator family fam using btree add operator 3 =#=(integer,bigint);
+commit;
+select amvalidate(oid) from pg_opclass
+where opcnamespace = 'fk_opfamily'::regnamespace;
+select exists (select from pg_backend_memory_contexts
+ where name = 'RI fast-path finfo scratch') as metadata_cached;
+insert into warm values (2);
+insert into warm values (99);
+-- This constraint has no cached fast-path metadata yet.
+insert into cold values (2);
+insert into cold values (99);
+select * from warm order by k;
+select * from cold order by k;
+-- Change the family between AFTER triggers. The RI trigger sorts first, so
+-- rows buffered before the DDL must also be checked through SPI at batch end.
+alter operator family fam using btree drop operator 3(integer,bigint);
+alter operator family fam using btree add operator 3 =(integer,bigint);
+create table batch(k bigint references p(k));
+create function change_family() returns trigger language plpgsql as $$
+begin
+ if new.k = 1 then
+ alter operator family fam using btree drop operator 3(integer,bigint);
+ alter operator family fam using btree add operator 3 =#=(integer,bigint);
+ end if;
+ return null;
+end
+$$;
+create trigger zzz_change_family after insert on batch
+ for each row execute function change_family();
+begin;
+insert into batch values (1), (2);
+select * from batch order by k;
+rollback;
+-- No later RI trigger is needed to notice that the index became unsuitable.
+begin;
+insert into batch values (1);
+select * from batch;
+rollback;
+-- Reject missing keys both after and before the DDL boundary.
+insert into batch values (1), (99);
+insert into batch values (99), (1);
+select * from batch;
+reset search_path;
+drop table fk_opfamily.warm, fk_opfamily.cold, fk_opfamily.batch, fk_opfamily.p;
+drop function fk_opfamily.change_family();
+drop operator class fk_opfamily.int_ops using btree;
+drop operator family fk_opfamily.fam using btree;
+drop operator fk_opfamily.=#=(integer,bigint);
+drop schema fk_opfamily;
+
--
-- Now some cases with inheritance
-- Basic 2 table case: 1 column of matching types.
base-commit: a625fc570c22e199471e1a2656e2c32b8dc0c0fd
--
2.50.1 (Apple Git-155)
[application/x-patch] REL_19_STABLE-v1-0001-Check-RI-fast-path-operator-family-membership.patch (13.4K, ../../CAM527d9PzFzagr67N0=Ex2ng1p5HzrcAszy3j5OoZKHXQMARXA@mail.gmail.com/3-REL_19_STABLE-v1-0001-Check-RI-fast-path-operator-family-membership.patch)
download | inline diff:
From ff40ce13e736ec7824b583a213f388faf4e7d6c4 Mon Sep 17 00:00:00 2001
From: Nik Samokhvalov <nik@postgres.ai>
Date: Mon, 14 Sep 2026 17:19:36 -0700
Subject: [PATCH] Check RI fast-path operator family membership
A loose cross-type equality member can be replaced without changing an
existing foreign key's recorded operator. Invalidate fast-path metadata
on pg_amop changes and fall back to SPI unless that operator is still an
equality member of the referenced index's operator family.
Add regression coverage for warmed and uncached metadata, including an
operator family change between AFTER triggers. PG19 has no batching code,
so no batch-flush fallback is needed on this branch.
Backpatch-through: 19
---
src/backend/utils/adt/ri_triggers.c | 37 +++++-
src/test/regress/expected/foreign_key.out | 141 ++++++++++++++++++++++
src/test/regress/sql/foreign_key.sql | 88 ++++++++++++++
3 files changed, 264 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 5d55a2006e..4eb6731ac1 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -2481,7 +2481,7 @@ get_ri_constraint_root(Oid constrOid)
}
/*
- * Callback for pg_constraint inval events
+ * Callback for pg_constraint and pg_amop inval events
*
* While most syscache callbacks just flush all their entries, pg_constraint
* gets enough update traffic that it's probably worth being smarter.
@@ -2506,6 +2506,10 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
Assert(ri_constraint_cache != NULL);
+ /* pg_amop changes can affect any constraint's fast-path metadata. */
+ if (cacheid == AMOPOPID)
+ hashvalue = 0;
+
/*
* If the list of currently valid entries gets excessively large, we mark
* them all invalid so we can empty the list. This arrangement avoids
@@ -3047,6 +3051,32 @@ ri_check_fastpath_index(RI_ConstraintInfo *riinfo,
}
}
+ /*
+ * The equality operator stored in pg_constraint must still be an equality
+ * member of the index opfamily. A loose cross-type member can be
+ * replaced without changing the constraint itself; leave that case to
+ * SPI, which continues to use the operator recorded by the constraint.
+ */
+ for (int i = 0; i < riinfo->nkeys; i++)
+ {
+ int idx_col;
+
+ for (idx_col = 0; idx_col < idx_rel->rd_index->indnkeyatts; idx_col++)
+ {
+ if (idx_rel->rd_index->indkey.values[idx_col] ==
+ riinfo->pk_attnums[i])
+ break;
+ }
+ Assert(idx_col < idx_rel->rd_index->indnkeyatts);
+
+ if (get_op_opfamily_strategy(riinfo->pf_eq_oprs[i],
+ idx_rel->rd_opfamily[idx_col]) != BTEqualStrategyNumber)
+ {
+ riinfo->fastpath_state = RI_FASTPATH_UNUSABLE;
+ return false;
+ }
+ }
+
riinfo->fastpath_state = RI_FASTPATH_USABLE;
return true;
}
@@ -3503,10 +3533,13 @@ ri_InitHashTables(void)
RI_INIT_CONSTRAINTHASHSIZE,
&ctl, HASH_ELEM | HASH_BLOBS);
- /* Arrange to flush cache on pg_constraint changes */
+ /* Arrange to flush cache on pg_constraint or pg_amop changes */
CacheRegisterSyscacheCallback(CONSTROID,
InvalidateConstraintCacheCallBack,
(Datum) 0);
+ CacheRegisterSyscacheCallback(AMOPOPID,
+ InvalidateConstraintCacheCallBack,
+ (Datum) 0);
ctl.keysize = sizeof(RI_QueryKey);
ctl.entrysize = sizeof(RI_QueryHashEntry);
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 9cea669b6f..06addc4dd8 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1030,6 +1030,147 @@ CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY
ptest3) REFERENCES pktable);
ERROR: foreign key constraint "pktable_ptest4_ptest3_fkey" cannot be implemented
DETAIL: Key columns "ptest4" of the referencing table and "ptest1" of the referenced table are of incompatible types: inet and integer.
+-- Replacing a loose cross-type operator family member must invalidate the
+-- fast-path metadata. The FK continues using its stored equality operator,
+-- which need no longer be a member of the index family. Use the very same
+-- implementation for the replacement, keeping the family semantics unchanged.
+create schema fk_opfamily;
+set search_path = fk_opfamily, pg_catalog;
+create operator family fam using btree;
+create operator class int_ops for type integer using btree family fam as
+ operator 1 <(integer,integer), operator 2 <=(integer,integer),
+ operator 3 =(integer,integer), operator 4 >=(integer,integer),
+ operator 5 >(integer,integer), function 1 btint4cmp(integer,integer);
+alter operator family fam using btree add
+ operator 1 <(integer,bigint), operator 2 <=(integer,bigint),
+ operator 3 =(integer,bigint), operator 4 >=(integer,bigint),
+ operator 5 >(integer,bigint),
+ operator 1 <(bigint,integer), operator 2 <=(bigint,integer),
+ operator 3 =(bigint,integer), operator 4 >=(bigint,integer),
+ operator 5 >(bigint,integer),
+ operator 1 <(bigint,bigint), operator 2 <=(bigint,bigint),
+ operator 3 =(bigint,bigint), operator 4 >=(bigint,bigint),
+ operator 5 >(bigint,bigint),
+ function 1 (integer,bigint) btint48cmp(integer,bigint),
+ function 1 (bigint,integer) btint84cmp(bigint,integer),
+ function 1 (bigint,bigint) btint8cmp(bigint,bigint);
+create operator =#= (leftarg=integer, rightarg=bigint, function=int48eq);
+create table p(k integer);
+create unique index p_idx on p(k int_ops);
+create table warm(k bigint references p(k));
+create table cold(k bigint references p(k));
+insert into p values (1), (2);
+insert into warm values (1);
+select amvalidate(oid) from pg_opclass
+where opcnamespace = 'fk_opfamily'::regnamespace;
+ amvalidate
+------------
+ t
+(1 row)
+
+select exists (select from pg_backend_memory_contexts
+ where name = 'RI fast-path finfo scratch') as metadata_cached;
+ metadata_cached
+-----------------
+ t
+(1 row)
+
+-- Change only pg_amop after warming the cache.
+begin;
+alter operator family fam using btree drop operator 3(integer,bigint);
+alter operator family fam using btree add operator 3 =#=(integer,bigint);
+commit;
+select amvalidate(oid) from pg_opclass
+where opcnamespace = 'fk_opfamily'::regnamespace;
+ amvalidate
+------------
+ t
+(1 row)
+
+select exists (select from pg_backend_memory_contexts
+ where name = 'RI fast-path finfo scratch') as metadata_cached;
+ metadata_cached
+-----------------
+ f
+(1 row)
+
+insert into warm values (2);
+insert into warm values (99);
+ERROR: insert or update on table "warm" violates foreign key constraint "warm_k_fkey"
+DETAIL: Key (k)=(99) is not present in table "p".
+-- This constraint has no cached fast-path metadata yet.
+insert into cold values (2);
+insert into cold values (99);
+ERROR: insert or update on table "cold" violates foreign key constraint "cold_k_fkey"
+DETAIL: Key (k)=(99) is not present in table "p".
+select * from warm order by k;
+ k
+---
+ 1
+ 2
+(2 rows)
+
+select * from cold order by k;
+ k
+---
+ 2
+(1 row)
+
+-- Change the family between AFTER triggers. The RI trigger sorts first, so
+-- rows buffered before the DDL must also be checked through SPI at batch end.
+alter operator family fam using btree drop operator 3(integer,bigint);
+alter operator family fam using btree add operator 3 =(integer,bigint);
+create table batch(k bigint references p(k));
+create function change_family() returns trigger language plpgsql as $$
+begin
+ if new.k = 1 then
+ alter operator family fam using btree drop operator 3(integer,bigint);
+ alter operator family fam using btree add operator 3 =#=(integer,bigint);
+ end if;
+ return null;
+end
+$$;
+create trigger zzz_change_family after insert on batch
+ for each row execute function change_family();
+begin;
+insert into batch values (1), (2);
+select * from batch order by k;
+ k
+---
+ 1
+ 2
+(2 rows)
+
+rollback;
+-- No later RI trigger is needed to notice that the index became unsuitable.
+begin;
+insert into batch values (1);
+select * from batch;
+ k
+---
+ 1
+(1 row)
+
+rollback;
+-- Reject missing keys both after and before the DDL boundary.
+insert into batch values (1), (99);
+ERROR: insert or update on table "batch" violates foreign key constraint "batch_k_fkey"
+DETAIL: Key (k)=(99) is not present in table "p".
+insert into batch values (99), (1);
+ERROR: insert or update on table "batch" violates foreign key constraint "batch_k_fkey"
+DETAIL: Key (k)=(99) is not present in table "p".
+select * from batch;
+ k
+---
+(0 rows)
+
+reset search_path;
+drop table fk_opfamily.warm, fk_opfamily.cold, fk_opfamily.batch, fk_opfamily.p;
+drop function fk_opfamily.change_family();
+drop operator class fk_opfamily.int_ops using btree;
+drop operator family fk_opfamily.fam using btree;
+drop operator fk_opfamily.=#=(integer,bigint);
+drop schema fk_opfamily;
--
-- Now some cases with inheritance
-- Basic 2 table case: 1 column of matching types.
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 07d8921931..789d221361 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -692,6 +692,94 @@ ptest3) REFERENCES pktable(ptest1, ptest2));
CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY KEY(ptest1, ptest2), FOREIGN KEY(ptest4,
ptest3) REFERENCES pktable);
+-- Replacing a loose cross-type operator family member must invalidate the
+-- fast-path metadata. The FK continues using its stored equality operator,
+-- which need no longer be a member of the index family. Use the very same
+-- implementation for the replacement, keeping the family semantics unchanged.
+create schema fk_opfamily;
+set search_path = fk_opfamily, pg_catalog;
+create operator family fam using btree;
+create operator class int_ops for type integer using btree family fam as
+ operator 1 <(integer,integer), operator 2 <=(integer,integer),
+ operator 3 =(integer,integer), operator 4 >=(integer,integer),
+ operator 5 >(integer,integer), function 1 btint4cmp(integer,integer);
+alter operator family fam using btree add
+ operator 1 <(integer,bigint), operator 2 <=(integer,bigint),
+ operator 3 =(integer,bigint), operator 4 >=(integer,bigint),
+ operator 5 >(integer,bigint),
+ operator 1 <(bigint,integer), operator 2 <=(bigint,integer),
+ operator 3 =(bigint,integer), operator 4 >=(bigint,integer),
+ operator 5 >(bigint,integer),
+ operator 1 <(bigint,bigint), operator 2 <=(bigint,bigint),
+ operator 3 =(bigint,bigint), operator 4 >=(bigint,bigint),
+ operator 5 >(bigint,bigint),
+ function 1 (integer,bigint) btint48cmp(integer,bigint),
+ function 1 (bigint,integer) btint84cmp(bigint,integer),
+ function 1 (bigint,bigint) btint8cmp(bigint,bigint);
+create operator =#= (leftarg=integer, rightarg=bigint, function=int48eq);
+create table p(k integer);
+create unique index p_idx on p(k int_ops);
+create table warm(k bigint references p(k));
+create table cold(k bigint references p(k));
+insert into p values (1), (2);
+insert into warm values (1);
+select amvalidate(oid) from pg_opclass
+where opcnamespace = 'fk_opfamily'::regnamespace;
+select exists (select from pg_backend_memory_contexts
+ where name = 'RI fast-path finfo scratch') as metadata_cached;
+-- Change only pg_amop after warming the cache.
+begin;
+alter operator family fam using btree drop operator 3(integer,bigint);
+alter operator family fam using btree add operator 3 =#=(integer,bigint);
+commit;
+select amvalidate(oid) from pg_opclass
+where opcnamespace = 'fk_opfamily'::regnamespace;
+select exists (select from pg_backend_memory_contexts
+ where name = 'RI fast-path finfo scratch') as metadata_cached;
+insert into warm values (2);
+insert into warm values (99);
+-- This constraint has no cached fast-path metadata yet.
+insert into cold values (2);
+insert into cold values (99);
+select * from warm order by k;
+select * from cold order by k;
+-- Change the family between AFTER triggers. The RI trigger sorts first, so
+-- rows buffered before the DDL must also be checked through SPI at batch end.
+alter operator family fam using btree drop operator 3(integer,bigint);
+alter operator family fam using btree add operator 3 =(integer,bigint);
+create table batch(k bigint references p(k));
+create function change_family() returns trigger language plpgsql as $$
+begin
+ if new.k = 1 then
+ alter operator family fam using btree drop operator 3(integer,bigint);
+ alter operator family fam using btree add operator 3 =#=(integer,bigint);
+ end if;
+ return null;
+end
+$$;
+create trigger zzz_change_family after insert on batch
+ for each row execute function change_family();
+begin;
+insert into batch values (1), (2);
+select * from batch order by k;
+rollback;
+-- No later RI trigger is needed to notice that the index became unsuitable.
+begin;
+insert into batch values (1);
+select * from batch;
+rollback;
+-- Reject missing keys both after and before the DDL boundary.
+insert into batch values (1), (99);
+insert into batch values (99), (1);
+select * from batch;
+reset search_path;
+drop table fk_opfamily.warm, fk_opfamily.cold, fk_opfamily.batch, fk_opfamily.p;
+drop function fk_opfamily.change_family();
+drop operator class fk_opfamily.int_ops using btree;
+drop operator family fk_opfamily.fam using btree;
+drop operator fk_opfamily.=#=(integer,bigint);
+drop schema fk_opfamily;
+
--
-- Now some cases with inheritance
-- Basic 2 table case: 1 column of matching types.
base-commit: f4b511ae93a587982ac025c7a2512586fe87489d
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-15 09:00 Nikolay Samokhvalov <nik@postgres.ai>
parent: Nikolay Samokhvalov <nik@postgres.ai>
1 sibling, 0 replies; 16+ messages in thread
From: Nikolay Samokhvalov @ 2026-09-15 09:00 UTC (permalink / raw)
To: pgsql-hackers <pgsql-hackers@lists.postgresql.org>; +Cc: Amit Langote <amitlangote09@gmail.com>
For completeness, here's the reproducer I should have included. Run as a
superuser in a fresh database, with psql -X and ON_ERROR_STOP unset:
\set VERBOSITY sqlstate
create schema fk_opfamily;
set search_path = fk_opfamily, pg_catalog;
create operator family fam using btree;
create operator class int_ops for type integer using btree family fam as
operator 1 <(integer,integer), operator 2 <=(integer,integer),
operator 3 =(integer,integer), operator 4 >=(integer,integer),
operator 5 >(integer,integer), function 1 btint4cmp(integer,integer);
alter operator family fam using btree add
operator 1 <(integer,bigint), operator 2 <=(integer,bigint),
operator 3 =(integer,bigint), operator 4 >=(integer,bigint),
operator 5 >(integer,bigint),
operator 1 <(bigint,integer), operator 2 <=(bigint,integer),
operator 3 =(bigint,integer), operator 4 >=(bigint,integer),
operator 5 >(bigint,integer),
operator 1 <(bigint,bigint), operator 2 <=(bigint,bigint),
operator 3 =(bigint,bigint), operator 4 >=(bigint,bigint),
operator 5 >(bigint,bigint),
function 1 (integer,bigint) btint48cmp(integer,bigint),
function 1 (bigint,integer) btint84cmp(bigint,integer),
function 1 (bigint,bigint) btint8cmp(bigint,bigint);
create operator =#= (leftarg=integer, rightarg=bigint, function=int48eq);
create table p(k integer);
create unique index p_idx on p(k int_ops);
create table warm(k bigint references p(k));
create table cold(k bigint references p(k));
insert into p values (1), (2);
insert into warm values (1);
select amvalidate(oid) from pg_opclass
where opcnamespace = 'fk_opfamily'::regnamespace;
select exists (select from pg_backend_memory_contexts
where name = 'RI fast-path finfo scratch') as metadata_cached;
begin;
alter operator family fam using btree drop operator 3(integer,bigint);
alter operator family fam using btree add operator 3 =#=(integer,bigint);
commit;
select amvalidate(oid) from pg_opclass
where opcnamespace = 'fk_opfamily'::regnamespace;
select exists (select from pg_backend_memory_contexts
where name = 'RI fast-path finfo scratch') as metadata_cached;
insert into warm values (2);
insert into warm values (99);
insert into cold values (2);
insert into cold values (99);
select * from warm order by k;
select * from cold order by k;
reset search_path;
drop schema fk_opfamily cascade;
On unpatched master a625fc57 and PG19 f4b511ae, amvalidate returns t
before and after the DDL, but metadata_cached stays t and both cold
inserts fail with XX000. The warm table contains 1 and 2; cold is empty.
With the corresponding patches, metadata_cached goes from t to f.
The valid inserts succeed and both inserts of 99 fail with 23503; warm
contains 1 and 2, and cold contains 2.
Thanks,
Nik
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-16 03:12 Amit Langote <amitlangote09@gmail.com>
parent: Nikolay Samokhvalov <nik@postgres.ai>
1 sibling, 1 reply; 16+ messages in thread
From: Amit Langote @ 2026-09-16 03:12 UTC (permalink / raw)
To: Nikolay Samokhvalov <nik@postgres.ai>; +Cc: pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi Nik,
On Tue, Sep 15, 2026 at 11:14 AM Nikolay Samokhvalov <nik@postgres.ai> wrote:
>
> On Fri, Sep 11, 2026 at 2:25 AM Amit Langote
> <amitlangote09@gmail.com> wrote:
> > For 0001, SPI's FOR KEY SHARE also requires UPDATE privilege on at
> > least one column. I've used ExecCheckOneRelPerms() to cover that along
> > with column-level SELECT. The tests exercise both per-row and batched
> > checks, including rejection without UPDATE and acceptance with UPDATE
> > on an unrelated column.
>
> I kept testing with my AI harness and found another case on master and
> PG19. Replacing a loose cross-type equality member leaves the FK's stored
> operator unchanged. An uncached fast-path check then errors; warmed
> metadata is not invalidated by the pg_amop change. The replacement calls
> the same int48eq function, so the family semantics are unchanged and SPI
> continues to enforce the FK normally.
>
> Attached are standalone fixes for master a625fc57 and PG19 f4b511ae.
> They invalidate the metadata on pg_amop changes and use SPI unless the
> stored operator is still an equality member. On master, buffered rows
> also need the SPI fallback if the family changes between AFTER triggers.
>
> Both assertion builds pass the native foreign_key test, full regression
> and isolation suites, and the injection-point suites. The tests cover
> warmed and uncached metadata and missing keys on both sides of the DDL.
Thanks for the report and the patch.
I've added an open item:
RI fastpath misses pg_amop updates
Commit: 2da86c1ef9b
Owner: Amit Langote
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-16 15:40 Melanie Plageman <melanieplageman@gmail.com>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Melanie Plageman @ 2026-09-16 15:40 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Nikolay Samokhvalov <nik@postgres.ai>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
On Tue, Sep 15, 2026 at 11:12 PM Amit Langote <amitlangote09@gmail.com> wrote:
>
> Thanks for the report and the patch.
>
> I've added an open item:
>
> RI fastpath misses pg_amop updates
> Commit: 2da86c1ef9b
> Owner: Amit Langote
Will the fix be something like what Nikolay proposed or something
different that solves both the issue in the fast path and the
pre-existing issue with the cached cast functions?
- Melanie
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-16 23:00 Amit Langote <amitlangote09@gmail.com>
parent: Melanie Plageman <melanieplageman@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Amit Langote @ 2026-09-16 23:00 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; +Cc: Nikolay Samokhvalov <nik@postgres.ai>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Sep 17, 2026 at 0:41 Melanie Plageman <melanieplageman@gmail.com>
wrote:
> On Tue, Sep 15, 2026 at 11:12 PM Amit Langote <amitlangote09@gmail.com>
> wrote:
> >
> > Thanks for the report and the patch.
> >
> > I've added an open item:
> >
> > RI fastpath misses pg_amop updates
> > Commit: 2da86c1ef9b
> > Owner: Amit Langote
>
> Will the fix be something like what Nikolay proposed or something
> different that solves both the issue in the fast path and the
> pre-existing issue with the cached cast functions?
There’s some overlap but I’m planning to fix the cast issue separately,
that is, not combine it with the fix for this open item which I’d like to
fix by beta4 freeze.
- Amit
>
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-18 09:21 Amit Langote <amitlangote09@gmail.com>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Amit Langote @ 2026-09-18 09:21 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; +Cc: Nikolay Samokhvalov <nik@postgres.ai>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi,
On Thu, Sep 17, 2026 at 8:00 AM Amit Langote <amitlangote09@gmail.com> wrote:
>
> On Thu, Sep 17, 2026 at 0:41 Melanie Plageman <melanieplageman@gmail.com> wrote:
>>
>> On Tue, Sep 15, 2026 at 11:12 PM Amit Langote <amitlangote09@gmail.com> wrote:
>> >
>> > Thanks for the report and the patch.
>> >
>> > I've added an open item:
>> >
>> > RI fastpath misses pg_amop updates
>> > Commit: 2da86c1ef9b
>> > Owner: Amit Langote
>>
>> Will the fix be something like what Nikolay proposed or something
>> different that solves both the issue in the fast path and the
>> pre-existing issue with the cached cast functions?
>
>
> There’s some overlap but I’m planning to fix the cast issue separately, that is, not combine it with the fix for this open item which I’d like to fix by beta4 freeze.
Attached are the patches for fixing the two open items, which I plan
to push tomorrow.
Patch 0001 needs to fix a batching specific function (or it won't
compile) so there are separate versions for master and 19.
For 0002, I am attaching only the patch that fixes the per-row fast
path, which has the same shape in both master and 19. Nik had posted
one patch to fix both paths, but I decided to break it into one patch
that fixes the per-row path (which applies to both master and 19) and
another that is only needed in master for fixing the batching path for
the same opfamily change errors. I'm adding the latter to the list of
patches I now have locally for fixing the various batching path issues
I am aware of.
--
Thanks, Amit Langote
Attachments:
[application/octet-stream] master-0001-Fix-RI-fast-path-permission-checks.patch (12.1K, ../../CA+HiwqFAxgv5NNSyYPu+_nPMOUXgchERh9B_WfZnsGe-j8vysQ@mail.gmail.com/2-master-0001-Fix-RI-fast-path-permission-checks.patch)
download | inline diff:
From 038dbd373d138d6bcd5d275868772274361dcfc9 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Fri, 18 Sep 2026 17:31:49 +0900
Subject: [PATCH 1/2] Fix RI fast-path permission checks
The fast path required table-level SELECT on the referenced table,
rejecting checks that the SPI path allows with column-level grants.
It also omitted the UPDATE privilege required by FOR KEY SHARE.
When table privileges do not suffice, use ExecCheckOneRelPerms() with
the referenced key columns as selectedCols and an empty updatedCols.
This accepts SELECT on all referenced columns and UPDATE on any column,
the same privileges the executor would require for the SELECT ... FOR
KEY SHARE the SPI path runs. Keep the table-privilege check as a
shortcut that avoids constructing a column bitmap in the usual case.
Add missing regression test coverage for the fixed cases.
Reported-by: Nikolay Samokhvalov <nik@postgres.ai>
Author: Nikolay Samokhvalov <nik@postgres.ai>
Co-authored-by: Amit Langote <amitlangote09@gmail.com>
Discussion: https://www.postgr.es/m/CAM527d9BgPjeOOYmbCBTd57R145qHCk-dzw9qNq%2BnOrDq1j__A%40mail.gmail.com
Backpatch-through: 19
---
src/backend/utils/adt/ri_triggers.c | 47 +++++++++++++-----
src/test/regress/expected/foreign_key.out | 55 ++++++++++++++++++++-
src/test/regress/sql/foreign_key.sql | 59 ++++++++++++++++++++++-
3 files changed, 146 insertions(+), 15 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 43f40172b67..3196d971939 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -395,7 +395,8 @@ static bool ri_LockPKTuple(Relation pk_rel, TupleTableSlot *slot, Snapshot snap,
static bool ri_fastpath_is_applicable(const RI_ConstraintInfo *riinfo);
static bool ri_check_fastpath_index(RI_ConstraintInfo *riinfo,
Relation pk_rel, Relation idx_rel);
-static void ri_CheckPermissions(Relation query_rel);
+static void ri_CheckPermissions(const RI_ConstraintInfo *riinfo,
+ Relation query_rel);
static bool recheck_matched_pk_tuple(Relation idxrel, ScanKeyData *skeys,
int nkeys, TupleTableSlot *new_slot);
static void build_index_scankeys(const RI_ConstraintInfo *riinfo,
@@ -2939,7 +2940,7 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo,
saved_sec_context |
SECURITY_LOCAL_USERID_CHANGE |
SECURITY_NOFORCE_RLS);
- ri_CheckPermissions(pk_rel);
+ ri_CheckPermissions(riinfo, pk_rel);
/*
* Begin the scan under the switched user id, so that any access method
@@ -3106,7 +3107,7 @@ ri_FastPathBatchFlush(RI_FastPathEntry *fpentry, Relation fk_rel,
* albeit checked once per flush rather than once per row, like in
* ri_FastPathCheck().
*/
- ri_CheckPermissions(pk_rel);
+ ri_CheckPermissions(riinfo, pk_rel);
/*
* Begin the scan under the switched user id, so that any access method
@@ -3572,13 +3573,16 @@ ri_check_fastpath_index(RI_ConstraintInfo *riinfo,
/*
* ri_CheckPermissions
- * Check that the current user has permissions to look into the schema of
- * and SELECT from 'query_rel'
+ * Check permissions for the SELECT ... FOR KEY SHARE used by the SPI
+ * path, as the referenced table's owner.
*/
static void
-ri_CheckPermissions(Relation query_rel)
+ri_CheckPermissions(const RI_ConstraintInfo *riinfo, Relation query_rel)
{
AclResult aclresult;
+ AclMode requiredPerms = ACL_SELECT | ACL_SELECT_FOR_UPDATE;
+ RTEPermissionInfo *perminfo;
+ bool result;
/* USAGE on schema. */
aclresult = object_aclcheck(NamespaceRelationId,
@@ -3588,11 +3592,32 @@ ri_CheckPermissions(Relation query_rel)
aclcheck_error(aclresult, OBJECT_SCHEMA,
get_namespace_name(RelationGetNamespace(query_rel)));
- /* SELECT on relation. */
- aclresult = pg_class_aclcheck(RelationGetRelid(query_rel), GetUserId(),
- ACL_SELECT);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, OBJECT_TABLE,
+ /* Avoid building the column bitmap when table privileges suffice. */
+ if (pg_class_aclmask(RelationGetRelid(query_rel), GetUserId(),
+ requiredPerms, ACLMASK_ALL) == requiredPerms)
+ return;
+
+ /*
+ * SELECT is needed only on the referenced key columns. FOR KEY SHARE
+ * also needs UPDATE privilege, which may be granted on any column. Use
+ * the executor's checks for both, leaving updatedCols empty as the SPI
+ * query does.
+ */
+ perminfo = makeNode(RTEPermissionInfo);
+ perminfo->relid = RelationGetRelid(query_rel);
+ perminfo->requiredPerms = requiredPerms;
+ for (int i = 0; i < riinfo->nkeys; i++)
+ {
+ int attno = riinfo->pk_attnums[i] - FirstLowInvalidHeapAttributeNumber;
+
+ perminfo->selectedCols = bms_add_member(perminfo->selectedCols, attno);
+ }
+
+ result = ExecCheckOneRelPerms(perminfo);
+ bms_free(perminfo->selectedCols);
+ pfree(perminfo);
+ if (!result)
+ aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLE,
RelationGetRelationName(query_rel));
}
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 8d81240f1c6..c54a9895b55 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -402,17 +402,68 @@ CREATE TABLE FKTABLE ( ftest1 int REFERENCES PKTABLE, ftest2 int );
INSERT INTO PKTABLE VALUES (1, 'Test1');
INSERT INTO PKTABLE VALUES (2, 'Test2');
INSERT INTO PKTABLE VALUES (3, 'Test3');
--- Grant usage on PKTABLE to user regress_foreign_key_user
+-- Grant SELECT on PKTABLE to user regress_foreign_key_user
CREATE USER regress_foreign_key_user NOLOGIN;
GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
ALTER TABLE PKTABLE OWNER to regress_foreign_key_user;
-- Inserting into FKTABLE should work
INSERT INTO FKTABLE VALUES (3, 5);
--- Revoke usage on PKTABLE from user regress_foreign_key_user
+-- Revoke SELECT on PKTABLE from user regress_foreign_key_user
REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
-- Inserting into FKTABLE should fail
INSERT INTO FKTABLE VALUES (2, 6);
ERROR: permission denied for table pktable
+-- SELECT on the referenced key column is enough, without SELECT on ptest2.
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+-- SELECT on an unrelated column does not suffice.
+REVOKE SELECT (ptest1) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+ERROR: permission denied for table pktable
+REVOKE SELECT (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+-- FOR KEY SHARE also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+ERROR: permission denied for table pktable
+-- UPDATE on any column suffices, even one that the check does not read.
+GRANT UPDATE (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+-- Table-level SELECT can be combined with column-level UPDATE.
+GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+REVOKE UPDATE (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+ERROR: permission denied for table pktable
+DROP TABLE FKTABLE;
+DROP TABLE PKTABLE;
+-- Check all referenced columns, including when index and FK order differ.
+CREATE TABLE PKTABLE ( ptest0 text, ptest1 int, ptest2 int,
+ PRIMARY KEY (ptest2, ptest1) );
+CREATE TABLE FKTABLE ( ftest1 int, ftest2 int );
+INSERT INTO PKTABLE VALUES ('unused', 1, 2);
+INSERT INTO FKTABLE VALUES (1, 2);
+ALTER TABLE FKTABLE ADD CONSTRAINT fktable_fk
+ FOREIGN KEY (ftest1, ftest2) REFERENCES PKTABLE (ptest1, ptest2) NOT VALID;
+ALTER TABLE PKTABLE OWNER TO regress_foreign_key_user;
+ALTER TABLE FKTABLE OWNER TO regress_foreign_key_user;
+REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+-- Lack of SELECT on FKTABLE forces validation to check each row.
+REVOKE SELECT ON FKTABLE FROM regress_foreign_key_user;
+SET ROLE regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+ERROR: permission denied for table pktable
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+-- Per-row validation also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+ERROR: permission denied for table pktable
+-- UPDATE on the unrelated column is enough.
+GRANT UPDATE (ptest0) ON PKTABLE TO regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk;
+RESET ROLE;
DROP TABLE FKTABLE;
DROP TABLE PKTABLE;
DROP USER regress_foreign_key_user;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 184d9efdc97..c013d1f8834 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -286,7 +286,7 @@ INSERT INTO PKTABLE VALUES (1, 'Test1');
INSERT INTO PKTABLE VALUES (2, 'Test2');
INSERT INTO PKTABLE VALUES (3, 'Test3');
--- Grant usage on PKTABLE to user regress_foreign_key_user
+-- Grant SELECT on PKTABLE to user regress_foreign_key_user
CREATE USER regress_foreign_key_user NOLOGIN;
GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
@@ -295,12 +295,67 @@ ALTER TABLE PKTABLE OWNER to regress_foreign_key_user;
-- Inserting into FKTABLE should work
INSERT INTO FKTABLE VALUES (3, 5);
--- Revoke usage on PKTABLE from user regress_foreign_key_user
+-- Revoke SELECT on PKTABLE from user regress_foreign_key_user
REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
-- Inserting into FKTABLE should fail
INSERT INTO FKTABLE VALUES (2, 6);
+-- SELECT on the referenced key column is enough, without SELECT on ptest2.
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+
+-- SELECT on an unrelated column does not suffice.
+REVOKE SELECT (ptest1) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+REVOKE SELECT (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+
+-- FOR KEY SHARE also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+
+-- UPDATE on any column suffices, even one that the check does not read.
+GRANT UPDATE (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+
+-- Table-level SELECT can be combined with column-level UPDATE.
+GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+REVOKE UPDATE (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+
+DROP TABLE FKTABLE;
+DROP TABLE PKTABLE;
+
+-- Check all referenced columns, including when index and FK order differ.
+CREATE TABLE PKTABLE ( ptest0 text, ptest1 int, ptest2 int,
+ PRIMARY KEY (ptest2, ptest1) );
+CREATE TABLE FKTABLE ( ftest1 int, ftest2 int );
+INSERT INTO PKTABLE VALUES ('unused', 1, 2);
+INSERT INTO FKTABLE VALUES (1, 2);
+ALTER TABLE FKTABLE ADD CONSTRAINT fktable_fk
+ FOREIGN KEY (ftest1, ftest2) REFERENCES PKTABLE (ptest1, ptest2) NOT VALID;
+ALTER TABLE PKTABLE OWNER TO regress_foreign_key_user;
+ALTER TABLE FKTABLE OWNER TO regress_foreign_key_user;
+REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+
+-- Lack of SELECT on FKTABLE forces validation to check each row.
+REVOKE SELECT ON FKTABLE FROM regress_foreign_key_user;
+SET ROLE regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+
+-- Per-row validation also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+-- UPDATE on the unrelated column is enough.
+GRANT UPDATE (ptest0) ON PKTABLE TO regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk;
+RESET ROLE;
+
DROP TABLE FKTABLE;
DROP TABLE PKTABLE;
--
2.47.3
[application/octet-stream] 0002-Invalidate-RI-fast-path-metadata-on-operator-family-.patch (11.3K, ../../CA+HiwqFAxgv5NNSyYPu+_nPMOUXgchERh9B_WfZnsGe-j8vysQ@mail.gmail.com/3-0002-Invalidate-RI-fast-path-metadata-on-operator-family-.patch)
download | inline diff:
From 27bb608e25b18148dc3f55b08a62fc489c09158d Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Fri, 18 Sep 2026 17:55:30 +0900
Subject: [PATCH 2/2] Invalidate RI fast-path metadata on operator family
changes
The RI fast path checks a foreign key by probing the referenced
unique index directly, using the equality operator recorded for
the constraint. Whether the fast path can be used is decided once
and cached in RI_ConstraintInfo, and that cache is invalidated on
pg_constraint changes but not on pg_amop. So after an ALTER
OPERATOR FAMILY drops the recorded operator and adds another in its
place, the cached decision is stale, and the next fast-path check
probes the index with an operator no longer in the opfamily and
errors out with "operator XXX is not a member of opfamily XXX".
The SPI path is unaffected, because the planner just stops matching
the index.
To fix, register an AMOPOPID syscache callback to flush the RI
cache on pg_amop changes, and have ri_check_fastpath_index()
recheck that the recorded operator is still the equality member of
the index's opfamily, falling back to SPI when it is not.
Add regression test coverage.
Reported-by: Nikolay Samokhvalov <nik@postgres.ai>
Author: Nikolay Samokhvalov <nik@postgres.ai>
Discussion: https://www.postgr.es/m/CAM527d9PzFzagr67N0%3DEx2ng1p5HzrcAszy3j5OoZKHXQMARXA%40mail.gmail.com
Backpatch-through: 19
---
src/backend/utils/adt/ri_triggers.c | 37 ++++++++-
src/test/regress/expected/foreign_key.out | 92 +++++++++++++++++++++++
src/test/regress/sql/foreign_key.sql | 58 ++++++++++++++
3 files changed, 185 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 3196d971939..bd428a4d740 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -2602,7 +2602,7 @@ get_ri_constraint_root(Oid constrOid)
}
/*
- * Callback for pg_constraint inval events
+ * Callback for pg_constraint and pg_amop inval events
*
* While most syscache callbacks just flush all their entries, pg_constraint
* gets enough update traffic that it's probably worth being smarter.
@@ -2627,6 +2627,10 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid,
Assert(ri_constraint_cache != NULL);
+ /* pg_amop changes can affect any constraint's fast-path metadata. */
+ if (cacheid == AMOPOPID)
+ hashvalue = 0;
+
/*
* If the list of currently valid entries gets excessively large, we mark
* them all invalid so we can empty the list. This arrangement avoids
@@ -3567,6 +3571,32 @@ ri_check_fastpath_index(RI_ConstraintInfo *riinfo,
}
}
+ /*
+ * The equality operator stored in pg_constraint must still be an equality
+ * member of the index opfamily. A loose cross-type member can be
+ * replaced without changing the constraint itself; leave that case to
+ * SPI, which continues to use the operator recorded by the constraint.
+ */
+ for (int i = 0; i < riinfo->nkeys; i++)
+ {
+ int idx_col;
+
+ for (idx_col = 0; idx_col < idx_rel->rd_index->indnkeyatts; idx_col++)
+ {
+ if (idx_rel->rd_index->indkey.values[idx_col] ==
+ riinfo->pk_attnums[i])
+ break;
+ }
+ Assert(idx_col < idx_rel->rd_index->indnkeyatts);
+
+ if (get_op_opfamily_strategy(riinfo->pf_eq_oprs[i],
+ idx_rel->rd_opfamily[idx_col]) != BTEqualStrategyNumber)
+ {
+ riinfo->fastpath_state = RI_FASTPATH_UNUSABLE;
+ return false;
+ }
+ }
+
riinfo->fastpath_state = RI_FASTPATH_USABLE;
return true;
}
@@ -4047,10 +4077,13 @@ ri_InitHashTables(void)
RI_INIT_CONSTRAINTHASHSIZE,
&ctl, HASH_ELEM | HASH_BLOBS);
- /* Arrange to flush cache on pg_constraint changes */
+ /* Arrange to flush cache on pg_constraint or pg_amop changes */
CacheRegisterSyscacheCallback(CONSTROID,
InvalidateConstraintCacheCallBack,
(Datum) 0);
+ CacheRegisterSyscacheCallback(AMOPOPID,
+ InvalidateConstraintCacheCallBack,
+ (Datum) 0);
ctl.keysize = sizeof(RI_QueryKey);
ctl.entrysize = sizeof(RI_QueryHashEntry);
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index c54a9895b55..93b401ce221 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1081,6 +1081,98 @@ CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY
ptest3) REFERENCES pktable);
ERROR: foreign key constraint "pktable_ptest4_ptest3_fkey" cannot be implemented
DETAIL: Key columns "ptest4" of the referencing table and "ptest1" of the referenced table are of incompatible types: inet and integer.
+-- Replacing a loose cross-type operator family member must invalidate the
+-- fast-path metadata. The FK continues using its stored equality operator,
+-- which need no longer be a member of the index family. Use the very same
+-- implementation for the replacement, keeping the family semantics unchanged.
+create schema fk_opfamily;
+set search_path = fk_opfamily, pg_catalog;
+create operator family fam using btree;
+create operator class int_ops for type integer using btree family fam as
+ operator 1 <(integer,integer), operator 2 <=(integer,integer),
+ operator 3 =(integer,integer), operator 4 >=(integer,integer),
+ operator 5 >(integer,integer), function 1 btint4cmp(integer,integer);
+alter operator family fam using btree add
+ operator 1 <(integer,bigint), operator 2 <=(integer,bigint),
+ operator 3 =(integer,bigint), operator 4 >=(integer,bigint),
+ operator 5 >(integer,bigint),
+ operator 1 <(bigint,integer), operator 2 <=(bigint,integer),
+ operator 3 =(bigint,integer), operator 4 >=(bigint,integer),
+ operator 5 >(bigint,integer),
+ operator 1 <(bigint,bigint), operator 2 <=(bigint,bigint),
+ operator 3 =(bigint,bigint), operator 4 >=(bigint,bigint),
+ operator 5 >(bigint,bigint),
+ function 1 (integer,bigint) btint48cmp(integer,bigint),
+ function 1 (bigint,integer) btint84cmp(bigint,integer),
+ function 1 (bigint,bigint) btint8cmp(bigint,bigint);
+create operator =#= (leftarg=integer, rightarg=bigint, function=int48eq);
+create table p(k integer);
+create unique index p_idx on p(k int_ops);
+create table warm(k bigint references p(k));
+create table cold(k bigint references p(k));
+insert into p values (1), (2);
+insert into warm values (1);
+select amvalidate(oid) from pg_opclass
+where opcnamespace = 'fk_opfamily'::regnamespace;
+ amvalidate
+------------
+ t
+(1 row)
+
+select exists (select from pg_backend_memory_contexts
+ where name = 'RI fast-path finfo scratch') as metadata_cached;
+ metadata_cached
+-----------------
+ t
+(1 row)
+
+-- Change only pg_amop after warming the cache.
+begin;
+alter operator family fam using btree drop operator 3(integer,bigint);
+alter operator family fam using btree add operator 3 =#=(integer,bigint);
+commit;
+select amvalidate(oid) from pg_opclass
+where opcnamespace = 'fk_opfamily'::regnamespace;
+ amvalidate
+------------
+ t
+(1 row)
+
+select exists (select from pg_backend_memory_contexts
+ where name = 'RI fast-path finfo scratch') as metadata_cached;
+ metadata_cached
+-----------------
+ f
+(1 row)
+
+insert into warm values (2);
+insert into warm values (99);
+ERROR: insert or update on table "warm" violates foreign key constraint "warm_k_fkey"
+DETAIL: Key (k)=(99) is not present in table "p".
+-- This constraint has no cached fast-path metadata yet.
+insert into cold values (2);
+insert into cold values (99);
+ERROR: insert or update on table "cold" violates foreign key constraint "cold_k_fkey"
+DETAIL: Key (k)=(99) is not present in table "p".
+select * from warm order by k;
+ k
+---
+ 1
+ 2
+(2 rows)
+
+select * from cold order by k;
+ k
+---
+ 2
+(1 row)
+
+reset search_path;
+drop table fk_opfamily.warm, fk_opfamily.cold, fk_opfamily.p;
+drop operator class fk_opfamily.int_ops using btree;
+drop operator family fk_opfamily.fam using btree;
+drop operator fk_opfamily.=#=(integer,bigint);
+drop schema fk_opfamily;
--
-- Now some cases with inheritance
-- Basic 2 table case: 1 column of matching types.
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index c013d1f8834..9415a3f938a 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -747,6 +747,64 @@ ptest3) REFERENCES pktable(ptest1, ptest2));
CREATE TABLE PKTABLE (ptest1 int, ptest2 inet, ptest3 int, ptest4 inet, PRIMARY KEY(ptest1, ptest2), FOREIGN KEY(ptest4,
ptest3) REFERENCES pktable);
+-- Replacing a loose cross-type operator family member must invalidate the
+-- fast-path metadata. The FK continues using its stored equality operator,
+-- which need no longer be a member of the index family. Use the very same
+-- implementation for the replacement, keeping the family semantics unchanged.
+create schema fk_opfamily;
+set search_path = fk_opfamily, pg_catalog;
+create operator family fam using btree;
+create operator class int_ops for type integer using btree family fam as
+ operator 1 <(integer,integer), operator 2 <=(integer,integer),
+ operator 3 =(integer,integer), operator 4 >=(integer,integer),
+ operator 5 >(integer,integer), function 1 btint4cmp(integer,integer);
+alter operator family fam using btree add
+ operator 1 <(integer,bigint), operator 2 <=(integer,bigint),
+ operator 3 =(integer,bigint), operator 4 >=(integer,bigint),
+ operator 5 >(integer,bigint),
+ operator 1 <(bigint,integer), operator 2 <=(bigint,integer),
+ operator 3 =(bigint,integer), operator 4 >=(bigint,integer),
+ operator 5 >(bigint,integer),
+ operator 1 <(bigint,bigint), operator 2 <=(bigint,bigint),
+ operator 3 =(bigint,bigint), operator 4 >=(bigint,bigint),
+ operator 5 >(bigint,bigint),
+ function 1 (integer,bigint) btint48cmp(integer,bigint),
+ function 1 (bigint,integer) btint84cmp(bigint,integer),
+ function 1 (bigint,bigint) btint8cmp(bigint,bigint);
+create operator =#= (leftarg=integer, rightarg=bigint, function=int48eq);
+create table p(k integer);
+create unique index p_idx on p(k int_ops);
+create table warm(k bigint references p(k));
+create table cold(k bigint references p(k));
+insert into p values (1), (2);
+insert into warm values (1);
+select amvalidate(oid) from pg_opclass
+where opcnamespace = 'fk_opfamily'::regnamespace;
+select exists (select from pg_backend_memory_contexts
+ where name = 'RI fast-path finfo scratch') as metadata_cached;
+-- Change only pg_amop after warming the cache.
+begin;
+alter operator family fam using btree drop operator 3(integer,bigint);
+alter operator family fam using btree add operator 3 =#=(integer,bigint);
+commit;
+select amvalidate(oid) from pg_opclass
+where opcnamespace = 'fk_opfamily'::regnamespace;
+select exists (select from pg_backend_memory_contexts
+ where name = 'RI fast-path finfo scratch') as metadata_cached;
+insert into warm values (2);
+insert into warm values (99);
+-- This constraint has no cached fast-path metadata yet.
+insert into cold values (2);
+insert into cold values (99);
+select * from warm order by k;
+select * from cold order by k;
+reset search_path;
+drop table fk_opfamily.warm, fk_opfamily.cold, fk_opfamily.p;
+drop operator class fk_opfamily.int_ops using btree;
+drop operator family fk_opfamily.fam using btree;
+drop operator fk_opfamily.=#=(integer,bigint);
+drop schema fk_opfamily;
+
--
-- Now some cases with inheritance
-- Basic 2 table case: 1 column of matching types.
--
2.47.3
[application/octet-stream] 19-0001-Fix-RI-fast-path-permission-checks.patch (11.8K, ../../CA+HiwqFAxgv5NNSyYPu+_nPMOUXgchERh9B_WfZnsGe-j8vysQ@mail.gmail.com/4-19-0001-Fix-RI-fast-path-permission-checks.patch)
download | inline diff:
From c565dc72dbeaeffb7951d1a94436ebbf244016f3 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Fri, 18 Sep 2026 17:32:44 +0900
Subject: [PATCH 1/2] Fix RI fast-path permission checks
The fast path required table-level SELECT on the referenced table,
rejecting checks that the SPI path allows with column-level grants.
It also omitted the UPDATE privilege required by FOR KEY SHARE.
When table privileges do not suffice, use ExecCheckOneRelPerms() with
the referenced key columns as selectedCols and an empty updatedCols.
This accepts SELECT on all referenced columns and UPDATE on any column,
the same privileges the executor would require for the SELECT ... FOR
KEY SHARE the SPI path runs. Keep the table-privilege check as a
shortcut that avoids constructing a column bitmap in the usual case.
Add missing regression test coverage for the fixed cases.
Reported-by: Nikolay Samokhvalov <nik@postgres.ai>
Author: Nikolay Samokhvalov <nik@postgres.ai>
Co-authored-by: Amit Langote <amitlangote09@gmail.com>
Discussion: https://www.postgr.es/m/CAM527d9BgPjeOOYmbCBTd57R145qHCk-dzw9qNq%2BnOrDq1j__A%40mail.gmail.com
Backpatch-through: 19
---
src/backend/utils/adt/ri_triggers.c | 45 +++++++++++++----
src/test/regress/expected/foreign_key.out | 55 ++++++++++++++++++++-
src/test/regress/sql/foreign_key.sql | 59 ++++++++++++++++++++++-
3 files changed, 145 insertions(+), 14 deletions(-)
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 5d55a2006ee..c8e3df59bf7 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -301,7 +301,8 @@ static bool ri_LockPKTuple(Relation pk_rel, TupleTableSlot *slot, Snapshot snap,
static bool ri_fastpath_is_applicable(const RI_ConstraintInfo *riinfo);
static bool ri_check_fastpath_index(RI_ConstraintInfo *riinfo,
Relation pk_rel, Relation idx_rel);
-static void ri_CheckPermissions(Relation query_rel);
+static void ri_CheckPermissions(const RI_ConstraintInfo *riinfo,
+ Relation query_rel);
static bool recheck_matched_pk_tuple(Relation idxrel, ScanKeyData *skeys,
int nkeys, TupleTableSlot *new_slot);
static void build_index_scankeys(const RI_ConstraintInfo *riinfo,
@@ -2819,7 +2820,7 @@ ri_FastPathCheck(RI_ConstraintInfo *riinfo,
saved_sec_context |
SECURITY_LOCAL_USERID_CHANGE |
SECURITY_NOFORCE_RLS);
- ri_CheckPermissions(pk_rel);
+ ri_CheckPermissions(riinfo, pk_rel);
/*
* Begin the scan under the switched user id, so that any access method
@@ -3053,13 +3054,16 @@ ri_check_fastpath_index(RI_ConstraintInfo *riinfo,
/*
* ri_CheckPermissions
- * Check that the current user has permissions to look into the schema of
- * and SELECT from 'query_rel'
+ * Check permissions for the SELECT ... FOR KEY SHARE used by the SPI
+ * path, as the referenced table's owner.
*/
static void
-ri_CheckPermissions(Relation query_rel)
+ri_CheckPermissions(const RI_ConstraintInfo *riinfo, Relation query_rel)
{
AclResult aclresult;
+ AclMode requiredPerms = ACL_SELECT | ACL_SELECT_FOR_UPDATE;
+ RTEPermissionInfo *perminfo;
+ bool result;
/* USAGE on schema. */
aclresult = object_aclcheck(NamespaceRelationId,
@@ -3069,11 +3073,32 @@ ri_CheckPermissions(Relation query_rel)
aclcheck_error(aclresult, OBJECT_SCHEMA,
get_namespace_name(RelationGetNamespace(query_rel)));
- /* SELECT on relation. */
- aclresult = pg_class_aclcheck(RelationGetRelid(query_rel), GetUserId(),
- ACL_SELECT);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, OBJECT_TABLE,
+ /* Avoid building the column bitmap when table privileges suffice. */
+ if (pg_class_aclmask(RelationGetRelid(query_rel), GetUserId(),
+ requiredPerms, ACLMASK_ALL) == requiredPerms)
+ return;
+
+ /*
+ * SELECT is needed only on the referenced key columns. FOR KEY SHARE
+ * also needs UPDATE privilege, which may be granted on any column. Use
+ * the executor's checks for both, leaving updatedCols empty as the SPI
+ * query does.
+ */
+ perminfo = makeNode(RTEPermissionInfo);
+ perminfo->relid = RelationGetRelid(query_rel);
+ perminfo->requiredPerms = requiredPerms;
+ for (int i = 0; i < riinfo->nkeys; i++)
+ {
+ int attno = riinfo->pk_attnums[i] - FirstLowInvalidHeapAttributeNumber;
+
+ perminfo->selectedCols = bms_add_member(perminfo->selectedCols, attno);
+ }
+
+ result = ExecCheckOneRelPerms(perminfo);
+ bms_free(perminfo->selectedCols);
+ pfree(perminfo);
+ if (!result)
+ aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_TABLE,
RelationGetRelationName(query_rel));
}
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 9cea669b6fd..d0c93ac8744 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -402,17 +402,68 @@ CREATE TABLE FKTABLE ( ftest1 int REFERENCES PKTABLE, ftest2 int );
INSERT INTO PKTABLE VALUES (1, 'Test1');
INSERT INTO PKTABLE VALUES (2, 'Test2');
INSERT INTO PKTABLE VALUES (3, 'Test3');
--- Grant usage on PKTABLE to user regress_foreign_key_user
+-- Grant SELECT on PKTABLE to user regress_foreign_key_user
CREATE USER regress_foreign_key_user NOLOGIN;
GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
ALTER TABLE PKTABLE OWNER to regress_foreign_key_user;
-- Inserting into FKTABLE should work
INSERT INTO FKTABLE VALUES (3, 5);
--- Revoke usage on PKTABLE from user regress_foreign_key_user
+-- Revoke SELECT on PKTABLE from user regress_foreign_key_user
REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
-- Inserting into FKTABLE should fail
INSERT INTO FKTABLE VALUES (2, 6);
ERROR: permission denied for table pktable
+-- SELECT on the referenced key column is enough, without SELECT on ptest2.
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+-- SELECT on an unrelated column does not suffice.
+REVOKE SELECT (ptest1) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+ERROR: permission denied for table pktable
+REVOKE SELECT (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+-- FOR KEY SHARE also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+ERROR: permission denied for table pktable
+-- UPDATE on any column suffices, even one that the check does not read.
+GRANT UPDATE (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+-- Table-level SELECT can be combined with column-level UPDATE.
+GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+REVOKE UPDATE (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+ERROR: permission denied for table pktable
+DROP TABLE FKTABLE;
+DROP TABLE PKTABLE;
+-- Check all referenced columns, including when index and FK order differ.
+CREATE TABLE PKTABLE ( ptest0 text, ptest1 int, ptest2 int,
+ PRIMARY KEY (ptest2, ptest1) );
+CREATE TABLE FKTABLE ( ftest1 int, ftest2 int );
+INSERT INTO PKTABLE VALUES ('unused', 1, 2);
+INSERT INTO FKTABLE VALUES (1, 2);
+ALTER TABLE FKTABLE ADD CONSTRAINT fktable_fk
+ FOREIGN KEY (ftest1, ftest2) REFERENCES PKTABLE (ptest1, ptest2) NOT VALID;
+ALTER TABLE PKTABLE OWNER TO regress_foreign_key_user;
+ALTER TABLE FKTABLE OWNER TO regress_foreign_key_user;
+REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+-- Lack of SELECT on FKTABLE forces validation to check each row.
+REVOKE SELECT ON FKTABLE FROM regress_foreign_key_user;
+SET ROLE regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+ERROR: permission denied for table pktable
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+-- Per-row validation also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+ERROR: permission denied for table pktable
+-- UPDATE on the unrelated column is enough.
+GRANT UPDATE (ptest0) ON PKTABLE TO regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk;
+RESET ROLE;
DROP TABLE FKTABLE;
DROP TABLE PKTABLE;
DROP USER regress_foreign_key_user;
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 07d89219318..5e27b9b2d24 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -286,7 +286,7 @@ INSERT INTO PKTABLE VALUES (1, 'Test1');
INSERT INTO PKTABLE VALUES (2, 'Test2');
INSERT INTO PKTABLE VALUES (3, 'Test3');
--- Grant usage on PKTABLE to user regress_foreign_key_user
+-- Grant SELECT on PKTABLE to user regress_foreign_key_user
CREATE USER regress_foreign_key_user NOLOGIN;
GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
@@ -295,12 +295,67 @@ ALTER TABLE PKTABLE OWNER to regress_foreign_key_user;
-- Inserting into FKTABLE should work
INSERT INTO FKTABLE VALUES (3, 5);
--- Revoke usage on PKTABLE from user regress_foreign_key_user
+-- Revoke SELECT on PKTABLE from user regress_foreign_key_user
REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
-- Inserting into FKTABLE should fail
INSERT INTO FKTABLE VALUES (2, 6);
+-- SELECT on the referenced key column is enough, without SELECT on ptest2.
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+
+-- SELECT on an unrelated column does not suffice.
+REVOKE SELECT (ptest1) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+REVOKE SELECT (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+
+-- FOR KEY SHARE also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+
+-- UPDATE on any column suffices, even one that the check does not read.
+GRANT UPDATE (ptest2) ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+
+-- Table-level SELECT can be combined with column-level UPDATE.
+GRANT SELECT ON PKTABLE TO regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6);
+REVOKE UPDATE (ptest2) ON PKTABLE FROM regress_foreign_key_user;
+INSERT INTO FKTABLE VALUES (2, 6); -- fails
+
+DROP TABLE FKTABLE;
+DROP TABLE PKTABLE;
+
+-- Check all referenced columns, including when index and FK order differ.
+CREATE TABLE PKTABLE ( ptest0 text, ptest1 int, ptest2 int,
+ PRIMARY KEY (ptest2, ptest1) );
+CREATE TABLE FKTABLE ( ftest1 int, ftest2 int );
+INSERT INTO PKTABLE VALUES ('unused', 1, 2);
+INSERT INTO FKTABLE VALUES (1, 2);
+ALTER TABLE FKTABLE ADD CONSTRAINT fktable_fk
+ FOREIGN KEY (ftest1, ftest2) REFERENCES PKTABLE (ptest1, ptest2) NOT VALID;
+ALTER TABLE PKTABLE OWNER TO regress_foreign_key_user;
+ALTER TABLE FKTABLE OWNER TO regress_foreign_key_user;
+REVOKE SELECT ON PKTABLE FROM regress_foreign_key_user;
+GRANT SELECT (ptest1) ON PKTABLE TO regress_foreign_key_user;
+
+-- Lack of SELECT on FKTABLE forces validation to check each row.
+REVOKE SELECT ON FKTABLE FROM regress_foreign_key_user;
+SET ROLE regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+GRANT SELECT (ptest2) ON PKTABLE TO regress_foreign_key_user;
+
+-- Per-row validation also requires UPDATE privilege.
+REVOKE UPDATE ON PKTABLE FROM regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk; -- fails
+-- UPDATE on the unrelated column is enough.
+GRANT UPDATE (ptest0) ON PKTABLE TO regress_foreign_key_user;
+ALTER TABLE FKTABLE VALIDATE CONSTRAINT fktable_fk;
+RESET ROLE;
+
DROP TABLE FKTABLE;
DROP TABLE PKTABLE;
--
2.47.3
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-19 07:05 Amit Langote <amitlangote09@gmail.com>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Amit Langote @ 2026-09-19 07:05 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; +Cc: Nikolay Samokhvalov <nik@postgres.ai>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
On Fri, Sep 18, 2026 at 6:21 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Thu, Sep 17, 2026 at 8:00 AM Amit Langote <amitlangote09@gmail.com> wrote:
> > On Thu, Sep 17, 2026 at 0:41 Melanie Plageman <melanieplageman@gmail.com> wrote:
> >> On Tue, Sep 15, 2026 at 11:12 PM Amit Langote <amitlangote09@gmail.com> wrote:
> >> > Thanks for the report and the patch.
> >> >
> >> > I've added an open item:
> >> >
> >> > RI fastpath misses pg_amop updates
> >> > Commit: 2da86c1ef9b
> >> > Owner: Amit Langote
> >>
> >> Will the fix be something like what Nikolay proposed or something
> >> different that solves both the issue in the fast path and the
> >> pre-existing issue with the cached cast functions?
> >
> >
> > There’s some overlap but I’m planning to fix the cast issue separately, that is, not combine it with the fix for this open item which I’d like to fix by beta4 freeze.
>
> Attached are the patches for fixing the two open items, which I plan
> to push tomorrow.
>
> Patch 0001 needs to fix a batching specific function (or it won't
> compile) so there are separate versions for master and 19.
>
> For 0002, I am attaching only the patch that fixes the per-row fast
> path, which has the same shape in both master and 19. Nik had posted
> one patch to fix both paths, but I decided to break it into one patch
> that fixes the per-row path (which applies to both master and 19) and
> another that is only needed in master for fixing the batching path for
> the same opfamily change errors. I'm adding the latter to the list of
> patches I now have locally for fixing the various batching path issues
> I am aware of.
I have pushed 0001 and 0002 now and closed the open items.
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-23 15:14 Nathan Bossart <nathandbossart@gmail.com>
parent: Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Nathan Bossart @ 2026-09-23 15:14 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Melanie Plageman <melanieplageman@gmail.com>; Nikolay Samokhvalov <nik@postgres.ai>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
On Sat, Sep 19, 2026 at 04:05:30PM +0900, Amit Langote wrote:
> I have pushed 0001 and 0002 now and closed the open items.
There's still an open item for "RI fastpath doesn't call
ExecutorCheckPerms_hook". Can that one be marked as resolved?
--
nathan
^ permalink raw reply [nested|flat] 16+ messages in thread
* Re: PG19: two RI fast-path issues found while testing the batching revert
@ 2026-09-23 23:53 Amit Langote <amitlangote09@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 16+ messages in thread
From: Amit Langote @ 2026-09-23 23:53 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Melanie Plageman <melanieplageman@gmail.com>; Nikolay Samokhvalov <nik@postgres.ai>; pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Hi Nathan,
On Thu, Sep 24, 2026 at 12:14 AM Nathan Bossart
<nathandbossart@gmail.com> wrote:
> On Sat, Sep 19, 2026 at 04:05:30PM +0900, Amit Langote wrote:
> > I have pushed 0001 and 0002 now and closed the open items.
>
> There's still an open item for "RI fastpath doesn't call
> ExecutorCheckPerms_hook". Can that one be marked as resolved?
Sorry, just noticed I had copy-pasted the wrong link for a new item I
added on Tuesday. The correct link is this:
https://www.postgresql.org/message-id/CA%2BHiwqE4oLLTJqRA%3DpzahLGcnXVPxgYtNsLvOm_-tW04rOqSXw%40mail...
I will push the patch and close this item later today.
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 16+ messages in thread
end of thread, other threads:[~2026-09-23 23:53 UTC | newest]
Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-09-10 16:02 PG19: two RI fast-path issues found while testing the batching revert Nikolay Samokhvalov <nik@postgres.ai>
2026-09-10 23:41 ` Amit Langote <amitlangote09@gmail.com>
2026-09-11 00:33 ` Nikolay Samokhvalov <nik@postgres.ai>
2026-09-11 09:25 ` Amit Langote <amitlangote09@gmail.com>
2026-09-11 10:08 ` Amit Langote <amitlangote09@gmail.com>
2026-09-12 15:04 ` Nikolay Samokhvalov <nik@postgres.ai>
2026-09-13 08:20 ` Nikolay Samokhvalov <nik@postgres.ai>
2026-09-15 02:13 ` Nikolay Samokhvalov <nik@postgres.ai>
2026-09-15 09:00 ` Nikolay Samokhvalov <nik@postgres.ai>
2026-09-16 03:12 ` Amit Langote <amitlangote09@gmail.com>
2026-09-16 15:40 ` Melanie Plageman <melanieplageman@gmail.com>
2026-09-16 23:00 ` Amit Langote <amitlangote09@gmail.com>
2026-09-18 09:21 ` Amit Langote <amitlangote09@gmail.com>
2026-09-19 07:05 ` Amit Langote <amitlangote09@gmail.com>
2026-09-23 15:14 ` Nathan Bossart <nathandbossart@gmail.com>
2026-09-23 23:53 ` Amit Langote <amitlangote09@gmail.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox