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

On Mon, Mar 30, 2026 at 1:55 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> Junwang pointed out off-list that FK tuples added to
> RI_FastPathEntry.batch[] were being copied into TopTransactionContext
> rather than flush_cxt, so they would accumulate until the batch was
> exhausted rather than being reclaimed per flush. Fixed in
> ri_FastPathBatchAdd() in 0002.
>
> Also added a couple of comments in trigger.c that were missing: an
> Assert and explanation in RegisterAfterTriggerBatchCallback()
> clarifying the query_depth >= 0 precondition, a comment at the
> AfterTriggerEndQuery call site explaining why
> FireAfterTriggerBatchCallbacks() must precede the query_depth
> decrement and AfterTriggerFreeQuery, and brief intent comments at the
> AfterTriggerFireDeferred and AfterTriggerSetState call sites.
>
> Plan is to commit 0001 tomorrow barring objections and let it sit for
> a bit before committing 0002. Feedback on 0002, particularly on the
> AfterTriggerBatchCallback mechanism in trigger.c, welcome in the
> meantime.

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