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

On Fri, Sep 11, 2026 at 6:25 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Fri, Sep 11, 2026 at 9:33 AM Nikolay Samokhvalov <nik(at)postgres(dot)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