agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Jehan-Guillaume de Rorthais <[email protected]>
Subject: [PATCH v3 2/2] Error out if one iteration of non-direct DML affects more than one row on the foreign server
Date: Fri, 18 Jul 2025 16:52:38 +0200
When a foreign table points to a partitioned table or an inheritance
parent on the foreign server, a non-direct DML can affect multiple
rows when only one row is intended to be affected. This happens
because postgres_fdw uses only ctid to identify a row to work on.
Though ctid uniquely identifies a row in a single table, in a
partitioned table or in an inheritance hierarchy, there can be be
multiple rows, in different partitions, with the same ctid. So a DML
statement sent to the foreign server by postgres_fdw ends up affecting
more than one rows, only one of which is intended to be affected.
In such a case it's good to throw an error instead of corrupting
remote database with unwanted UPDATE/DELETEs. Subsequent commits will
try to fix this situation.
Author: Ashutosh Bapat <[email protected]>
Author: Kyotaro Horiguchi <[email protected]>
Rebased by Jehan-Guillaume de Rorthais <[email protected]>
---
.../postgres_fdw/expected/postgres_fdw.out | 26 ++++++++------
contrib/postgres_fdw/postgres_fdw.c | 36 +++++++++++++++----
2 files changed, 46 insertions(+), 16 deletions(-)
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 62019eaa881..b0ef54a2889 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -8984,10 +8984,11 @@ UPDATE fa SET aa = (CASE WHEN random() <= 1 THEN 'zzzz' ELSE NULL END) WHERE aa
(5 rows)
UPDATE fa SET aa = (CASE WHEN random() <= 1 THEN 'zzzz' ELSE NULL END) WHERE aa = 'aaa';
+ERROR: foreign server affected 2 rows when only one was expected
SELECT tableoid::regclass, ctid, * FROM fa;
- tableoid | ctid | aa
-----------+-------+------
- fa | (0,2) | zzzz
+ tableoid | ctid | aa
+----------+-------+-----
+ fa | (0,1) | aaa
fa | (0,1) | bbb
(2 rows)
@@ -9008,11 +9009,13 @@ DELETE FROM fa WHERE aa = (CASE WHEN random() <= 1 THEN 'aaa' ELSE 'bbb' END);
(6 rows)
DELETE FROM fa WHERE aa = (CASE WHEN random() <= 1 THEN 'aaa' ELSE 'bbb' END);
+ERROR: foreign server affected 2 rows when only one was expected
SELECT tableoid::regclass, ctid, * FROM fa;
- tableoid | ctid | aa
+ tableoid | ctid | aa
----------+-------+-----
+ fa | (0,1) | aaa
fa | (0,1) | bbb
-(1 row)
+(2 rows)
-- cleanup
DROP FOREIGN TABLE fa;
@@ -9048,10 +9051,11 @@ UPDATE fplt SET b = (CASE WHEN random() <= 1 THEN 10 ELSE 20 END) WHERE a = 1;
(5 rows)
UPDATE fplt SET b = (CASE WHEN random() <= 1 THEN 10 ELSE 20 END) WHERE a = 1;
+ERROR: foreign server affected 2 rows when only one was expected
SELECT tableoid::regclass, ctid, * FROM fplt;
- tableoid | ctid | a | b
-----------+-------+---+----
- fplt | (0,2) | 1 | 10
+ tableoid | ctid | a | b
+----------+-------+---+---
+ fplt | (0,1) | 1 | 1
fplt | (0,1) | 2 | 2
(2 rows)
@@ -9071,11 +9075,13 @@ DELETE FROM fplt WHERE a = (CASE WHEN random() <= 1 THEN 1 ELSE 10 END);
(6 rows)
DELETE FROM fplt WHERE a = (CASE WHEN random() <= 1 THEN 1 ELSE 10 END);
+ERROR: foreign server affected 2 rows when only one was expected
SELECT tableoid::regclass, ctid, * FROM fplt;
tableoid | ctid | a | b
----------+-------+---+---
- fplt | (0,1) | 2 | 2
-(1 row)
+ fplt | (0,1) | 1 | 1
+ fplt | (0,1) | 2 | 2
+(2 rows)
DROP TABLE plt;
DROP FOREIGN TABLE fplt;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index e0a34b27c7c..09c87d0e5d8 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -4132,7 +4132,8 @@ execute_foreign_modify(EState *estate,
ItemPointer ctid = NULL;
const char **p_values;
PGresult *res;
- int n_rows;
+ int n_rows_returned;
+ int n_rows_affected;
StringInfoData sql;
/* The operation should be INSERT, UPDATE, or DELETE */
@@ -4213,27 +4214,50 @@ execute_foreign_modify(EState *estate,
pgfdw_report_error(ERROR, res, fmstate->conn, true, fmstate->query);
/* Check number of rows affected, and fetch RETURNING tuple if any */
+ n_rows_affected = atoi(PQcmdTuples(res));
if (fmstate->has_returning)
{
Assert(*numSlots == 1);
- n_rows = PQntuples(res);
- if (n_rows > 0)
+ n_rows_returned = PQntuples(res);
+ if (n_rows_returned > 0)
store_returning_result(fmstate, slots[0], res);
+
+ // FIXME: shouldn't we check the max number of rows returned is one?
}
else
- n_rows = atoi(PQcmdTuples(res));
+ n_rows_returned = 0;
/* And clean up */
PQclear(res);
MemoryContextReset(fmstate->temp_cxt);
- *numSlots = n_rows;
+ /*
+ * UPDATE & DELETE command can only affect one row, make sure this contract
+ * is respected.
+ * CMD_INSERT can insert multiple row when called from ForeignBatchInsert.
+ */
+ if (operation != CMD_INSERT)
+ {
+ /* No rows should be returned if no rows were affected */
+ if (n_rows_affected == 0 && n_rows_returned != 0)
+ elog(ERROR, "foreign server returned %d rows when no row was affected",
+ n_rows_returned);
+
+ /* ERROR if more than one row was updated on the remote end */
+ if (n_rows_affected > 1)
+ ereport(ERROR,
+ (errcode (ERRCODE_FDW_ERROR), /* XXX */
+ errmsg ("foreign server affected %d rows when only one was expected",
+ n_rows_affected)));
+ }
+
+ *numSlots = n_rows_returned;
/*
* Return NULL if nothing was inserted/updated/deleted on the remote end
*/
- return (n_rows > 0) ? slots : NULL;
+ return (n_rows_affected > 0) ? slots : NULL;
}
/*
--
2.50.0
--MP_/4ZRYdF7Ah.pt5w65PuZdaPz--
view thread (816+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v3 2/2] Error out if one iteration of non-direct DML affects more than one row on the foreign server
In-Reply-To: <no-message-id-220156@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox