8000 Fix mishandling of system columns in FDW queries. · postgrespro/postgres@9c58101 · GitHub
[go: up one dir, main page]
Skip to content 65F3

Commit 9c58101

Browse files
tglsfdctglsfdc
committed
Fix mishandling of system columns in FDW queries.
postgres_fdw would send query conditions involving system columns to the remote server, even though it makes no effort to ensure that system columns other than CTID match what the remote side thinks. tableoid, in particular, probably won't match and might have some use in queries. Hence, prevent sending conditions that include non-CTID system columns. Also, create_foreignscan_plan neglected to check local restriction conditions while determining whether to set fsSystemCol for a foreign scan plan node. This again would bollix the results for queries that test a foreign table's tableoid. Back-patch the first fix to 9.3 where postgres_fdw was introduced. Back-patch the second to 9.2. The code is probably broken in 9.1 as well, but the patch doesn't apply cleanly there; given the weak state of support for FDWs in 9.1, it doesn't seem worth fixing. Etsuro Fujita, reviewed by Ashutosh Bapat, and somewhat modified by me
1 parent eca2b9b commit 9c58101

File tree

4 files changed

+116
-2
lines changed

4 files changed

+116
-2
lines changed

‎contrib/postgres_fdw/deparse.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,18 @@ foreign_expr_walker(Node *node,
254254
var->varlevelsup == 0)
255255
{
256256
/* Var belongs to foreign table */
257+
258+
/*
259+
* System columns other than ctid should not be sent to
260+
* the remote, since we don't make any effort to ensure
261+
* that local and remote values match (tableoid, in
262+
* particular, almost certainly doesn't match).
263+
*/
264+
if (var->varattno < 0 &&
265+
var->varattno != SelfItemPointerAttributeNumber)
266+
return false;
267+
268+
/* Else check the collation */
257269
collation = var->varcollid;
258270
state = OidIsValid(collation) ? FDW_COLLATE_SAFE : FDW_COLLATE_NONE;
259271
}

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,74 @@ DEALLOCATE st2;
832832
DEALLOCATE st3;
833833
DEALLOCATE st4;
834834
DEALLOCATE st5;
835+
-- System columns, except ctid, should not be sent to remote
836+
EXPLAIN (VERBOSE, COSTS false)
837+
SELECT * FROM ft1 t1 WHERE t1.tableoid = 'pg_class'::regclass LIMIT 1;
838+
QUERY PLAN
839+
-------------------------------------------------------------------------------
840+
Limit
841+
Output: c1, c2, c3, c4, c5, c6, c7, c8
842+
-> Foreign Scan on public.ft1 t1
843+
Output: c1, c2, c3, c4, c5, c6, c7, c8
844+
Filter: (t1.tableoid = 1259::oid)
845+
Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1"
846+
(6 rows)
847+
848+
SELECT * FROM ft1 t1 WHERE t1.tableoid = 'ft1'::regclass LIMIT 1;
849+
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
850+
----+----+-------+------------------------------+--------------------------+----+------------+-----
851+
1 | 1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
852+
(1 row)
853+
854+
EXPLAIN (VERBOSE, COSTS false)
855+
SELECT tableoid::regclass, * FROM ft1 t1 LIMIT 1;
856+
QUERY PLAN
857+
-------------------------------------------------------------------------------
858+
Limit
8 10000 59+
Output: ((tableoid)::regclass), c1, c2, c3, c4, c5, c6, c7, c8
860+
-> Foreign Scan on public.ft1 t1
861+
Output: (tableoid)::regclass, c1, c2, c3, c4, c5, c6, c7, c8
862+
Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1"
863+
(5 rows)
864+
865+
SELECT tableoid::regclass, * FROM ft1 t1 LIMIT 1;
866+
tableoid | c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
867+
----------+----+----+-------+------------------------------+--------------------------+----+------------+-----
868+
ft1 | 1 | 1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
869+
(1 row)
870+
871+
EXPLAIN (VERBOSE, COSTS false)
872+
SELECT * FROM ft1 t1 WHERE t1.ctid = '(0,2)';
873+
QUERY PLAN
874+
-------------------------------------------------------------------------------------------------------
875+
Foreign Scan on public.ft1 t1
876+
Output: c1, c2, c3, c4, c5, c6, c7, c8
877+
Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE ((ctid = '(0,2)'::tid))
878+
(3 rows)
879+
880+
SELECT * FROM ft1 t1 WHERE t1.ctid = '(0,2)';
881+
c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
882+
----+----+-------+------------------------------+--------------------------+----+------------+-----
883+
2 | 2 | 00002 | Sat Jan 03 00:00:00 1970 PST | Sat Jan 03 00:00:00 1970 | 2 | 2 | foo
884+
(1 row)
885+
886+
EXPLAIN (VERBOSE, COSTS false)
887+
SELECT ctid, * FROM ft1 t1 LIMIT 1;
888+
QUERY PLAN
889+
-------------------------------------------------------------------------------------
890+
Limit
891+
Output: ctid, c1, c2, c3, c4, c5, c6, c7, c8
892+
-> Foreign Scan on public.ft1 t1
893+
Output: ctid, c1, c2, c3, c4, c5, c6, c7, c8
894+
Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8, ctid FROM "S 1"."T 1"
895+
(5 rows)
896+
897+
SELECT ctid, * FROM ft1 t1 LIMIT 1;
898+
ctid | c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
899+
-------+----+----+-------+------------------------------+--------------------------+----+------------+-----
900+
(0,1) | 1 | 1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1 | 1 | foo
901+
(1 row)
902+
835903
-- ===================================================================
836904
-- used in pl/pgsql function
837905
-- ===================================================================

‎contrib/postgres_fdw/sql/postgres_fdw.sql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,20 @@ DEALLOCATE st3;
254254
DEALLOCATE st4;
255255
DEALLOCATE st5;
256256

257+
-- System columns, except ctid, should not be sent to remote
258+
EXPLAIN (VERBOSE, COSTS false)
259+
SELECT * FROM ft1 t1 WHERE t1.tableoid = 'pg_class'::regclass LIMIT 1;
260+
SELECT * FROM ft1 t1 WHERE t1.tableoid = 'ft1'::regclass LIMIT 1;
261+
EXPLAIN (VERBOSE, COSTS false)
262 B41A +
SELECT tableoid::regclass, * FROM ft1 t1 LIMIT 1;
263+
SELECT tableoid::regclass, * FROM ft1 t1 LIMIT 1;
264+
EXPLAIN (VERBOSE, COSTS false)
265+
SELECT * FROM ft1 t1 WHERE t1.ctid = '(0,2)';
266+
SELECT * FROM ft1 t1 WHERE t1.ctid = '(0,2)';
267+
EXPLAIN (VERBOSE, COSTS false)
268+
SELECT ctid, * FROM ft1 t1 LIMIT 1;
269+
SELECT ctid, * FROM ft1 t1 LIMIT 1;
270+
257271
-- ===================================================================
258272
-- used in pl/pgsql function
259273
-- ===================================================================

‎src/backend/optimizer/plan/createplan.c

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <math.h>
2121

2222
#include "access/skey.h"
23+
#include "access/sysattr.h"
2324
#include "catalog/pg_class.h"
2425
#include "foreign/fdwapi.h"
2526
#include "miscadmin.h"
@@ -1957,6 +1958,8 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
19571958
RelOptInfo *rel = best_path->path.parent;
19581959
Index scan_relid = rel->relid;
19591960
RangeTblEntry *rte;
1961+
Bitmapset *attrs_used = NULL;
1962+
ListCell *lc;
19601963
int i;
19611964

19621965
/* it should be a base rel... */
@@ -2004,17 +2007,34 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
20042007
* Detect whether any system columns are requested from rel. This is a
20052008
* bit of a kluge and might go away someday, so we intentionally leave it
20062009
* out of the API presented to FDWs.
2010+
*
2011+
* First, examine all the attributes needed for joins or final output.
2012+
* Note: we must look at reltargetlist, not the attr_needed data, because
2013+
* attr_needed isn't computed for inheritance child rels.
20072014
*/
2015+
pull_varattnos((Node *) rel->reltargetlist, rel->relid, &attrs_used);
2016+
2017+
/* Add all the attributes used by restriction clauses. */
2018+
foreach(lc, rel->baserestrictinfo)
2019+
{
2020+
RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
2021+
2022+
pull_varattnos((Node *) rinfo->clause, rel->relid, &attrs_used);
2023+
}
2024+
2025+
/* Now, are any system columns requested from rel? */
20082026
scan_plan->fsSystemCol = false;
2009-
for (i = rel->min_attr; i < 0; i++)
2027+
for (i = FirstLowInvalidHeapAttributeNumber + 1; i < 0; i++)
20102028
{
2011-
if (!bms_is_empty(rel->attr_needed[i - rel->min_attr]))
2029+
if (bms_is_member(i - FirstLowInvalidHeapAttributeNumber, attrs_used))
20122030
{
20132031
scan_plan->fsSystemCol = true;
20142032
break;
20152033
}
20162034
}
20172035

2036+
bms_free(attrs_used);
2037+
20182038
return scan_plan;
20192039
}
20202040

0 commit comments

Comments
 (0)
0