Skip to content

Commit 2099b91

Browse files
robertmhaasrobertmhaas
committed
postgres_fdw: Avoid possible misbehavior when RETURNING tableoid column only.
deparseReturningList ended up adding up RETURNING NULL to the code, but code elsewhere saw an empty list of attributes and concluded that it should not expect tuples from the remote side. Etsuro Fujita and Robert Haas, reviewed by Thom Brown
1 parent 1f3294c commit 2099b91

File tree

3 files changed

+73
-7
lines changed

3 files changed

+73
-7
lines changed

‎contrib/postgres_fdw/deparse.c

+11-7
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ static void deparseTargetList(StringInfo buf,
110110
PlannerInfo *root,
111111
Index rtindex,
112112
Relation rel,
113+
bool is_returning,
113114
Bitmapset *attrs_used,
114115
List **retrieved_attrs);
115116
static void deparseReturningList(StringInfo buf, PlannerInfo *root,
@@ -721,7 +722,7 @@ deparseSelectSql(StringInfo buf,
721722
* Construct SELECT list
722723
*/
723724
appendStringInfoString(buf, "SELECT ");
724-
deparseTargetList(buf, root, baserel->relid, rel, attrs_used,
725+
deparseTargetList(buf, root, baserel->relid, rel, false, attrs_used,
725726
retrieved_attrs);
726727

727728
/*
@@ -735,7 +736,8 @@ deparseSelectSql(StringInfo buf,
735736

736737
/*
737738
* Emit a target list that retrieves the columns specified in attrs_used.
738-
* This is used for both SELECT and RETURNING targetlists.
739+
* This is used for both SELECT and RETURNING targetlists; the is_returning
740+
* parameter is true only for a RETURNING targetlist.
739741
*
740742
* The tlist text is appended to buf, and we also create an integer List
741743
* of the columns being retrieved, which is returned to *retrieved_attrs.
@@ -745,6 +747,7 @@ deparseTargetList(StringInfo buf,
745747
PlannerInfo *root,
746748
Index rtindex,
747749
Relation rel,
750+
bool is_returning,
748751
Bitmapset *attrs_used,
749752
List **retrieved_attrs)
750753
{
@@ -774,6 +777,8 @@ deparseTargetList(StringInfo buf,
774777
{
775778
if (!first)
776779
appendStringInfoString(buf, ", ");
780+
else if (is_returning)
781+
appendStringInfoString(buf, " RETURNING ");
777782
first = false;
778783

779784
deparseColumnRef(buf, rtindex, i, root);
@@ -791,6 +796,8 @@ deparseTargetList(StringInfo buf,
791796
{
792797
if (!first)
793798
appendStringInfoString(buf, ", ");
799+
else if (is_returning)
800+
appendStringInfoString(buf, " RETURNING ");
794801
first = false;
795802

796803
appendStringInfoString(buf, "ctid");
@@ -800,7 +807,7 @@ deparseTargetList(StringInfo buf,
800807
}
801808

802809
/* Don't generate bad syntax if no undropped columns */
803-
if (first)
810+
if (first && !is_returning)
804811
appendStringInfoString(buf, "NULL");
805812
}
806813

@@ -1016,11 +1023,8 @@ deparseReturningList(StringInfo buf, PlannerInfo *root,
10161023
}
10171024

10181025
if (attrs_used != NULL)
1019-
{
1020-
appendStringInfoString(buf, " RETURNING ");
1021-
deparseTargetList(buf, root, rtindex, rel, attrs_used,
1026+
deparseTargetList(buf, root, rtindex, rel, true, attrs_used,
10221027
retrieved_attrs);
1023-
}
10241028
else
10251029
*retrieved_attrs = NIL;
10261030
}

‎contrib/postgres_fdw/expected/postgres_fdw.out

+53
Original file line numberDiff line numberDiff line change
@@ -2226,6 +2226,59 @@ SELECT c1,c2,c3,c4 FROM ft2 ORDER BY c1;
22262226
1104 | 204 | ddd |
22272227
(819 rows)
22282228

2229+
EXPLAIN (verbose, costs off)
2230+
INSERT INTO ft2 (c1,c2,c3) VALUES (9999,999,'foo') RETURNING tableoid::regclass;
2231+
QUERY PLAN
2232+
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
2233+
Insert on public.ft2
2234+
Output: (tableoid)::regclass
2235+
Remote SQL: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
2236+
-> Result
2237+
Output: 9999, 999, NULL::integer, 'foo'::text, NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying, 'ft2 '::character(10), NULL::user_enum
2238+
(5 rows)
2239+
2240+
INSERT INTO ft2 (c1,c2,c3) VALUES (9999,999,'foo') RETURNING tableoid::regclass;
2241+
tableoid
2242+
----------
2243+
ft2
2244+
(1 row)
2245+
2246+
EXPLAIN (verbose, costs off)
2247+
UPDATE ft2 SET c3 = 'bar' WHERE c1 = 9999 RETURNING tableoid::regclass;
2248+
QUERY PLAN
2249+
-------------------------------------------------------------------------------------------------------------------
2250+
Update on public.ft2
2251+
Output: (tableoid)::regclass
2252+
Remote SQL: UPDATE "S 1"."T 1" SET c3 = $2 WHERE ctid = $1
2253+
-> Foreign Scan on public.ft2
2254+
Output: c1, c2, NULL::integer, 'bar'::text, c4, c5, c6, c7, c8, ctid
2255+
Remote SQL: SELECT "C 1", c2, c4, c5, c6, c7, c8, ctid FROM "S 1"."T 1" WHERE (("C 1" = 9999)) FOR UPDATE
2256+
(6 rows)
2257+
2258+
UPDATE ft2 SET c3 = 'bar' WHERE c1 = 9999 RETURNING tableoid::regclass;
2259+
tableoid
2260+
----------
2261+
ft2
2262+
(1 row)
2263+
2264+
EXPLAIN (verbose, costs off)
2265+
DELETE FROM ft2 WHERE c1 = 9999 RETURNING tableoid::regclass;
2266+
QUERY PLAN
2267+
------------------------------------------------------------------------------------
2268+
Delete on public.ft2
2269+
Output: (tableoid)::regclass
2270+
Remote SQL: DELETE FROM "S 1"."T 1" WHERE ctid = $1
2271+
-> Foreign Scan on public.ft2
2272+
Output: ctid
2273+
Remote SQL: SELECT ctid FROM "S 1"."T 1" WHERE (("C 1" = 9999)) FOR UPDATE
2274+
(6 rows)
2275+
2276+
DELETE FROM ft2 WHERE c1 = 9999 RETURNING tableoid::regclass;
2277+
tableoid
2278+
----------
2279+
ft2
2280+
(1 row)
2281+
22292282
-- Test that trigger on remote table works as expected
22302283
CREATE OR REPLACE FUNCTION "S 1".F_BRTRIG() RETURNS trigger AS $$
22312284
BEGIN

‎contrib/postgres_fdw/sql/postgres_fdw.sql

+9
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,15 @@ EXPLAIN (verbose, costs off)
352352
DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;
353353
DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;
354354
SELECT c1,c2,c3,c4 FROM ft2 ORDER BY c1;
355+
EXPLAIN (verbose, costs off)
356+
INSERT INTO ft2 (c1,c2,c3) VALUES (9999,999,'foo') RETURNING tableoid::regclass;
357+
INSERT INTO ft2 (c1,c2,c3) VALUES (9999,999,'foo') RETURNING tableoid::regclass;
358+
EXPLAIN (verbose, costs off)
359+
UPDATE ft2 SET c3 = 'bar' WHERE c1 = 9999 RETURNING tableoid::regclass;
360+
UPDATE ft2 SET c3 = 'bar' WHERE c1 = 9999 RETURNING tableoid::regclass;
361+
EXPLAIN (verbose, costs off)
362+
DELETE FROM ft2 WHERE c1 = 9999 RETURNING tableoid::regclass;
363+
DELETE FROM ft2 WHERE c1 = 9999 RETURNING tableoid::regclass;
355364

356365
-- Test that trigger on remote table works as expected
357366
CREATE OR REPLACE FUNCTION "S 1".F_BRTRIG() RETURNS trigger AS $$

0 commit comments

Comments
 (0)