Re: Revert RI fast-path batching from REL_19_STABLE

> On Sep 7, 2026, at 21:14, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> On Thu, Sep 3, 2026 at 10:36 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>> Attached is a two-patch series to remove batching from the RI fast
>> path in REL_19_STABLE, while retaining the per-row fast path. Batching
>> accumulates rows from successive foreign-key trigger invocations and
>> checks them together against the referenced index when the batch fills
>> or the trigger-firing cycle ends, instead of checking each row during
>> its trigger invocation. I plan to leave the batched implementation
>> unchanged in master for v20 development.
>>
>> Given the concerns raised about shipping this code in v19 [1], I
>> reviewed the fixes made since feature freeze. The batching fixes are
>> considerably more concerning than those to the underlying per-row fast
>> path, because they have repeatedly had to address how a live batch of
>> pending checks interacts with trigger-firing cycles, subtransactions,
>> deferred constraints, and SET CONSTRAINTS, rather than merely
>> correcting isolated implementation errors. Missing one such
>> interaction can leave a foreign key check buffered and never
>> performed, allowing a violating row to be committed without an error.
>> That fix history, together with the two still-open batching-related
>> items, has persuaded me that the concerns about shipping the batching
>> code in v19 are justified.
>>
>> 0001 is the same patch I posted separately at:
>>
>> https://postgr.es/m/CA+HiwqEhm+_=bs=2wavAJz-UqC+1KebD31++mapJQQGweE8iQQ@mail.gmail.com
>>
>> It fixes a snapshot-ordering bug in the per-row path that must be
>> addressed before 0002 removes batching and makes ordinary DML use that
>> path. In READ COMMITTED, ri_FastPathCheck() could take its scan
>> snapshot before waiting to lock the referenced relation. A referenced
>> row committed during that wait would not be visible to the old
>> snapshot, causing a false foreign key violation. Batching currently
>> masks this problem because it opens and caches the relations before
>> taking the snapshot used for the batch. The SPI path likewise acquires
>> the referenced-relation lock before selecting the snapshot used for
>> the check.
>
> I asked GPT-6 whether I had missed anything else about snapshot
> handling in the per-row fast path compared with SPI. It pointed out
> that the fast path did not make the scan snapshot active. SQL executed
> inside a STABLE user-defined cast could therefore use an older active
> snapshot and miss changes visible to the index scan. I confirmed this
> with a reproducer where the fast path rejected a valid foreign key but
> SPI accepted it. Updated 0001 also pushes/pops the snapshot and adds a
> regression test.
>
>> 0002 removes the batching layer, the per-batch RI cache, and the
>> after-trigger callback and subtransaction machinery added to manage
>> their lifetime. It restores the affected after-trigger code to its
>> pre-batching form.
>>
>> 0002 removes tests that exercise only the batching implementation and
>> its callback and cache lifetime machinery. It retains, and where
>> necessary adapts, tests that continue to exercise the underlying RI
>> cases through the per-row path, including ALTER TABLE validation,
>> scan-key construction, deferred checks, re-entrancy, and metadata
>> invalidation. This preserves useful regression coverage in v19 and
>> minimizes unnecessary divergence in test coverage, simplifying future
>> backpatching of test cases. When I first tested the batching removal,
>> the only failure among the retained tests was the injection-point test
>> that exposed the snapshot-ordering bug fixed by 0001.
>
> The cross-type recheck isolation test also exercises code retained in
> the per-row fast path, so the updated 0002 restores it.
>
>> The per-row path has also required fixes since feature freeze,
>> including the snapshot-ordering fix in 0001. So far, however, its fix
>> history has not shown the same pattern of complexity as the fixes
>> needed to manage the lifetime of batched checks. On that basis, I
>> propose retaining the per-row path in v19 while removing the batching
>> layer.
>>
>> Thoughts on removing batching while retaining the per-row fast path in
>> v19, and on retaining batching in its current state in master for v20
>> development, would be welcome.
>
> I'd like to push 0001 soon. I'll allow a few more days for comments
> before committing 0002.
>
> --
> Thanks, Amit Langote
> <v2-0001-Take-RI-fast-path-snapshot-after-locking-referenc.patch><v2-0002-Remove-batching-from-RI-fast-path-checks.patch>

Hi Amit,

I applied v2 to REL_19_STABLE and ran tests. The test failed because foreign_key.out contained 6 extra empty lines. After deleting them, the test passed.

Other than that, the patch looks good to me. For your convenience, I fixed the test in v3.

* 0001 - just deleted 6 empty lines from foreign_key.out
* 0002 - unchanged from v2.

By the way, I failed to apply v2 to master due to conflicts on release-19.sgml.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/