On Tue, Mar 24, 2026 at 10:56 PM Amit Langote <[email protected]> wrote: > On Tue, Mar 24, 2026 at 8:47 PM Amit Langote <[email protected]> wrote: > > > > Hi Junwang, > > > > On Fri, Mar 20, 2026 at 1:20 AM Junwang Zhao <[email protected]> wrote: > > > I squashed 0004 into 0003 so that each file can be committed > > > independently. > > > I also runned pgindent for each file. > > > > Thanks for that. > > > > Here's another version. > > > > In 0001, I noticed that the condition change in ri_HashCompareOp could > > be simplified further. Also improved the commentary surrounding that. > > I also updated the commit message to clarify parity with the SPI path. > > > > Updated the commit message of 0002 to talk about why caching the > > snapshot for the entire trigger firing cycle of a given constraint > > makes a trade off compared to the SPI path which retakes the snapshot > > for every row checked and could in principle avoid failure for FK rows > > whose corresponding PK row was added by a concurrently committed > > transaction, at least in the READ COMMITTED case. > > > > Updated the commit message of 0003 to clarify that it replaces > > ri_FastPathCheckCached() from 0002 with the BatchAdd/BatchFlush pair, > > and that the cached resources are used unchanged -- only the probing > > cadence changes from per-row to per-flush. Per-flush CCI is safe > > because all AFTER triggers for the buffered rows have already fired > > by flush time; a new test case is added to show that. > > Kept thinking about this on a walk after I sent this and came to the > conclusion that it might be better to just not cache the snapshot with > only the above argument in its favor. If repeated GetSnapshotData() > is expensive, the solution should be to fix that instead of simply > side-stepping it. > > By taking a snapshot per-batch without caching it, and so likewise the > IndexScanDesc, I'm seeing the same ~3x speedup in the batched > SK_SEARCHARRAY case, so I don't see much point in being very stubborn > about snapshot caching. Like in the attached (there's an unrelated > memory context switch thinko fix). Note that relations (pk_rel, > idx_rel) and the slot remain cached across the batch; only the > snapshot and scandesc are taken fresh per flush. > > I'll post an updated version tomorrow morning. I think it might be > better to just merge 0003 into 0002, because without snapshot and > scandesc caching the standalone value of 0002 is mostly just relation > and slot caching -- the interesting parts (batch callbacks, lifecycle > management) are all scaffolding for the batching. So v10 will be two > patches: 0001 core fast path, 0002 everything else.
And here's a set like that. I noticed that we don't need a dedicated scan_cxt now that scandesc is not cached and a few other simplifications. -- Thanks, Amit Langote
v10-0002-Batch-FK-rows-and-use-SK_SEARCHARRAY-for-fast-pa.patch
Description: Binary data
v10-0001-Add-fast-path-for-foreign-key-constraint-checks.patch
Description: Binary data
