Re: Eliminating SPI / SQL from some RI triggers - take 3 - Mailing list pgsql-hackers

From Chao Li
Subject Re: Eliminating SPI / SQL from some RI triggers - take 3
Date
Msg-id C2133B47-79CD-40FF-B088-02D20D654806@gmail.com
Whole thread Raw
In response to Re: Eliminating SPI / SQL from some RI triggers - take 3  (Amit Langote <amitlangote09@gmail.com>)
Responses Re: Eliminating SPI / SQL from some RI triggers - take 3
List pgsql-hackers

> On Apr 3, 2026, at 13:52, Amit Langote <amitlangote09@gmail.com> wrote:
>
> Hi,
>
> On Thu, Apr 2, 2026 at 5:00 PM 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>
>>
>> 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
deferredRI 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
beonly to bypass the ALTER TABLE validation case, where batch callbacks would never fire, and MyTriggerDepth is 0. So,
maybewe 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
Iam 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/





Attachment

pgsql-hackers by date:

Previous
From: shveta malik
Date:
Subject: Re: synchronized_standby_slots behavior inconsistent with quorum-based synchronous replication
Next
From: John Naylor
Date:
Subject: Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc?