Skip to content

Commit 7294f99

Browse files
tglsfdctglsfdc
committed
In postgres_fdw, don't try to ship MULTIEXPR updates to remote server.
In a statement like "UPDATE remote_tab SET (x,y) = (SELECT ...)", we'd conclude that the statement could be directly executed remotely, because the sub-SELECT is in a resjunk tlist item that's not examined for shippability. Currently that ends up crashing if the sub-SELECT contains any remote Vars. Prevent the crash by deeming MULTIEXEC Params to be unshippable. This is a bit of a brute-force solution, since if the sub-SELECT *doesn't* contain any remote Vars, the current execution technology would work; but that's not a terribly common use-case for this syntax, I think. In any case, we generally don't try to ship sub-SELECTs, so it won't surprise anybody that this doesn't end up as a remote direct update. I'd be inclined to see if that general limitation can be fixed before worrying about this case further. Per report from Lukáš Sobotka. Back-patch to 9.6. 9.5 had MULTIEXPR, but we didn't try to perform remote direct updates then, so the case didn't arise anyway. Discussion: https://postgr.es/m/CAJif3k+iA_ekBB5Zw2hDBaE1wtiQa4LH4_JUXrrMGwTrH0J01Q@mail.gmail.com
1 parent c4c76d1 commit 7294f99

File tree

3 files changed

+67
-0
lines changed

3 files changed

+67
-0
lines changed

‎contrib/postgres_fdw/deparse.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,22 @@ foreign_expr_walker(Node *node,
392392
{
393393
Param *p = (Param *) node;
394394

395+
/*
396+
* If it's a MULTIEXPR Param, punt. We can't tell from here
397+
* whether the referenced sublink/subplan contains any remote
398+
* Vars; if it does, handling that is too complicated to
399+
* consider supporting at present. Fortunately, MULTIEXPR
400+
* Params are not reduced to plain PARAM_EXEC until the end of
401+
* planning, so we can easily detect this case. (Normal
402+
* PARAM_EXEC Params are safe to ship because their values
403+
* come from somewhere else in the plan tree; but a MULTIEXPR
404+
* references a sub-select elsewhere in the same targetlist,
405+
* so we'd be on the hook to evaluate it somehow if we wanted
406+
* to handle such cases as direct foreign updates.)
407+
*/
408+
if (p->paramkind == PARAM_MULTIEXPR)
409+
return false;
410+
395411
/*
396412
* Collation rule is same as for Consts and non-foreign Vars.
397413
*/

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5437,6 +5437,37 @@ DELETE FROM ft2
54375437
(10 rows)
54385438

54395439
DELETE FROM ft2 WHERE ft2.c1 > 1200;
5440+
-- Test UPDATE with a MULTIEXPR sub-select
5441+
-- (maybe someday this'll be remotely executable, but not today)
5442+
EXPLAIN (verbose, costs off)
5443+
UPDATE ft2 AS target SET (c2, c7) = (
5444+
SELECT c2 * 10, c7
5445+
FROM ft2 AS src
5446+
WHERE target.c1 = src.c1
5447+
) WHERE c1 > 1100;
5448+
QUERY PLAN
5449+
---------------------------------------------------------------------------------------------------------------------------------------------------
5450+
Update on public.ft2 target
5451+
Remote SQL: UPDATE "S 1"."T 1" SET c2 = $2, c7 = $3 WHERE ctid = $1
5452+
-> Foreign Scan on public.ft2 target
5453+
Output: target.c1, $1, NULL::integer, target.c3, target.c4, target.c5, target.c6, $2, target.c8, (SubPlan 1 (returns $1,$2)), target.ctid
5454+
Remote SQL: SELECT "C 1", c3, c4, c5, c6, c8, ctid FROM "S 1"."T 1" WHERE (("C 1" > 1100)) FOR UPDATE
5455+
SubPlan 1 (returns $1,$2)
5456+
-> Foreign Scan on public.ft2 src
5457+
Output: (src.c2 * 10), src.c7
5458+
Remote SQL: SELECT c2, c7 FROM "S 1"."T 1" WHERE (($1::integer = "C 1"))
5459+
(9 rows)
5460+
5461+
UPDATE ft2 AS target SET (c2, c7) = (
5462+
SELECT c2 * 10, c7
5463+
FROM ft2 AS src
5464+
WHERE target.c1 = src.c1
5465+
) WHERE c1 > 1100;
5466+
UPDATE ft2 AS target SET (c2) = (
5467+
SELECT c2 / 10
5468+
FROM ft2 AS src
5469+
WHERE target.c1 = src.c1
5470+
) WHERE c1 > 1100;
54405471
-- Test UPDATE/DELETE with WHERE or JOIN/ON conditions containing
54415472
-- user-defined operators/functions
54425473
ALTER SERVER loopback OPTIONS (DROP extensions);

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,6 +1178,26 @@ DELETE FROM ft2
11781178
RETURNING 100;
11791179
DELETE FROM ft2 WHERE ft2.c1 > 1200;
11801180

1181+
-- Test UPDATE with a MULTIEXPR sub-select
1182+
-- (maybe someday this'll be remotely executable, but not today)
1183+
EXPLAIN (verbose, costs off)
1184+
UPDATE ft2 AS target SET (c2, c7) = (
1185+
SELECT c2 * 10, c7
1186+
FROM ft2 AS src
1187+
WHERE target.c1 = src.c1
1188+
) WHERE c1 > 1100;
1189+
UPDATE ft2 AS target SET (c2, c7) = (
1190+
SELECT c2 * 10, c7
1191+
FROM ft2 AS src
1192+
WHERE target.c1 = src.c1
1193+
) WHERE c1 > 1100;
1194+
1195+
UPDATE ft2 AS target SET (c2) = (
1196+
SELECT c2 / 10
1197+
FROM ft2 AS src
1198+
WHERE target.c1 = src.c1
1199+
) WHERE c1 > 1100;
1200+
11811201
-- Test UPDATE/DELETE with WHERE or JOIN/ON conditions containing
11821202
-- user-defined operators/functions
11831203
ALTER SERVER loopback OPTIONS (DROP extensions);

0 commit comments

Comments
 (0)