Skip to content

Commit d160e2a

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 453d408 commit d160e2a

File tree

3 files changed

+73
-7
lines changed

3 files changed

+73
-7
lines changed

‎contrib/postgres_fdw/deparse.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ static void deparseTargetList(StringInfo buf,
111111
PlannerInfo *root,
112112
Index rtindex,
113113
Relation rel,
114+
bool is_returning,
114115
Bitmapset *attrs_used,
115116
List **retrieved_attrs);
116117
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

@@ -1019,11 +1026,8 @@ deparseReturningList(StringInfo buf, PlannerInfo *root,
10191026
}
10201027

10211028
if (attrs_used != NULL)
1022-
{
1023-
appendStringInfoString(buf, " RETURNING ");
1024-
deparseTargetList(buf, root, rtindex, rel, attrs_used,
1029+
deparseTargetList(buf, root, rtindex, rel, true, attrs_used,
10251030
retrieved_attrs);
1026-
}
10271031
else
10281032
*retrieved_attrs = NIL;
10291033
}

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2232,6 +2232,59 @@ SELECT c1,c2,c3,c4 FROM ft2 ORDER BY c1;
22322232
1104 | 204 | ddd |
22332233
(819 rows)
22342234

2235+
EXPLAIN (verbose, costs off)
2236+
INSERT INTO ft2 (c1,c2,c3) VALUES (9999,999,'foo') RETURNING tableoid::regclass;
2237+
QUERY PLAN
2238+
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
2239+
Insert on public.ft2
2240+
Output: (tableoid)::regclass
2241+
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)
2242+
-> Result
2243+
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
2244+
(5 rows)
2245+
2246+
INSERT INTO ft2 (c1,c2,c3) VALUES (9999,999,'foo') RETURNING tableoid::regclass;
2247+
tableoid
2248+
----------
2249+
ft2
2250+
(1 row)
2251+
2252+
EXPLAIN (verbose, costs off)
2253+
UPDATE ft2 SET c3 = 'bar' WHERE c1 = 9999 RETURNING tableoid::regclass;
2254+
QUERY PLAN
2255+
-------------------------------------------------------------------------------------------------------------------
2256+
Update on public.ft2
2257+
Output: (tableoid)::regclass
2258+
Remote SQL: UPDATE "S 1"."T 1" SET c3 = $2 WHERE ctid = $1
2259+
-> Foreign Scan on public.ft2
2260+
Output: c1, c2, NULL::integer, 'bar'::text, c4, c5, c6, c7, c8, ctid
2261+
Remote SQL: SELECT "C 1", c2, c4, c5, c6, c7, c8, ctid FROM "S 1"."T 1" WHERE (("C 1" = 9999)) FOR UPDATE
2262+
(6 rows)
2263+
2264+
UPDATE ft2 SET c3 = 'bar' WHERE c1 = 9999 RETURNING tableoid::regclass;
2265+
tableoid
2266+
----------
2267+
ft2
2268+
(1 row)
2269+
2270+
EXPLAIN (verbose, costs off)
2271+
DELETE FROM ft2 WHERE c1 = 9999 RETURNING tableoid::regclass;
2272+
QUERY PLAN
2273+
------------------------------------------------------------------------------------
2274+
Delete on public.ft2
2275+
Output: (tableoid)::regclass
2276+
Remote SQL: DELETE FROM "S 1"."T 1" WHERE ctid = $1
2277+
-> Foreign Scan on public.ft2
2278+
Output: ctid
2279+
Remote SQL: SELECT ctid FROM "S 1"."T 1" WHERE (("C 1" = 9999)) FOR UPDATE
2280+
(6 rows)
2281+
2282+
DELETE FROM ft2 WHERE c1 = 9999 RETURNING tableoid::regclass;
2283+
tableoid
2284+
----------
2285+
ft2
2286+
(1 row)
2287+
22352288
-- Test that trigger on remote table works as expected
22362289
CREATE OR REPLACE FUNCTION "S 1".F_BRTRIG() RETURNS trigger AS $$
22372290
BEGIN

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,15 @@ EXPLAIN (verbose, costs off)
358358
DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;
359359
DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;
360360
SELECT c1,c2,c3,c4 FROM ft2 ORDER BY c1;
361+
EXPLAIN (verbose, costs off)
362+
INSERT INTO ft2 (c1,c2,c3) VALUES (9999,999,'foo') RETURNING tableoid::regclass;
363+
INSERT INTO ft2 (c1,c2,c3) VALUES (9999,999,'foo') RETURNING tableoid::regclass;
364+
EXPLAIN (verbose, costs off)
365+
UPDATE ft2 SET c3 = 'bar' WHERE c1 = 9999 RETURNING tableoid::regclass;
366+
UPDATE ft2 SET c3 = 'bar' WHERE c1 = 9999 RETURNING tableoid::regclass;
367+
EXPLAIN (verbose, costs off)
368+
DELETE FROM ft2 WHERE c1 = 9999 RETURNING tableoid::regclass;
369+
DELETE FROM ft2 WHERE c1 = 9999 RETURNING tableoid::regclass;
361370

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

0 commit comments

Comments
 (0)