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.

-- 
Thanks, Amit Langote


Reply via email to