| From: | Tomas Vondra <tomas(at)vondra(dot)me> |
|---|---|
| To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | improvements / reworks for FDW insert batching |
| Date: | 2026-11-12 19:23:38 |
| Message-ID: | 0cde668e-c225-4ec5-81ba-cf946c81028e@vondra.me |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
While working on one of the fixed for the new releases [1], it became
clear the FDW insert batching (introduced in PG 14) has a couple weak
points that we should address. The fix [1] is merely a stopgap, this
patch series is an attempt to do that properly.
First, the API leaves the decision whether to enable batching completely
up to the FDW. The FDW can enable batching even in cases when it's not
really correct - for example with AFTER ROW triggers, when we need to
know which individual rows were enabled (there are multiple such cases).
In other cases (BEFORE ROW triggers) the FDW may enable batching, but
the executor ends up flushing the buffer before each row, making the
batching entirely useless.
These are just examples, but I think it's very clear most of this
decision should happen in core code. The FDW should have a say in what
batch size it prefers, but it's up to the core to decide if batching can
be enabled for the query.
Then there is the question in which cases it's actually correct to
enable insert batching. We can't really enable batching for cases that
need to know the new tuples, like AFTER ROW triggers, or even AFTER
STATEMENT triggers with transition tables.
The old check disabled some of these, but not all of them. Before the
fix this might cause crashes, now it means the query can silently drop
the new tuples, run triggers on different tuples, etc. There's probably
multiple different ways how it could go wrong.
0001 addresses both of these issues. It moves the batch size decision
from postgres_fdw.c to core, and tightens the checks when to allow
batching. It's not very pretty, because it copies the check into three
different places, but 0002 addresses that.
0002 simply replaces the duplicated check with a simple helper, called
from all those places.
0003 relaxes the conditions a little bit, by allowing batching in cases
that need only OLD TABLE transition tables.
Ultimately, all of these patches should be squashed into a single
change. I kept them separate to make review easier.
I wonder if maybe there are additional cases where we need to disable
batching (or where we could enable it, similarly to 0003). It's quite
possible I failed to consider something.
I also am not entirely sure whether we should backpatch this (either in
it's entirety, or some subset of the changes). I believe it's harmless,
as it can only slow down cases where the behavior is likely incorrect.
OTOH I don't recall any reports about issues with this, so chances are
people are not using it with ROW triggers etc. (But then it also should
not get slower for them.)
regards
[1]
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=1e0a55a0f
--
Tomas Vondra
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0003-Allow-batching-with-OLD-TABLE-transition-tables.patch | text/x-patch | 8.8 KB |
| v1-0002-Move-the-batching-decision-for-foreign-tables-to-.patch | text/x-patch | 12.1 KB |
| v1-0001-Rework-batching-decision-for-foreign-tables.patch | text/x-patch | 32.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | João Rito | 2026-11-12 19:19:11 | Re: Re: Assertion failure in update_local_synced_slot(), slotsync.c:364 |