public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Amit Langote <[email protected]>
Cc: Junwang Zhao <[email protected]>
Cc: Haibo Yan <[email protected]>
Cc: Pavel Stehule <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Tomas Vondra <[email protected]>
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
Date: Fri, 3 Apr 2026 16:57:36 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+HiwqGFRAH2O5bGpNMErFopFyn-2-Zu3+5y+BFQim9TE8z+Pw@mail.gmail.com>
References: <CA+HiwqF4C0ws3cO+z5cLkPuvwnAwkSp7sfvgGj3yQ=Li6KNMqA@mail.gmail.com>
<CA+HiwqGM6nvAV5O+=Nr+BXMPWOma0oeCRVzVP0XiLE8zX5TVAg@mail.gmail.com>
<CA+HiwqGMaovCUgDbGxVGnK0Mrivr+ph3YE2Ws+47-ugyPb4f7g@mail.gmail.com>
<CAFj8pRDaiBe_GOLk_yyYHTtPiDAAaLOM8u1-=Q3ZgXBTH+1igg@mail.gmail.com>
<CA+HiwqGA5Ay_MR0eJEEbt4j6WrVh4F+AasTp8yCbs5aJLOJn6Q@mail.gmail.com>
<CAEG8a3JM=NoqiTK0V6S9FNxZPvy1+C5F7rfafTtPKBVWnunL-g@mail.gmail.com>
<CA+HiwqEyiLCY6MTLbOJXDdLNNQLaURYHvdW797MQgbjEK9od4Q@mail.gmail.com>
<CAEG8a3+VBpwPf1Rm-ECD90whM9b3YnGhux5CVXdsL6khiBfzRQ@mail.gmail.com>
<CA+HiwqF2UHzF0sKCp-F2a-U29rqh_9ZPy=f1h+Fh_=M8efj3pg@mail.gmail.com>
<CAEG8a3L9Ew-WL8sxLROVOcypeaENPmd8qCmMvz4geoGL1TDGCA@mail.gmail.com>
<CAEG8a3+nUFQo4sdPQF9xy0J73J8RFJ5U9A5+_kMosGDaZ+1sXA@mail.gmail.com>
<[email protected]>
<CAEG8a3JyKdizWvYsF+z_mA1BKy=dpW11iKVMOG=bk6Tbz6M1Bw@mail.gmail.com>
<CAEG8a3+Hf4tvvbts29_k_AFhWQmRYfEo_SW4C5FY_140iKghBw@mail.gmail.com>
<CA+HiwqFV-PY-3BxM6j5TaAiC3AwedDxo-6vwRSbvygg3zF+xAQ@mail.gmail.com>
<CA+HiwqHpaisS-e+0gAgzh92qZAFxncAJMmmTRZZN=efoeTPgOg@mail.gmail.com>
<CA+HiwqFwZ6WLRbY8R7VC7JVp5Jot6ktZOkr9wDxTjoK=W1SgdQ@mail.gmail.com>
<CA+HiwqF_Kz=R8juHJBiOATvabWSOugK4VaGOxoJ_n=E2c7UM9w@mail.gmail.com>
<CA+HiwqHCB7kcbspkhaLN9enoj5x=ehzhFM4PXDgWUUP8Px41GA@mail.gmail.com>
<[email protected]>
<CA+HiwqHpVtP485wEKuXdOkdoZWhvVvfFH40-og07Jp3MPx21eg@mail.gmail.com>
<CAEG8a3JWHkJSXe9nNcVK7wnYKUEqWuMGFDhy5BynB_9tEjmEUg@mail.gmail.com>
<CA+HiwqFjfumKrWy03q5M309xJJVYt0WgGfH6AZ8BjFhSwppwsQ@mail.gmail.com>
<CAEG8a3JjP1LaKSv-r3AMJLRyLMzENJrKshWsDvDouMPM_sizmA@mail.gmail.com>
<CA+HiwqFQ+ZA7hSOygv4uv_t75B3r0_gosjadetCsAEoaZwTu6g@mail.gmail.com>
<CA+HiwqHdB0r8z6Mut13BxpYNq2W-os+Arju4mcZbCyU9PeaVog@mail.gmail.com>
<CAEG8a3K5ayVNkCDnK9OtNb+4OY0chJtW6ObgEOSFjqyymQda8Q@mail.gmail.com>
<CA+HiwqGJYCgEs_F-LBtrRdx-Y969LMr-OVogjFXU6U-0q5bOwQ@mail.gmail.com>
<CA+HiwqF2Ma6R_QyjfPBvFreYbezFpGcwASJE0a2bM+Y0jvof+g@mail.gmail.com>
<[email protected]>
<CA+HiwqGFRAH2O5bGpNMErFopFyn-2-Zu3+5y+BFQim9TE8z+Pw@mail.gmail.com>
> On Apr 3, 2026, at 13:52, Amit Langote <[email protected]> wrote:
>
> Hi,
>
> On Thu, Apr 2, 2026 at 5:00 PM Chao Li <[email protected]> wrote:
>>> On Apr 2, 2026, at 15:41, Amit Langote <[email protected]> wrote:
>>> Will push tomorrow morning (Friday) barring objections.
>>> <v17-0001-Batch-FK-rows-and-use-SK_SEARCHARRAY-for-fast-pa.patch>
>>
>> With a quick eyeball review, I found a typo:
>> ```
>> + * relcache invalidation. The entry itself is torn down at batch at batch end
>> ```
>>
>> There are two “at batch”.
>
> Thanks for spotting that. Fixed and pushed.
>
>> I plan to spend time testing and tracing this patch tomorrow. But I don’t want to block your progress, if I find anything, I will report to you.
>
> Sure, I didn't want to leave committing this to the weekend or the next week.
>
> --
> Thanks, Amit Langote
Hi Amit,
I spent several hours debugging this patch today, and I found a problem where the batch mode doesn't seem to handle deferred RI triggers, although the commit message suggests that it should.
I traced this scenario:
```
CREATE TABLE pk (a int primary key);
CREATE TABLE fk (a int references pk(a) DEFERRABLE INITIALLY DEFERRED);
BEGIN;
INSERT INTO fk VALUES (1);
INSERT INTO pk VALUES (1);
COMMIT;
```
When COMMIT is executed, it reaches RI_FKey_check(), where AfterTriggerIsActive() checks whether afterTriggers.query_depth >= 0. But in the deferred case, afterTriggers.query_depth is -1.
From the code:
```
if (ri_fastpath_is_applicable(riinfo))
{
if (AfterTriggerIsActive())
{
/* Batched path: buffer and probe in groups */
ri_FastPathBatchAdd(riinfo, fk_rel, newslot);
}
else
{
/* ALTER TABLE validation: per-row, no cache */
ri_FastPathCheck(riinfo, fk_rel, newslot);
}
return PointerGetDatum(NULL);
}
```
So this ends up falling back to the per-row path for deferred RI checks at COMMIT, even though the intent here seems to be only to bypass the ALTER TABLE validation case, where batch callbacks would never fire, and MyTriggerDepth is 0. So, maybe we can just check MyTriggerDepth>0 in AfterTriggerIsActive().
I tried the attached fix. With it, deferred triggers go through the batch mode, and all existing tests still pass. But I am still new to PG development, so I’m not sure whether I may have missed something.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] fix_deferred_trigger.diff (1.0K, 2-fix_deferred_trigger.diff)
download | inline diff:
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 90e94fb8a5a..63355ebb02f 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -6806,10 +6806,10 @@ RegisterAfterTriggerBatchCallback(AfterTriggerBatchCallback callback,
* Allocate in TopTransactionContext so the item survives for the duration
* of the batch, which may span multiple trigger invocations.
*
- * Must be called while afterTriggers is active (query_depth >= 0);
+ * Must be called while afterTriggers is active (MyTriggerDepth > 0);
* callbacks registered outside a trigger-firing context would never fire.
*/
- Assert(afterTriggers.query_depth >= 0);
+ Assert(MyTriggerDepth > 0);
oldcxt = MemoryContextSwitchTo(TopTransactionContext);
item = palloc(sizeof(AfterTriggerCallbackItem));
item->callback = callback;
@@ -6858,5 +6858,5 @@ FireAfterTriggerBatchCallbacks(void)
bool
AfterTriggerIsActive(void)
{
- return afterTriggers.query_depth >= 0;
+ return MyTriggerDepth > 0;
}
view thread (63+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox