Horton
Menu

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

amit <amitlangote09@gmail.com>

View thread
From: Amit Langote <amitlangote09@gmail.com>
To: Junwang Zhao <zhjwpku@gmail.com>
Cc: Haibo Yan <tristan.yim@gmail.com>, Pavel Stehule <pavel.stehule@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Tomas Vondra <tomas@vondra.me>
Date: 2026-03-30T11:15:05Z
Lists: pgsql-hackers

Attachments

On Mon, Mar 30, 2026 at 1:55 PM Amit Langote <amitlangote09@gmail.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

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix some const qualifier use in ri_triggers.c

  2. doc: Improve consistency of parallel vacuum description.

  3. Move afterTriggerFiringDepth into AfterTriggersData

  4. Fix typo left by 34a30786293

  5. Fix RI fast-path crash under nested C-level SPI

  6. Add nkeys parameter to recheck_matched_pk_tuple()

  7. Fix deferred FK check batching introduced by commit b7b27eb41a5

  8. Optimize fast-path FK checks with batched index probes

  9. Make FastPathMeta self-contained by copying FmgrInfo structs

  10. Add fast path for foreign key constraint checks