Hi, On Sat, Jul 19, 2025 at 12:53 AM Jehan-Guillaume de Rorthais <j...@dalibo.com> wrote: > We have been bitten by this old bug recently: > > https://www.postgresql.org/message-id/flat/CAFjFpRfcgwsHRmpvoOK-GUQi-n8MgAS%2BOxcQo%3DaBDn1COywmcg%40mail.gmail.com > > I suspect this bug lost attention when the only fixed submitted to the > commitfest has been flagged as "returned with feedback": > > https://commitfest.postgresql.org/patch/1819/
This is on my TODO list, but I didn't have time to work on it, unfortunately.
> Please, find in attachment the old patch forbidding more than one row to be
> deleted/updated from postgresExecForeignDelete and postgresExecForeignUpdate.
> I
> just rebased them without modification. I suppose this is much safer than
> leaving the FDW destroy arbitrary rows on the remote side based on their sole
> ctid.
Thanks for rebasing the patch set, but I don't think the idea
implemented in the second patch is safe; even with the patch we have:
create table plt (a int, b int) partition by list (a);
create table plt_p1 partition of plt for values in (1);
create table plt_p2 partition of plt for values in (2);
create function trig_null() returns trigger language plpgsql as
$$ begin return null; end; $$;
create trigger trig_null before update or delete on plt_p1
for each row execute function trig_null();
create foreign table fplt (a int, b int)
server loopback options (table_name 'plt');
insert into fplt values (1, 1), (2, 2);
INSERT 0 0
select tableoid::regclass, ctid, * from plt;
tableoid | ctid | a | b
----------+-------+---+---
plt_p1 | (0,1) | 1 | 1
plt_p2 | (0,1) | 2 | 2
(2 rows)
explain verbose update fplt set b = (case when random() <= 1 then 10
else 20 end) where a = 1;
QUERY PLAN
-------------------------------------------------------------------------------------------------
Update on public.fplt (cost=100.00..128.02 rows=0 width=0)
Remote SQL: UPDATE public.plt SET b = $2 WHERE ctid = $1
-> Foreign Scan on public.fplt (cost=100.00..128.02 rows=7 width=42)
Output: CASE WHEN (random() <= '1'::double precision) THEN 10
ELSE 20 END, ctid, fplt.*
Remote SQL: SELECT a, b, ctid FROM public.plt WHERE ((a = 1))
FOR UPDATE
(5 rows)
update fplt set b = (case when random() <= 1 then 10 else 20 end) where a = 1;
UPDATE 1
select tableoid::regclass, ctid, * from plt;
tableoid | ctid | a | b
----------+-------+---+----
plt_p1 | (0,1) | 1 | 1
plt_p2 | (0,2) | 2 | 10
(2 rows)
The row in the partition plt_p2 is updated, which is wrong as the row
doesn't satisfy the query's WHERE condition.
> Or maybe we should just not support foreign table to reference a
> remote partitioned table?
I don't think so because we can execute SELECT, INSERT, and direct
UPDATE/DELETE safely on such a foreign table.
I think a simple fix for this is to teach the system that the foreign
table is a partitioned table; in more detail, I would like to propose
to 1) add to postgres_fdw a table option, inherited, to indicate
whether the foreign table is a partitioned/inherited table or not, and
2) modify postgresPlanForeignModify() to throw an error if the given
operation is an update/delete on such a foreign table. Attached is a
WIP patch for that. I think it is the user's responsibility to set
the option properly, but we could modify postgresImportForeignSchema()
to support that. Also, I think this would be back-patchable.
What do you think about that?
Best regards,
Etsuro Fujita
postgres_fdw-disallow-upddel-in-problematic-cases-wip.patch
Description: Binary data
