PG19: two RI fast-path issues found while testing the batching revert

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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:

/messages/by-id/CA+HiwqE4oLLTJqRA=pzahLGcnXVPxgYtNsLvOm_-tW04rOqSXw@mail.gmail.com

I will push the patch and close this item later today.

--
Thanks, Amit Langote

On Thu, Sep 24, 2026 at 8:53 AM Amit Langote <amitlangote09@gmail.com> wrote:

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:

/messages/by-id/CA+HiwqE4oLLTJqRA=pzahLGcnXVPxgYtNsLvOm_-tW04rOqSXw@mail.gmail.com

I will push the patch and close this item later today.

Done.

--
Thanks, Amit Langote

Hi,

While reviewing the RI fast path after the batching removal, with help
from Claude Opus 5.5, I found more issues suggesting that directly
calling the cast function using FunctionCall3() might not be such a
good idea. Nikolay's report upthread, where a cast is redefined while
ri_triggers.c's internal cache still uses the old definition causing
"cache lookup failed for function", is only one of the issues caused
by it. When comparing how the corresponding SPI query containing the
same cast handles necessary cases, Opus found more issues resulting
from directly calling the cast function. For example, a cast
returning NULL also raises an internal "function ... returned NULL"
instead of a FK violation as shown here (Opus' example):

CREATE TABLE pk (id int4 PRIMARY KEY);
INSERT INTO pk VALUES (1);
CREATE TYPE word AS ENUM ('one', 'none');
CREATE FUNCTION word_to_int4(word) RETURNS int4
LANGUAGE sql IMMUTABLE AS $$ SELECT CASE WHEN $1::text = 'one'
THEN 1 END $$;
CREATE CAST (word AS int4) WITH FUNCTION word_to_int4(word) AS IMPLICIT;
CREATE TABLE fk_word (a word REFERENCES pk, n int);
INSERT INTO fk_word VALUES ('one', 0);
INSERT 0 1
INSERT INTO fk_word VALUES ('none', 0);
ERROR: function 16397 returned NULL

If the PK table is partitioned, the fast path is not taken, and you
get proper handling of the cast function defined this way:

CREATE TABLE pk (id int4 PRIMARY KEY) PARTITION BY LIST (id);
CREATE TABLE pk_part PARTITION OF pk DEFAULT;
INSERT INTO pk VALUES (1);
CREATE TYPE word AS ENUM ('one', 'none');
CREATE FUNCTION word_to_int4(word) RETURNS int4
LANGUAGE sql IMMUTABLE AS $$ SELECT CASE WHEN $1::text = 'one'
THEN 1 END $$;
CREATE CAST (word AS int4) WITH FUNCTION word_to_int4(word) AS IMPLICIT;
CREATE TABLE fk_word (a word REFERENCES pk, n int);
INSERT INTO fk_word VALUES ('one', 0);
INSERT INTO fk_word VALUES ('none', 0);
ERROR: insert or update on table "fk_word" violates foreign key
constraint "fk_word_a_fkey"
DETAIL: Key (a)=(none) is not present in table "pk".

So I propose fixing the fast-path selection criteria to avoid it when
such a cast is necessary, as done in the attached 0001. 0002 is a
patch to drop the code added in the fast-path commit to facilitate
this cast handling as it is now dead code. 0001+0002 apply on top of
the other fix I posted at [1]/messages/by-id/CA+HiwqFaipkMsZ8XP-9sMk21h0Q7rih=NpJQXsBAaD39VtnrOA@mail.gmail.com, which I plan to commit tomorrow.

Note that these errors are also reachable in the
RI_FKey_fk_upd_check_required() -> ri_KeysEqual() path, which predates
the addition of the fast-path. It compares the old and new key of an
updated referencing row by calling the cast function the same way, so
with the setup of the first example above:

UPDATE fk_word SET a = 'none';
ERROR: function 16467 returned NULL

I wrote a patch to fix that too (0003). The lack of field complaints
might stem from people rarely setting things up this way in the real
world, but I think it's worth committing. The fix is simple: when a
cast function would be needed, compare the bytes of the old and new
values instead. At worst, this runs a check that could have been
skipped.

I would like to commit 0001 and 0002 before RC1 next week, since
without them a foreign key using such a cast is checked differently
depending on whether the fast path or SPI handles it. I can wait for
others to opine on whether 0003 is warranted (I haven't checked the
back-patching pain).

--
Thanks, Amit Langote

[1]: /messages/by-id/CA+HiwqFaipkMsZ8XP-9sMk21h0Q7rih=NpJQXsBAaD39VtnrOA@mail.gmail.com

On Mon, Oct 5, 2026 at 10:18 PM Amit Langote <amitlangote09@gmail.com> wrote:

Hi,

While reviewing the RI fast path after the batching removal, with help
from Claude Opus 5.5, I found more issues suggesting that directly
calling the cast function using FunctionCall3() might not be such a
good idea. Nikolay's report upthread, where a cast is redefined while
ri_triggers.c's internal cache still uses the old definition causing
"cache lookup failed for function", is only one of the issues caused
by it. When comparing how the corresponding SPI query containing the
same cast handles necessary cases, Opus found more issues resulting
from directly calling the cast function. For example, a cast
returning NULL also raises an internal "function ... returned NULL"
instead of a FK violation as shown here (Opus' example):

CREATE TABLE pk (id int4 PRIMARY KEY);
INSERT INTO pk VALUES (1);
CREATE TYPE word AS ENUM ('one', 'none');
CREATE FUNCTION word_to_int4(word) RETURNS int4
LANGUAGE sql IMMUTABLE AS $$ SELECT CASE WHEN $1::text = 'one'
THEN 1 END $$;
CREATE CAST (word AS int4) WITH FUNCTION word_to_int4(word) AS IMPLICIT;
CREATE TABLE fk_word (a word REFERENCES pk, n int);
INSERT INTO fk_word VALUES ('one', 0);
INSERT 0 1
INSERT INTO fk_word VALUES ('none', 0);
ERROR: function 16397 returned NULL

If the PK table is partitioned, the fast path is not taken, and you
get proper handling of the cast function defined this way:

CREATE TABLE pk (id int4 PRIMARY KEY) PARTITION BY LIST (id);
CREATE TABLE pk_part PARTITION OF pk DEFAULT;
INSERT INTO pk VALUES (1);
CREATE TYPE word AS ENUM ('one', 'none');
CREATE FUNCTION word_to_int4(word) RETURNS int4
LANGUAGE sql IMMUTABLE AS $$ SELECT CASE WHEN $1::text = 'one'
THEN 1 END $$;
CREATE CAST (word AS int4) WITH FUNCTION word_to_int4(word) AS IMPLICIT;
CREATE TABLE fk_word (a word REFERENCES pk, n int);
INSERT INTO fk_word VALUES ('one', 0);
INSERT INTO fk_word VALUES ('none', 0);
ERROR: insert or update on table "fk_word" violates foreign key
constraint "fk_word_a_fkey"
DETAIL: Key (a)=(none) is not present in table "pk".

So I propose fixing the fast-path selection criteria to avoid it when
such a cast is necessary, as done in the attached 0001. 0002 is a
patch to drop the code added in the fast-path commit to facilitate
this cast handling as it is now dead code. 0001+0002 apply on top of
the other fix I posted at [1], which I plan to commit tomorrow.

Note that these errors are also reachable in the
RI_FKey_fk_upd_check_required() -> ri_KeysEqual() path, which predates
the addition of the fast-path. It compares the old and new key of an
updated referencing row by calling the cast function the same way, so
with the setup of the first example above:

UPDATE fk_word SET a = 'none';
ERROR: function 16467 returned NULL

I wrote a patch to fix that too (0003). The lack of field complaints
might stem from people rarely setting things up this way in the real
world, but I think it's worth committing. The fix is simple: when a
cast function would be needed, compare the bytes of the old and new
values instead. At worst, this runs a check that could have been
skipped.

I would like to commit 0001 and 0002 before RC1 next week, since
without them a foreign key using such a cast is checked differently
depending on whether the fast path or SPI handles it. I can wait for
others to opine on whether 0003 is warranted (I haven't checked the
back-patching pain).

Added an open item, though I'll try to commit 0001 and 0002 no later
than tomorrow.

--
Thanks, Amit Langote

On Mon Oct 5, 2026 at 10:18 AM -03, Amit Langote wrote:

So I propose fixing the fast-path selection criteria to avoid it when
such a cast is necessary, as done in the attached 0001. 0002 is a
patch to drop the code added in the fast-path commit to facilitate
this cast handling as it is now dead code. 0001+0002 apply on top of
the other fix I posted at [1], which I plan to commit tomorrow.

[ ... ]

I would like to commit 0001 and 0002 before RC1 next week, since
without them a foreign key using such a cast is checked differently
depending on whether the fast path or SPI handles it. I can wait for
others to opine on whether 0003 is warranted (I haven't checked the
back-patching pain).

Hi, thanks for the patches! I've reviewed and tested 0001 and 0002 and I
dind't find any issues.

I also noticed that a user-defined WITH INOUT AS IMPLICIT cast now works
through SPI. Before 0001 it failed with "no conversion function" from
ri_HashCompareOp(), so perhaps it's worth adding a test for it.

The pg_cast invalidation also seems to work fine. I tested it with a
session that had cached the fast path for a FK using a binary cast,
while another session replaced the cast with a function returning NULL,
and the next insert on the first session went through SPI and reported
the FK violation. The same works when the cast is replaced within the
same transaction.

I have just a single comment: I'm wondering if the tests that 0001
rewords (fk_defer_main, the fkint invalidation test, fp_reentry and the
ri_snapshot STABLE test) now all seems to go through SPI, so they no
longer test the fast path issues they were written for, what do you
think?

Some minor comments:

- "getBaseType(fk_type) != righttype &&" on the new check seems
redundant since IsBinaryCoercible() already handles the same type and
domain cases.

- The "typedef struct RI_CompareHashEntry RI_CompareHashEntry;" added
by 2da86c1ef9 before FastPathMeta is not needed anymore.

- The ri_check_fastpath_index() header comment only mentions index
properties, but the new check depends on the FK table. The comment
before the fast path call on RI_FKey_check() also only mentions
non-partitioned and non-temporal FKs.

- "pk_fast" on the ri_snapshot test now also goes through SPI, so the
name is a may be misleading.

I'll review 0003 next.

--
Matheus Alcantara
EDB: https://www.enterprisedb.com