Hi Junwang, Thanks for sending the new version.
On Tue, Mar 10, 2026 at ... Junwang Zhao <[email protected]> wrote: > 1. > Move ri_ReportViolation into ri_FastPathCheck, so table_open is no > longer needed, and ri_FastPathCheck now returns void. Good, kept. > 2. > After adding the batch fast path, the original ri_FastPathCheck is only > used by the ALTER TABLE validation path. This path cannot use the > cache because the registered AfterTriggerBatch callback will never run. > Therefore, the use_cache branch can be removed. Agreed. I went a step further and restructured 0002 to avoid the use_cache branching entirely. Instead of adding if/else blocks to ri_FastPathCheck, 0002 now adds a separate ri_FastPathCheckCached() function with its own resource lifecycle. 0003 then replaces it with ri_FastPathBatchAdd() -- a clean swap rather than completely undoing what 0002 added. This also removes the use_cache parameter from ri_FastPathProbeOne; the memory context switch to TopTransactionContext is now the caller's responsibility. > 3. > ri_FastPathBatchFlush creates a new fk_slot but does not cache it in > RI_FastPathEntry. I tried caching it in v5-0006 and ran some benchmarks, > it didn't show much improvement. I put the fk_slot in the cache entry since it's a small change. > 4. > ri_FastPathFlushArray currently uses SK_SEARCHARRAY only for > single-column checks. [...] my current understanding is that > SK_SEARCHARRAY may not work for multi-column checks. Right, I haven't investigated this deeply either. The FlushLoop fallback is the right approach for now. If we want to explore a SEARCHARRAY approach for multi-column keys in a follow-up, it would be worth checking with Peter Geoghegan or someone else familiar with the btree SAOP internals on how multiple array keys across columns are iterated and whether that's usable at all for this use case. Attached is v6, three patches -- combined the old 0003 (buffering) and 0004 (SK_SEARCHARRAY) into a single 0003, since the buffering alone has no performance benefit (or at least only minor) and the split added unnecessary diff/rebase churn. The biggest change in this version is the snapshot handling. Looking more carefully at what the SPI path actually does for RI_FKey_check (non-partitioned PK, detectNewRows = false), I found that ri_PerformCheck passes InvalidSnapshot to SPI_execute_snapshot, and _SPI_execute_plan ends up doing PushActiveSnapshot(GetTransactionSnapshot()). So the SPI path scans with the transaction snapshot, not the latest snapshot. So I've changed the fast path to match: ri_FastPathCheck now uses GetTransactionSnapshot() directly. Under READ COMMITTED this is a fresh snapshot; under REPEATABLE READ it's the frozen transaction-start snapshot, so PK rows committed after the transaction started are simply not visible. This means the second index probe (the IsolationUsesXactSnapshot crosscheck block) is no longer needed and is removed. The existing fk-snapshot isolation test confirms this is the correct behavior. Other changes since v5: * Fixed the batch callback firing during nested SPI: another AFTER trigger doing DML via SPI would call AfterTriggerEndQuery at a nested level, tearing down our cache mid-batch. Fixed by checking query_depth inside FireAfterTriggerBatchCallbacks. Added a test case with a trigger that auto-provisions PK rows via SPI. * Security context is now restored before ri_ReportViolation, not after (the ereport doesn't return). * search_vals[] and matched[] moved from RI_FastPathEntry to stack locals in ri_FastPathFlushArray -- they're rewritten from scratch on every flush with no state carried between calls. * Various comment fixes. I think this is getting close to committable shape. That said, another pair of eyes would be reassuring before I pull the trigger. Tomas, if you've had a chance to look, would welcome your thoughts. -- Thanks, Amit Langote
v6-0003-Batch-FK-rows-and-use-SK_SEARCHARRAY-for-fast-pat.patch
Description: Binary data
v6-0001-Add-fast-path-for-foreign-key-constraint-checks.patch
Description: Binary data
v6-0002-Cache-per-batch-resources-for-fast-path-foreign-k.patch
Description: Binary data
