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
Description: Binary data
