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

> On Mar 30, 2026, at 19:15, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> 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
> <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.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/