On Wed, Aug 26, 2026 at 10:25 PM Amit Langote <[email protected]> wrote: > On Sat, Aug 22, 2026 at 4:43 PM Amit Langote <[email protected]> wrote: > > On Thu, Aug 20, 2026 at 5:34 PM Amit Langote <[email protected]> > > wrote: > > > On Wed, Aug 19, 2026 at 10:12 PM Amit Langote <[email protected]> > > > wrote: > > > > On Wed, Aug 19, 2026 at 12:09 AM Noah Misch <[email protected]> wrote: > > > > > On Tue, Aug 18, 2026 at 10:42:14PM +0900, Amit Langote wrote: > > > > > > After considering Peter's report of another bug [1] that is fixed by > > > > > > 0001, I tested both his reproducer and nested firing involving the > > > > > > same constraint. > > > > > > > > > > > > The latter exposed a problem in v1: the cache was still keyed only > > > > > > by > > > > > > constraint OID, so a nested check of the same constraint reused the > > > > > > outer entry and did not register a callback at the nested query > > > > > > depth. > > > > > > In v2, the key is now (constraint OID, query depth), giving each > > > > > > firing level its own entry and callback. > > > > > > > > > > > > 0001 now includes regression tests for both cases. 0002 and 0003 > > > > > > are unchanged. > > > > > > > > > > > > I would like to commit these sometime this week and would > > > > > > appreciate a review. > > > > > > > > > > I won't be able to review this. The list should consider it up for > > > > > grabs. > > > > > > > > Attached is v3, rebased over latest master. > > > > > > > > I reorganized the series to separate firing-state restoration (0001), > > > > per-firing-cycle batch and callback scoping that fixes Peter's report > > > > [1] (0002), and per-subtransaction batch tracking as suggested by Noah > > > > (0003). 0003 also folds in the invariant assertion previously sent > > > > separately. The combined code is otherwise unchanged from v2. > > > > > > > > [1] > > > > https://postgr.es/m/CAH2-Wz%3DD533JbF_ak_Pc8kP0FKse-ju8DnMxtjvY%3D%3DyHsP4xgw%40mail.gmail.com > > > > > > I've now pushed 0001 and 0002. Since they fix live bugs, I decided to > > > commit them sooner rather than later. > > > > > > I plan to commit the attached remaining patch tomorrow, barring > > > objections, and then close this item. > > > > Pushed and closed the item. > > Ayush Tiwari reported another hole in the per-firing-cycle fix to me > off-list. Here is his reproducer: > > CREATE TABLE pk (id int PRIMARY KEY); > INSERT INTO pk VALUES (1); > > CREATE TABLE fk ( > a int REFERENCES pk (id), > b int CONSTRAINT fk_deferred REFERENCES pk (id) > DEFERRABLE INITIALLY DEFERRED); > > CREATE FUNCTION check_now() RETURNS trigger LANGUAGE plpgsql AS $$ > BEGIN > BEGIN > SET CONSTRAINTS fk_deferred IMMEDIATE; > EXCEPTION WHEN foreign_key_violation THEN > RAISE NOTICE 'caught by SET CONSTRAINTS'; > END; > RETURN NEW; > END$$; > > -- Name sorts after the RI trigger, so column a is already batched. > CREATE TRIGGER zz_check_now AFTER INSERT ON fk > FOR EACH ROW EXECUTE FUNCTION check_now(); > > BEGIN; > INSERT INTO fk VALUES (1, 999); > > On the current master with my fixes from last week, an > assertion-enabled build produces: > > CREATE TABLE > INSERT 0 1 > CREATE TABLE > CREATE FUNCTION > CREATE TRIGGER > BEGIN > WARNING: resource was not closed: relation "pk_pkey" > WARNING: resource was not closed: relation "pk" > WARNING: resource was not closed: TupleDesc 0xffff812677f0 (16392,-1) > WARNING: resource was not closed: TupleDesc 0xffff81270500 (16386,-1) > server closed the connection unexpectedly > This probably means the server terminated abnormally > before or while processing the request. > The connection to the server was lost. > > The relevant part of the backtrace is: > > #3 ExceptionalCondition (conditionName="false", > fileName="../src/backend/utils/adt/ri_triggers.c", lineNumber=4543) > #4 AtEOSubXact_RI (isCommit=true, mySubid=2, parentSubid=1) > at ../src/backend/utils/adt/ri_triggers.c:4543 > #5 CommitSubTransaction () > at ../src/backend/access/transam/xact.c:5247 > #6 ReleaseCurrentSubTransaction () > at ../src/backend/access/transam/xact.c:4836 > #7 exec_stmt_block (...) > at ../src/pl/plpgsql/src/pl_exec.c:1859 > > SET CONSTRAINTS ... IMMEDIATE starts a nested firing cycle without > opening a new query level, so keying the cache by constraint OID and > query depth does not distinguish this cycle from the enclosing one. > > I am preparing a patch that uses firing depth, gives the SET > CONSTRAINTS cycle its own callback list, and removes stale entries on > either subtransaction commit or abort. I will post it shortly.
Here is that patch. One thing missing from my report: the crash wasn't the worst of it. The nested cycle's batch was never flushed, so SET CONSTRAINTS ... IMMEDIATE returned success for a constraint it hadn't checked, and the orphan row reached commit. I'm not proposing to commit this yet. I'm assessing whether the batching layer holds up under a test harness and will give the RMT my assessment by Monday on whether to keep it in v19. If the batching layer is reverted from v19, this patch is master-only, assuming it's ok for master to keep the layer in its current form. If it stays, it belongs in both branches. I'm happy to commit and backpatch sooner if that's preferred for the open item. -- Thanks, Amit Langote
v1-0001-Fix-RI-fast-path-batching-in-a-nested-SET-CONSTRA.patch
Description: Binary data
