From: Chao Li <li.evan.chao@gmail.com>
Message-Id: <C2133B47-79CD-40FF-B088-02D20D654806@gmail.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_952447D7-1125-4AEC-84E5-8C3185133ABA"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: Eliminating SPI / SQL from some RI triggers - take 3
Date: Fri, 3 Apr 2026 16:57:36 +0800
In-Reply-To: 
 <CA+HiwqGFRAH2O5bGpNMErFopFyn-2-Zu3+5y+BFQim9TE8z+Pw@mail.gmail.com>
Cc: Junwang Zhao <zhjwpku@gmail.com>,
 Haibo Yan <tristan.yim@gmail.com>,
 Pavel Stehule <pavel.stehule@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>,
 Tomas Vondra <tomas@vondra.me>
To: Amit Langote <amitlangote09@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>
 <2BE661BA-D909-4093-BF78-DB9B0C099337@gmail.com>
 <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>
 <77FA04FE-1F84-4DA1-8855-8BBFD8CC889A@gmail.com>
 <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>
 <C3ACBAE1-CC5F-4688-9828-4668CDE37F83@gmail.com>
 <CA+HiwqGFRAH2O5bGpNMErFopFyn-2-Zu3+5y+BFQim9TE8z+Pw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/C2133B47-79CD-40FF-B088-02D20D654806%40gmail.com>
Precedence: bulk


--Apple-Mail=_952447D7-1125-4AEC-84E5-8C3185133ABA
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> On Apr 3, 2026, at 13:52, Amit Langote <amitlangote09@gmail.com> =
wrote:
>=20
> Hi,
>=20
> On Thu, Apr 2, 2026 at 5:00=E2=80=AFPM Chao Li =
<li.evan.chao@gmail.com> wrote:
>>> On Apr 2, 2026, at 15:41, Amit Langote <amitlangote09@gmail.com> =
wrote:
>>> Will push tomorrow morning (Friday) barring objections.
>>> <v17-0001-Batch-FK-rows-and-use-SK_SEARCHARRAY-for-fast-pa.patch>
>>=20
>> With a quick eyeball review, I found a typo:
>> ```
>> + * relcache invalidation.  The entry itself is torn down at batch at =
batch end
>> ```
>>=20
>> There are two =E2=80=9Cat batch=E2=80=9D.
>=20
> Thanks for spotting that.  Fixed and pushed.
>=20
>> I plan to spend time testing and tracing this patch tomorrow. But I =
don=E2=80=99t want to block your progress, if I find anything, I will =
report to you.
>=20
> Sure, I didn't want to leave committing this to the weekend or the =
next week.
>=20
> --=20
> 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 >=3D 0. =
But in the deferred case, afterTriggers.query_depth is -1.

=46rom 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=E2=80=99m not sure whether I may have missed =
something.

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


--Apple-Mail=_952447D7-1125-4AEC-84E5-8C3185133ABA
Content-Disposition: attachment;
	filename=fix_deferred_trigger.diff
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="fix_deferred_trigger.diff"
Content-Transfer-Encoding: 7bit

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;
 }

--Apple-Mail=_952447D7-1125-4AEC-84E5-8C3185133ABA--