> On Apr 6, 2026, at 17:45, Amit Langote <[email protected]> wrote:
>
> On Fri, Apr 3, 2026 at 6:39 PM Amit Langote <[email protected]> wrote:
>> On Fri, Apr 3, 2026 at 5:58 PM Chao Li <[email protected]> wrote:
>>>> On Apr 3, 2026, at 13:52, Amit Langote <[email protected]> wrote:
>>>> On Thu, Apr 2, 2026 at 5:00 PM Chao Li <[email protected]> wrote:
>>>>> 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.
>>>
>>> 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.
>>
>> I think you might be right. Thanks for the patch. It looks correct
>> to me at a glance, but I will need to check it a bit more closely
>> before committing.
>
> Thinking about this some more, your fix is on the right track but
> needs a bit more work -- MyTriggerDepth > 0 is too broad since it
> fires for BEFORE triggers too. I have a revised version using a new
> afterTriggerFiringDepth counter that I'll push shortly.
>
> Added an open item for tracking in the meantime:
> https://wiki.postgresql.org/wiki/PostgreSQL_19_Open_Items#Open_Issues
>
> --
> Thanks, Amit Langote
> <v2-0001-Fix-deferred-FK-check-batching-introduced-by-comm.patch>
V2 looks good to me. Besides the normal cases, I also traced an abnormal case
to verify that afterTriggerFiringDepth is always reset to 0:
```
evantest=# begin;
BEGIN
evantest=*# INSERT INTO fk VALUES (2);
INSERT 0 1
evantest=*# commit;
ERROR: insert or update on table "fk" violates foreign key constraint
"fk_a_fkey"
DETAIL: Key (a)=(2) is not present in table "pk".
```
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
