Re: Revert RI fast-path batching from REL_19_STABLE

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