Import batch-callback API from upstream (partial) - #22
Open
chahatagrawal117 wants to merge 5 commits into
Open
Conversation
chahatagrawal117
changed the title
chahatagrawal117
force-pushed
the
2026_pg_batch_callback_api
branch
from
August 18, 2026 14:40
819b263 to
942cf74
Compare
jasonyb
requested changes
Aug 18, 2026
jasonyb
left a comment
jasonyb
left a comment
There was a problem hiding this comment.
You only have a single commit. This is not in the format requested by docs.yugabyte.com/stable/contribute/core-database/merge-with-upstream-repositories#squash-point-imports.
Instead of probing the PK index on each trigger invocation, buffer FK rows in a new per-constraint cache entry (RI_FastPathEntry) and flush them as a batch. On each trigger invocation, the new ri_FastPathBatchAdd() buffers the FK row in RI_FastPathEntry. When the buffer fills (64 rows) or the trigger-firing cycle ends, the new ri_FastPathBatchFlush() probes the index for all buffered rows, sharing a single CommandCounterIncrement, snapshot, permission check, and security context switch across the batch, rather than repeating each per row as the SPI path does. Per-flush CCI is safe because all AFTER triggers for the buffered rows have already fired by flush time. For single-column foreign keys, the new ri_FastPathFlushArray() builds an ArrayType from the buffered FK values (casting to the PK-side type if needed) and constructs a scan key with the SK_SEARCHARRAY flag. The index AM sorts and deduplicates the array internally, then walks matching leaf pages in one ordered traversal instead of descending from the root once per row. A matched[] bitmap tracks which batch items were satisfied; the first unmatched item is reported as a violation. Multi-column foreign keys fall back to per-row probing via the new ri_FastPathFlushLoop(). The fast path introduced in the previous commit (2da86c1) yields ~1.8x speedup. This commit adds ~1.6x on top of that, for a combined ~2.9x speedup over the unpatched code (int PK / int FK, 1M rows, PK table and index cached in memory). FK tuples are materialized via ExecCopySlotHeapTuple() into a new purpose-specific memory context (flush_cxt), child of TopTransactionContext, which is also used for per-flush transient work: cast results, the search array, and index scan allocations. It is reset after each flush and deleted in teardown. The PK relation, index, tuple slots, and fast-path metadata are cached in RI_FastPathEntry across trigger invocations within a trigger-firing batch, avoiding repeated open/close overhead. The snapshot and IndexScanDesc are taken fresh per flush. The entry is not subject to cache invalidation: cached relations are held with locks for the transaction duration, and the entry's lifetime is bounded by the trigger-firing cycle. Lifecycle management for RI_FastPathEntry relies on three new mechanisms: - AfterTriggerBatchCallback: A new general-purpose callback mechanism in trigger.c. Callbacks registered via RegisterAfterTriggerBatchCallback() fire at the end of each trigger-firing batch (AfterTriggerEndQuery for immediate constraints, AfterTriggerFireDeferred at COMMIT, and AfterTriggerSetState for SET CONSTRAINTS IMMEDIATE). The RI code registers ri_FastPathEndBatch as a batch callback. - Batch callbacks only fire at the outermost query level (checked inside FireAfterTriggerBatchCallbacks), so nested queries from SPI inside other AFTER triggers do not tear down the cache mid-batch. - XactCallback: ri_FastPathXactCallback NULLs the static cache pointer at transaction end, handling the abort path where the batch callback never fired. - SubXactCallback: ri_FastPathSubXactCallback NULLs the static cache pointer on subtransaction abort, preventing the batch callback from accessing already-released resources. - AfterTriggerBatchIsActive(): A new exported accessor that returns true when afterTriggers.query_depth >= 0. During ALTER TABLE ... ADD FOREIGN KEY validation, RI triggers are called directly outside the after-trigger framework, so batch callbacks would never fire. The fast-path code uses this to fall back to the non-cached per-invocation path in that context. ri_FastPathEndBatch() flushes any partial batch before tearing down cached resources. Since the FK relation may already be closed by flush time (e.g. for deferred constraints at COMMIT), it reopens the relation using entry->fk_relid if needed. The existing ALTER TABLE validation path bypasses batching and continues to call ri_FastPathCheck() directly per row, because RI triggers are called outside the after-trigger framework there and batch callbacks would never fire to flush the buffer. Suggested-by: David Rowley <dgrowleyml@gmail.com> Author: Amit Langote <amitlangote09@gmail.com> Co-authored-by: Junwang Zhao <zhjwpku@gmail.com> Reviewed-by: Haibo Yan <tristan.yim@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Tested-by: Tomas Vondra <tomas@vondra.me> Discussion: https://postgr.es/m/CA+HiwqF4C0ws3cO+z5cLkPuvwnAwkSp7sfvgGj3yQ=Li6KNMqA@mail.gmail.com (cherry picked from commit b7b27eb) Partial import: skipped src/backend/utils/adt/ri_triggers.c changes (upstream's fast-path FK check code, ~600 lines) and the accompanying regression tests in src/test/regress/{sql,expected}/foreign_key.{sql,out}. Also skipped the RI_FastPathEntry typedef. yugabyte-db D55985 doesn't use those; they can be a follow-up point-import if YB later wants the fast-path FK check optimization.
That commit introduced AfterTriggerIsActive() to detect whether we are inside the after-trigger firing machinery, so that RI trigger functions can take the batched fast path. It was implemented using query_depth >= 0, which correctly identified immediate trigger firing but missed the deferred case where query_depth is -1 at COMMIT via AfterTriggerFireDeferred(). This caused deferred FK checks to fall back to the per-row fast path instead of the batched path. The correct check is whether we are inside an after-trigger firing loop specifically. Introduce afterTriggerFiringDepth, a counter incremented around the trigger-firing loops in AfterTriggerEndQuery, AfterTriggerFireDeferred, and AfterTriggerSetState, and decremented after FireAfterTriggerBatchCallbacks() returns. AfterTriggerIsActive() now returns afterTriggerFiringDepth > 0. Reported-by: Chao Li <li.evan.chao@gmail.com> Author: Chao Li <li.evan.chao@gmail.com> Co-authored-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/C2133B47-79CD-40FF-B088-02D20D654806@gmail.com (cherry picked from commit 5c54c3e)
When a C-language function uses SPI_connect/SPI_execute/SPI_finish to
INSERT into a table with FK constraints, the FK AFTER triggers fire
and schedule ri_FastPathEndBatch via
RegisterAfterTriggerBatchCallback(), opening PK relations under
CurrentResourceOwner at the time of the SPI call. The query_depth > 0
guard in FireAfterTriggerBatchCallbacks suppresses the callback at
that nesting level, deferring teardown to the outer query's
AfterTriggerEndQuery. By then the resource owner active during the SPI
call may have been released, decrementing the cached relations'
refcounts to zero. ri_FastPathTeardown, running under the outer
query's resource owner, then crashes in assert builds when it attempts
to close relations whose refcounts are already zero:
TRAP: failed Assert("rel->rd_refcnt > 0")
Fix by storing batch callbacks at the level where they should fire:
in AfterTriggersQueryData.batch_callbacks for immediate constraints
(fired by AfterTriggerEndQuery) and in AfterTriggersData.batch_callbacks
for deferred constraints (fired by AfterTriggerFireDeferred and
AfterTriggerSetState). RegisterAfterTriggerBatchCallback() routes the
callback to the current query-level list when query_depth >= 0, and to
the top-level list otherwise. FireAfterTriggerBatchCallbacks() takes a
list parameter and simply iterates and invokes it; memory cleanup is
handled by the caller. This replaces the query_depth > 0 guard with
list-level scoping. Note that deferred constraints are unaffected by
this bug: their callbacks fire at commit via AfterTriggerFireDeferred,
under the outer transaction's resource owner, which remains valid
throughout.
Also add firing_batch_callbacks to AfterTriggersData to enforce that
callbacks do not register new callbacks during
FireAfterTriggerBatchCallbacks(), which would be unsafe as it could
modify the list being iterated. An Assert in
RegisterAfterTriggerBatchCallback() enforces this discipline for
future callers. The flag is reset at transaction and subtransaction
boundaries to handle cases where an error thrown by a callback is
caught and the subtransaction is rolled back.
While at it, ensure callbacks are properly accounted for at all
transaction boundaries, as cleanup of b7b27eb: discard any
remaining top-level callbacks on both commit and abort in
AfterTriggerEndXact(), and clean up query-level callbacks in
AfterTriggerFreeQuery().
Note that ri_PerformCheck() calls SPI with fire_triggers=false, which
skips AfterTriggerBeginQuery/EndQuery for that SPI command. Any
triggers queued during that SPI command are not fired immediately but
deferred to the outer query level. Since the fast-path check for
those triggers runs under the outer query's resource owner rather than
a nested SPI resource owner, and ri_PerformCheck() does not create
a dedicated child resource owner, the bug described above does not
apply.
Reported-by: Evan Montgomery-Recht <montge@mianetworks.net>
Reported-by: Sandro Santilli <strk@kbt.io>
Analyzed-by: Evan Montgomery-Recht <montge@mianetworks.net>
Author: Amit Langote <amitlangote09@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CAEg7pwcKf01FmDqFAf-Hzu_pYnMYScY_Otid-pe9uw3BJ6gq9g@mail.gmail.com
(cherry picked from commit 34a3078)
Reported-by: jie wang <jugierwang@gmail.com> Reported-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/CAJnZyeDyaS=X-eYN=9rDYqK=6ma1gMLa0qDgfNbZKK0e0+q99Q@mail.gmail.com (cherry picked from commit 0302940)
The static variable afterTriggerFiringDepth introduced by commit 5c54c3e is logically part of the after-trigger state and in hindsight should have been a field in AfterTriggersData alongside query_depth and the other per-transaction after-trigger state. Move it there as firing_depth. Also update its comment to accurately reflect its sole remaining purpose: signaling to AfterTriggerIsActive() that after-trigger firing is active. Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/CA+HiwqFt4NGTNk7BinOsHHM48E9zGAa852vCfGoSe1bbL=JNFQ@mail.gmail.com (cherry picked from commit d6e96ba)
chahatagrawal117
force-pushed
the
2026_pg_batch_callback_api
branch
from
August 19, 2026 13:47
942cf74 to
a0cee04
Compare
Author
|
Split into 5 cherry-pick commits as per the docs. Each commit has the skipped-files note in its message. PTAL. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partial point-import of the upstream postgres batch-callback API chain, needed by yugabyte-db D55985 for cascade DELETE batching.
Each upstream commit is a separate
git cherry-pick -xon top ofyb-pg15.Upstream commits:
b7b27eb41a5c5c54c3ed1b9334a30786293003029409b409d6e96bacd3c0Imported
Batch-callback API infrastructure. Three files:
src/backend/commands/trigger.csrc/include/commands/trigger.hsrc/tools/pgindent/typedefs.listSkipped and why
src/backend/utils/adt/ri_triggers.c— upstream's own use of the API for fast-path FK check batching (RI_FastPathEntrycache,ri_FastPathBatchAdd,ri_FastPathCheck, and associated helpers). Not used by D55985.src/test/regress/sql/foreign_key.sql,src/test/regress/expected/foreign_key.out— regression tests for the fast-path FK check.RI_FastPathEntrytypedef insrc/tools/pgindent/typedefs.list.D55985 uses this API for cascade DELETE batching only, not for FK insert-check batching. A follow-up point-import can bring in the fast-path FK check code if YB later wants that optimization. Each of the 5 commit messages documents the same skip list.
Conflict resolution
Only the first cherry-pick (
b7b27eb41a5c) had conflicts. Resolved by dropping the skipped files (ri_triggers.c,foreign_key.sql/.out) and theRI_FastPathEntrytypedef entry. Cherry-picks 2–5 applied cleanly.Testing