Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 37c8457

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 9418d79 commit 37c8457

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
@@ -112,6 +112,7 @@ static void deparseTargetList(StringInfo buf,
112112
PlannerInfo *root,
113113
Index rtindex,
114114
Relation rel,
115+
bool is_returning,
115116
Bitmapset *attrs_used,
116117
List **retrieved_attrs);
117118
static void deparseReturningList(StringInfo buf, PlannerInfo *root,
@@ -776,7 +777,7 @@ deparseSelectSql(Bitmapset *attrs_used, List **retrieved_attrs,
776777
* Construct SELECT list
777778
*/
778779
appendStringInfoString(buf, "SELECT ");
779-
deparseTargetList(buf, root, foreignrel->relid, rel, attrs_used,
780+
deparseTargetList(buf, root, foreignrel->relid, rel, false, attrs_used,
780781
retrieved_attrs);
781782

782783
/*
@@ -790,7 +791,8 @@ deparseSelectSql(Bitmapset *attrs_used, List **retrieved_attrs,
790791

791792
/*
792793
* Emit a target list that retrieves the columns specified in attrs_used.
793-
* This is used for both SELECT and RETURNING targetlists.
794+
* This is used for both SELECT and RETURNING targetlists; the is_returning
795+
* parameter is true only for a RETURNING targetlist.
794796
*
795797
* The tlist text is appended to buf, and we also create an integer List
796798
* of the columns being retrieved, which is returned to *retrieved_attrs.
@@ -800,6 +802,7 @@ deparseTargetList(StringInfo buf,
800802
PlannerInfo *root,
801803
Index rtindex,
802804
Relation rel,
805+
bool is_returning,
803806
Bitmapset *attrs_used,
804807
List **retrieved_attrs)
805808
{
@@ -829,6 +832,8 @@ deparseTargetList(StringInfo buf,
829832
{
830833
if (!first)
831834
appendStringInfoString(buf, ", ");
835+
else if (is_returning)
836+
appendStringInfoString(buf, " RETURNING ");
832837
first = false;
833838

834839
deparseColumnRef(buf, rtindex, i, root);
@@ -846,6 +851,8 @@ deparseTargetList(StringInfo buf,
846851
{
847852
if (!first)
848853
appendStringInfoString(buf, ", ");
854+
else if (is_returning)
855+
appendStringInfoString(buf, " RETURNING ");
849856
first = false;
850857

851858
appendStringInfoString(buf, "ctid");
@@ -855,7 +862,7 @@ deparseTargetList(StringInfo buf,
855862
}
856863

857864
/* Don't generate bad syntax if no undropped columns */
858-
if (first)
865+
if (first && !is_returning)
859866
appendStringInfoString(buf, "NULL");
860867
}
861868

@@ -1113,11 +1120,8 @@ deparseReturningList(StringInfo buf, PlannerInfo *root,
11131120
}
11141121

11151122
if (attrs_used != NULL)
1116-
{
1117-
appendStringInfoString(buf, " RETURNING ");
1118-
deparseTargetList(buf, root, rtindex, rel, attrs_used,
1123+
deparseTargetList(buf, root, rtindex, rel, true, attrs_used,
11191124
retrieved_attrs);
1120-
}
11211125
else
11221126
*retrieved_attrs = NIL;
11231127
}

‎contrib/postgres_fdw/expected/postgres_fdw.out

+53
Original file line numberDiff line numberDiff line change
@@ -2408,6 +2408,59 @@ SELECT c1,c2,c3,c4 FROM ft2 ORDER BY c1;
24082408
1104 | 204 | ddd |
24092409
(819 rows)
24102410

2411+
EXPLAIN (verbose, costs off)
2412+
INSERT INTO ft2 (c1,c2,c3) VALUES (9999,999,'foo') RETURNING tableoid::regclass;
2413+
QUERY PLAN
2414+
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
2415+
Insert on public.ft2
2416+
Output: (tableoid)::regclass
2417+
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)
2418+
-> Result
2419+
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
2420+
(5 rows)
2421+
2422+
INSERT INTO ft2 (c1,c2,c3) VALUES (9999,999,'foo') RETURNING tableoid::regclass;
2423+
tableoid
2424+
----------
2425+
ft2
2426+
(1 row)
2427+
2428+
EXPLAIN (verbose, costs off)
2429+
UPDATE ft2 SET c3 = 'bar' WHERE c1 = 9999 RETURNING tableoid::regclass;
2430+
QUERY PLAN
2431+
-------------------------------------------------------------------------------------------------------------------
2432+
Update on public.ft2
2433+
Output: (tableoid)::regclass
2434+
Remote SQL: UPDATE "S 1"."T 1" SET c3 = $2 WHERE ctid = $1
2435+
-> Foreign Scan on public.ft2
2436+
Output: c1, c2, NULL::integer, 'bar'::text, c4, c5, c6, c7, c8, ctid
2437+
Remote SQL: SELECT "C 1", c2, c4, c5, c6, c7, c8, ctid FROM "S 1"."T 1" WHERE (("C 1" = 9999)) FOR UPDATE
2438+
(6 rows)
2439+
2440+
UPDATE ft2 SET c3 = 'bar' WHERE c1 = 9999 RETURNING tableoid::regclass;
2441+
tableoid
2442+
----------
2443+
ft2
2444+
(1 row)
2445+
2446+
EXPLAIN (verbose, costs off)
2447+
DELETE FROM ft2 WHERE c1 = 9999 RETURNING tableoid::regclass;
2448+
QUERY PLAN
2449+
------------------------------------------------------------------------------------
2450+
Delete on public.ft2
2451+
Output: (tableoid)::regclass
2452+
Remote SQL: DELETE FROM "S 1"."T 1" WHERE ctid = $1
2453+
-> Foreign Scan on public.ft2
2454+
Output: ctid
2455+
Remote SQL: SELECT ctid FROM "S 1"."T 1" WHERE (("C 1" = 9999)) FOR UPDATE
2456+
(6 rows)
2457+
2458+
DELETE FROM ft2 WHERE c1 = 9999 RETURNING tableoid::regclass;
2459+
tableoid
2460+
----------
2461+
ft2
2462+
(1 row)
2463+
24112464
-- Test that trigger on remote table works as expected
24122465
CREATE OR REPLACE FUNCTION "S 1".F_BRTRIG() RETURNS trigger AS $$
24132466
BEGIN

‎contrib/postgres_fdw/sql/postgres_fdw.sql

+9
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,15 @@ EXPLAIN (verbose, costs off)
413413
DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;
414414
DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;
415415
SELECT c1,c2,c3,c4 FROM ft2 ORDER BY c1;
416+
EXPLAIN (verbose, costs off)
417+
INSERT INTO ft2 (c1,c2,c3) VALUES (9999,999,'foo') RETURNING tableoid::regclass;
418+
INSERT INTO ft2 (c1,c2,c3) VALUES (9999,999,'foo') RETURNING tableoid::regclass;
419+
EXPLAIN (verbose, costs off)
420+
UPDATE ft2 SET c3 = 'bar' WHERE c1 = 9999 RETURNING tableoid::regclass;
421+
UPDATE ft2 SET c3 = 'bar' WHERE c1 = 9999 RETURNING tableoid::regclass;
422+
EXPLAIN (verbose, costs off)
423+
DELETE FROM ft2 WHERE c1 = 9999 RETURNING tableoid::regclass;
424+
DELETE FROM ft2 WHERE c1 = 9999 RETURNING tableoid::regclass;
416425

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

0 commit comments

Comments
 (0)