Re: Eliminating SPI / SQL from some RI triggers - take 3

On Tue, Apr 7, 2026 at 10:46 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> > On Apr 6, 2026, at 17:45, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > On Fri, Apr 3, 2026 at 6:39 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> >> On Fri, Apr 3, 2026 at 5:58 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> >>> 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
>
> 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".
> ```

Thanks for checking. Pushed.

--
Thanks, Amit Langote