On Tue, Apr 7, 2026 at 10:46 AM Chao Li <[email protected]> wrote: > > 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: > >>> 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
