Re: Eliminating SPI / SQL from some RI triggers - take 3

On Tue, Mar 31, 2026 at 7:57 PM Junwang Zhao <zhjwpku@gmail.com> wrote:
> On Tue, Mar 31, 2026 at 5:17 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > On Tue, Mar 31, 2026 at 6:09 PM Chao Li <li.evan.chao@gmail.com> wrote:
> > > > On Mar 30, 2026, at 19:15, Amit Langote <amitlangote09@gmail.com> wrote:
> > > > Kept looking at 0002 and found a couple of things to improve or change
> > > > my thoughts about.  I decided to move the permission check from fast
> > > > path cache entry creation into ri_FastPathBatchFlush(), alongside the
> > > > snapshot, so that permission changes between flushes are respected
> > > > rather than checked once at batch start; the check happens for every
> > > > row in the SPI and non-batched fast path.  Also, improved comments in
> > > > a few places to mention design decisions better.
> > > >
> > > > 0001 is mostly unchanged from v11 except I updated its commit message
> > > > to explain why only RI_FKey_check is covered and not the action
> > > > triggers as the topic has come up in previous threads about this
> > > > topic.
> > > >
> > > > Still planning to commit 0001 tomorrow.
> > > >
> > > > --
> > > > Thanks, Amit Langote
> > > > <v12-0001-Add-fast-path-for-foreign-key-constraint-checks.patch><v12-0002-Batch-FK-rows-and-use-SK_SEARCHARRAY-for-fast-pa.patch>
> > >
> > > Hi Amit,
> > >
> > > While reading the recent commits, I saw that 0001 has been pushed as 2da86c1ef9b5446e0e22c0b6a5846293e58d98e3. However, I also just noticed a use-after-free issue in ri_LoadConstraintInfo(). It dereferences conForm after ReleaseSysCache(tup), which is unsafe. I am attaching a tiny patch to fix that.
> >
> > Thanks.  I noticed that too and pushed the fix an hour ago:
> >
> > https://www.postgresql.org/message-id/E1w7U6V-002H6n-0o%40gemulon.postgresql.org
> >
> > --
> > Thanks, Amit Langote
>
> prion is happy now, the fix works, thanks.

Yep, good.

Because I noticed a use-after-free with prion, I thought to check our
preparedness for CLOBBER_CACHE_ALWAYS and found issues in both the
committed patch (and similar code in 0002): riinfo going stale inside
ri_FastPathCheck() after relation opens and dangling fpmeta pointer
after riinfo invalidation.  0001 fixes those; I'll apply it tomorrow
morning.

0002 is the rebased batching patch.

--
Thanks, Amit Langote