On Wed, Sep 9, 2026 at 2:59 AM Robert Haas <[email protected]> wrote: > On Thu, Sep 3, 2026 at 9:36 AM Amit Langote <[email protected]> wrote: > > 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. > > Does this approach have any significant downsides that we should be > thinking about? For example, are we relying on the batching to buy > back slowdowns that the per-row fast path might otherwise introduce in > some cases? Or is this just a case of the per-row fast path is an > optimization and then the batching is a further optimization, so if > the second one is buggy we can take it out without causing any > problems for the first one?
Yes, it's the latter. Batching is a further optimization on top of the per-row fast path. It wasn't introduced to compensate for a known slowdown caused by the per-row path. The per-row path avoids SPI’s plan-cache and executor overhead while performing each check synchronously. The proposed removal also gives up reuse of open relations and tuple slots. The original series had a separate patch that cached open relations and tuple slots while keeping checks synchronous. When adding batching, I decided to use the same cache entry to track both those resources and outstanding rows. The proposed v19 patch removes both. Note that the per-row path retained in v19 still caches lookup and comparison metadata attached to RI_ConstraintInfo, with its own invalidation and lifetime handling. The additional relation and slot cache holds open relation references and reusable tuple slots. These are normally released at the end of each trigger-firing cycle, with transaction and subtransaction abort cleanup covering interrupted teardown. > I do think I generally agree that the batching stuff feels much > riskier than the per-row fast path stuff. I think batching in this > context intrinsically requires changing the timing of trigger firing, > and that is risky because (1) important things may be different at the > two timings, such as the choice of snapshot, and (2) the change in > timing may be user-perceptible in some way. However, I'm not entirely > sure whether (1) batching is in good enough shape that it makes sense > to keep it in v20 or (2) the per-row fast-path is in good enough shape > to stay in v19. In other words, I think we should do at least as much > as what you're proposing here, but possibly more. However, I'm not > very sure what the right answer is at this point. Your concerns about delaying checks also make me reconsider retaining batching in master. How about this: * For v19, retain the per-row fast path and remove batching and the relation and slot cache, as currently proposed. I don't think there is time left to develop an intermediate resource-caching version. The remaining work should be fixing bugs in what we retain. * For master, remove batched probing while retaining resource caching for synchronous checks. Initially, keep the current resource lifetime: a multirow INSERT or UPDATE would reuse relations and slots across its FK trigger calls, then release them after the statement's queued AFTER triggers have finished firing. The same applies when deferred triggers are fired by SET CONSTRAINTS ... IMMEDIATE or at commit. Concretely, RI_FastPathEntry currently holds both open relations and reusable slots, and the pending-row buffer and state used for batch flushing. I would remove the pending-row storage and batch-flushing machinery, and have each trigger invocation complete its check synchronously using cached resources. Reusing slots would still require protection against re-entrant checks. The cleanup callback invoked after the queued AFTER triggers have finished firing would remain. It would only drop the cached slots and close the relation references, retaining the locks until transaction end. On transaction or subtransaction abort, ResourceOwner would continue to release tracked resources, and RI cleanup would discard the affected cache entries. Keeping this cleanup structure would preserve the current resource lifetime while removing the delayed checks. Any reintroduction of SK_SEARCHARRAY batching would require a separate proposal addressing user-visible changes from delaying checks, including their ordering relative to other AFTER ROW triggers. As Tomas Vondra also pointed out off-list, those triggers can change data read by an FK check or execute SQL that fires further triggers. Taking the right snapshot and ensuring that every batch gets flushed do not by themselves preserve those interactions. This separation would also make it easier to measure the performance benefit of resource reuse independently from batched probing, and assess each against its own complexity. That still leaves your concern about whether the per-row path is sufficiently solid for v19. The snapshot fixes and Amit's recent collation report show that there are differences from SPI that we (I) missed. The proposed collation fix falls back to SPI when the referenced index and column collations differ. I still propose retaining the per-row path, but agree that its readiness needs to be assessed separately from the decision to remove batching. -- Thanks, Amit Langote
