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