pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Track RI fast-path FK-check batches per firing cycle 4+ messages / 1 participants [nested] [flat]
* pgsql: Track RI fast-path FK-check batches per firing cycle @ 2026-08-20 08:22 Amit Langote <amitlan@postgresql.org> 0 siblings, 0 replies; 4+ messages in thread From: Amit Langote @ 2026-08-20 08:22 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Track RI fast-path FK-check batches per firing cycle Commit 34a30786293 fixed an RI fast-path crash under nested C-level SPI by keeping batch-callback lists per after-trigger query depth. That fix was incomplete: the RI fast path still tracked callback registration with one global flag. Once an outer firing cycle had registered its callback, the flag suppressed registration for a nested cycle, leaving the nested batch to be handled by the outer callback, too late and with the wrong snapshot, potentially after the ResourceOwner holding its relations had gone away. Nor is per-depth callback registration sufficient while the cache is keyed only by constraint OID. If nested firing checks the same constraint, it reuses the outer entry, combining rows that must be checked in separate firing cycles. Key the cache by both constraint OID and query depth. Register a callback for each depth that creates an entry, and make ri_FastPathEndBatch() flush and release only entries belonging to the ending depth. Add AfterTriggerCurrentQueryDepth() so ri_triggers.c can obtain the current depth; depth -1 represents deferred firing. Add regression coverage for nested firing through a cursor portal, whose resources must not outlive the nested cycle, nested firing of the same constraint at different query depths, and deferred firing at query depth -1. Reported-by: Noah Misch <noah@leadboat.com> Reported-by: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/20260705222115.be.noahmisch@microsoft.com Discussion: https://postgr.es/m/CAH2-Wz=D533JbF_ak_Pc8kP0FKse-ju8DnMxtjvY==yHsP4xgw@mail.gmail.com Backpatch-through: 19 Branch ------ REL_19_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/d2a710c7e9e08e101694fe6c1f6789f18f9d3dcf Modified Files -------------- src/backend/commands/trigger.c | 14 ++++ src/backend/utils/adt/ri_triggers.c | 106 +++++++++++++++++++++++------- src/include/commands/trigger.h | 1 + src/test/regress/expected/foreign_key.out | 92 ++++++++++++++++++++++++++ src/test/regress/sql/foreign_key.sql | 78 ++++++++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 6 files changed, 269 insertions(+), 23 deletions(-) ^ permalink raw reply [nested|flat] 4+ messages in thread
* pgsql: Track RI fast-path FK-check batches per firing cycle @ 2026-08-20 08:22 Amit Langote <amitlan@postgresql.org> 0 siblings, 0 replies; 4+ messages in thread From: Amit Langote @ 2026-08-20 08:22 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Track RI fast-path FK-check batches per firing cycle Commit 34a30786293 fixed an RI fast-path crash under nested C-level SPI by keeping batch-callback lists per after-trigger query depth. That fix was incomplete: the RI fast path still tracked callback registration with one global flag. Once an outer firing cycle had registered its callback, the flag suppressed registration for a nested cycle, leaving the nested batch to be handled by the outer callback, too late and with the wrong snapshot, potentially after the ResourceOwner holding its relations had gone away. Nor is per-depth callback registration sufficient while the cache is keyed only by constraint OID. If nested firing checks the same constraint, it reuses the outer entry, combining rows that must be checked in separate firing cycles. Key the cache by both constraint OID and query depth. Register a callback for each depth that creates an entry, and make ri_FastPathEndBatch() flush and release only entries belonging to the ending depth. Add AfterTriggerCurrentQueryDepth() so ri_triggers.c can obtain the current depth; depth -1 represents deferred firing. Add regression coverage for nested firing through a cursor portal, whose resources must not outlive the nested cycle, nested firing of the same constraint at different query depths, and deferred firing at query depth -1. Reported-by: Noah Misch <noah@leadboat.com> Reported-by: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/20260705222115.be.noahmisch@microsoft.com Discussion: https://postgr.es/m/CAH2-Wz=D533JbF_ak_Pc8kP0FKse-ju8DnMxtjvY==yHsP4xgw@mail.gmail.com Backpatch-through: 19 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/6fc2a486417d07f45ad47bc70d90a18902c304e5 Modified Files -------------- src/backend/commands/trigger.c | 14 ++++ src/backend/utils/adt/ri_triggers.c | 106 +++++++++++++++++++++++------- src/include/commands/trigger.h | 1 + src/test/regress/expected/foreign_key.out | 92 ++++++++++++++++++++++++++ src/test/regress/sql/foreign_key.sql | 78 ++++++++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 6 files changed, 269 insertions(+), 23 deletions(-) ^ permalink raw reply [nested|flat] 4+ messages in thread
* pgsql: Track RI fast-path FK-check batches per subtransaction @ 2026-08-22 07:23 Amit Langote <amitlan@postgresql.org> 0 siblings, 0 replies; 4+ messages in thread From: Amit Langote @ 2026-08-22 07:23 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Track RI fast-path FK-check batches per subtransaction Commit 4113873 confined RI fast-path batching to the top transaction level to avoid mishandling the batch cache on subtransaction abort. That disabled batching for a foreign-key load wrapped in a savepoint, such as: BEGIN; SAVEPOINT s; COPY fk_table FROM ... This was a surprising performance cliff and departed from the usual per-subtransaction resource handling. Track cache entries per subtransaction instead. Add AtEOSubXact_RI(), called from CommitSubTransaction() and AbortSubTransaction() after ResourceOwnerRelease(). On abort, it removes only entries opened by the ending subtransaction, whose resources have just been released, while leaving entries opened by an outer level intact. Thus, an inner subtransaction abort during outer-level trigger firing does not discard the outer statement's batch. On commit, no matching entry is expected because its batch should already have been flushed at statement end. Each entry records the subtransaction that opened its resources. After an abort, the remaining slot storage and per-entry flush contexts are reclaimed when TopTransactionContext is reset at top-level transaction end. A fast-path batch is filled and flushed within a single trigger-firing cycle, so every row added to an entry must come from the subtransaction that created it. AtEOSubXact_RI() relies on this invariant to identify an aborting subtransaction's entries by the subid stamped at entry creation. Assert the invariant in ri_FastPathBatchAdd(). Add regression coverage for batching during nested firing inside a subtransaction, both with different constraints and with the same constraint at the inner and outer firing levels. Reported-by: Noah Misch <noah@leadboat.com> Reported-by: Nikolay Samokhvalov <nik@postgres.ai> Discussion: https://postgr.es/m/20260705222115.be.noahmisch@microsoft.com Backpatch-through: 19 Branch ------ REL_19_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/268958a2e5feac370844e164a4e7e2535f6d2bc7 Modified Files -------------- src/backend/access/transam/xact.c | 2 + src/backend/utils/adt/ri_triggers.c | 94 ++++++++++++++++++++++++++++--- src/include/commands/trigger.h | 2 + src/test/regress/expected/foreign_key.out | 70 +++++++++++++++++++++++ src/test/regress/sql/foreign_key.sql | 57 +++++++++++++++++++ 5 files changed, 217 insertions(+), 8 deletions(-) ^ permalink raw reply [nested|flat] 4+ messages in thread
* pgsql: Track RI fast-path FK-check batches per subtransaction @ 2026-08-22 07:26 Amit Langote <amitlan@postgresql.org> 0 siblings, 0 replies; 4+ messages in thread From: Amit Langote @ 2026-08-22 07:26 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Track RI fast-path FK-check batches per subtransaction Commit 4113873 confined RI fast-path batching to the top transaction level to avoid mishandling the batch cache on subtransaction abort. That disabled batching for a foreign-key load wrapped in a savepoint, such as: BEGIN; SAVEPOINT s; COPY fk_table FROM ... This was a surprising performance cliff and departed from the usual per-subtransaction resource handling. Track cache entries per subtransaction instead. Add AtEOSubXact_RI(), called from CommitSubTransaction() and AbortSubTransaction() after ResourceOwnerRelease(). On abort, it removes only entries opened by the ending subtransaction, whose resources have just been released, while leaving entries opened by an outer level intact. Thus, an inner subtransaction abort during outer-level trigger firing does not discard the outer statement's batch. On commit, no matching entry is expected because its batch should already have been flushed at statement end. Each entry records the subtransaction that opened its resources. After an abort, the remaining slot storage and per-entry flush contexts are reclaimed when TopTransactionContext is reset at top-level transaction end. A fast-path batch is filled and flushed within a single trigger-firing cycle, so every row added to an entry must come from the subtransaction that created it. AtEOSubXact_RI() relies on this invariant to identify an aborting subtransaction's entries by the subid stamped at entry creation. Assert the invariant in ri_FastPathBatchAdd(). Add regression coverage for batching during nested firing inside a subtransaction, both with different constraints and with the same constraint at the inner and outer firing levels. Reported-by: Noah Misch <noah@leadboat.com> Reported-by: Nikolay Samokhvalov <nik@postgres.ai> Discussion: https://postgr.es/m/20260705222115.be.noahmisch@microsoft.com Backpatch-through: 19 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/e2c812f1475dc75ab6f8a39fb5696d8d32d05fa1 Modified Files -------------- src/backend/access/transam/xact.c | 2 + src/backend/utils/adt/ri_triggers.c | 94 ++++++++++++++++++++++++++++--- src/include/commands/trigger.h | 2 + src/test/regress/expected/foreign_key.out | 70 +++++++++++++++++++++++ src/test/regress/sql/foreign_key.sql | 57 +++++++++++++++++++ 5 files changed, 217 insertions(+), 8 deletions(-) ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-08-22 07:26 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-08-20 08:22 pgsql: Track RI fast-path FK-check batches per firing cycle Amit Langote <amitlan@postgresql.org> 2026-08-20 08:22 pgsql: Track RI fast-path FK-check batches per firing cycle Amit Langote <amitlan@postgresql.org> 2026-08-22 07:23 pgsql: Track RI fast-path FK-check batches per subtransaction Amit Langote <amitlan@postgresql.org> 2026-08-22 07:26 pgsql: Track RI fast-path FK-check batches per subtransaction Amit Langote <amitlan@postgresql.org>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox