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

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 7992191f79d55e13f910bf270b1b3af1d733f275..a75462b02afbf9f588d6bb7c782e6fb92d863907 100644 (file)
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -254,6 +254,18 @@ foreign_expr_walker(Node *node,
                                        var->varlevelsup == 0)
                                {
                                        /* Var belongs to foreign table */
+
+                                       /*
+                                        * System columns other than ctid should not be sent to
+                                        * the remote, since we don't make any effort to ensure
+                                        * that local and remote values match (tableoid, in
+                                        * particular, almost certainly doesn't match).
+                                        */
+                                       if (var->varattno < 0 &&
+                                               var->varattno != SelfItemPointerAttributeNumber)
+                                               return false;
+
+                                       /* Else check the collation */
                                        collation = var->varcollid;
                                        state = OidIsValid(collation) ? FDW_COLLATE_SAFE : FDW_COLLATE_NONE;
                                }

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index f7e11edab1234debdc2b7fac1e1dd05a63dd5c80..c7ba042cb7fca0d16208620825338f7bc8735adf 100644 (file)
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -832,6 +832,74 @@ DEALLOCATE st2;
 DEALLOCATE st3;
 DEALLOCATE st4;
 DEALLOCATE st5;
+-- System columns, except ctid, should not be sent to remote
+EXPLAIN (VERBOSE, COSTS false)
+SELECT * FROM ft1 t1 WHERE t1.tableoid = 'pg_class'::regclass LIMIT 1;
+                                  QUERY PLAN                                   
+-------------------------------------------------------------------------------
+ Limit
+   Output: c1, c2, c3, c4, c5, c6, c7, c8
+   ->  Foreign Scan on public.ft1 t1
+         Output: c1, c2, c3, c4, c5, c6, c7, c8
+         Filter: (t1.tableoid = 1259::oid)
+         Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1"
+(6 rows)
+
+SELECT * FROM ft1 t1 WHERE t1.tableoid = 'ft1'::regclass LIMIT 1;
+ c1 | c2 |  c3   |              c4              |            c5            | c6 |     c7     | c8  
+----+----+-------+------------------------------+--------------------------+----+------------+-----
+  1 |  1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1  | 1          | foo
+(1 row)
+
+EXPLAIN (VERBOSE, COSTS false)
+SELECT tableoid::regclass, * FROM ft1 t1 LIMIT 1;
+                                  QUERY PLAN                                   
+-------------------------------------------------------------------------------
+ Limit
+   Output: ((tableoid)::regclass), c1, c2, c3, c4, c5, c6, c7, c8
+   ->  Foreign Scan on public.ft1 t1
+         Output: (tableoid)::regclass, c1, c2, c3, c4, c5, c6, c7, c8
+         Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1"
+(5 rows)
+
+SELECT tableoid::regclass, * FROM ft1 t1 LIMIT 1;
+ tableoid | c1 | c2 |  c3   |              c4              |            c5            | c6 |     c7     | c8  
+----------+----+----+-------+------------------------------+--------------------------+----+------------+-----
+ ft1      |  1 |  1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1  | 1          | foo
+(1 row)
+
+EXPLAIN (VERBOSE, COSTS false)
+SELECT * FROM ft1 t1 WHERE t1.ctid = '(0,2)';
+                                              QUERY PLAN                                               
+-------------------------------------------------------------------------------------------------------
+ Foreign Scan on public.ft1 t1
+   Output: c1, c2, c3, c4, c5, c6, c7, c8
+   Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE ((ctid = '(0,2)'::tid))
+(3 rows)
+
+SELECT * FROM ft1 t1 WHERE t1.ctid = '(0,2)';
+ c1 | c2 |  c3   |              c4              |            c5            | c6 |     c7     | c8  
+----+----+-------+------------------------------+--------------------------+----+------------+-----
+  2 |  2 | 00002 | Sat Jan 03 00:00:00 1970 PST | Sat Jan 03 00:00:00 1970 | 2  | 2          | foo
+(1 row)
+
+EXPLAIN (VERBOSE, COSTS false)
+SELECT ctid, * FROM ft1 t1 LIMIT 1;
+                                     QUERY PLAN                                      
+-------------------------------------------------------------------------------------
+ Limit
+   Output: ctid, c1, c2, c3, c4, c5, c6, c7, c8
+   ->  Foreign Scan on public.ft1 t1
+         Output: ctid, c1, c2, c3, c4, c5, c6, c7, c8
+         Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8, ctid FROM "S 1"."T 1"
+(5 rows)
+
+SELECT ctid, * FROM ft1 t1 LIMIT 1;
+ ctid  | c1 | c2 |  c3   |              c4              |            c5            | c6 |     c7     | c8  
+-------+----+----+-------+------------------------------+--------------------------+----+------------+-----
+ (0,1) |  1 |  1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1  | 1          | foo
+(1 row)
+
 -- ===================================================================
 -- used in pl/pgsql function
 -- ===================================================================

diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index ae9668463c4a7f0a5064873b0c2b4d77a35e0d28..01491f886c1a3b023b3013148a95d2e2f250693b 100644 (file)
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -254,6 +254,20 @@ DEALLOCATE st3;
 DEALLOCATE st4;
 DEALLOCATE st5;
 
+-- System columns, except ctid, should not be sent to remote
+EXPLAIN (VERBOSE, COSTS false)
+SELECT * FROM ft1 t1 WHERE t1.tableoid = 'pg_class'::regclass LIMIT 1;
+SELECT * FROM ft1 t1 WHERE t1.tableoid = 'ft1'::regclass LIMIT 1;
+EXPLAIN (VERBOSE, COSTS false)
+SELECT tableoid::regclass, * FROM ft1 t1 LIMIT 1;
+SELECT tableoid::regclass, * FROM ft1 t1 LIMIT 1;
+EXPLAIN (VERBOSE, COSTS false)
+SELECT * FROM ft1 t1 WHERE t1.ctid = '(0,2)';
+SELECT * FROM ft1 t1 WHERE t1.ctid = '(0,2)';
+EXPLAIN (VERBOSE, COSTS false)
+SELECT ctid, * FROM ft1 t1 LIMIT 1;
+SELECT ctid, * FROM ft1 t1 LIMIT 1;
+
 -- ===================================================================
 -- used in pl/pgsql function
 -- ===================================================================

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index fa478025c1ad55483b9f10b25ab33a3edb5d128a..bf8dbe09db5f2b3f278dd85186e2051d60f69401 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -20,6 +20,7 @@
 #include <math.h>
 
 #include "access/skey.h"
+#include "access/sysattr.h"
 #include "catalog/pg_class.h"
 #include "foreign/fdwapi.h"
 #include "miscadmin.h"
@@ -1957,6 +1958,8 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
        RelOptInfo *rel = best_path->path.parent;
        Index           scan_relid = rel->relid;
        RangeTblEntry *rte;
+       Bitmapset  *attrs_used = NULL;
+       ListCell   *lc;
        int                     i;
 
        /* it should be a base rel... */
@@ -2004,17 +2007,34 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
         * Detect whether any system columns are requested from rel.  This is a
         * bit of a kluge and might go away someday, so we intentionally leave it
         * out of the API presented to FDWs.
+        *
+        * First, examine all the attributes needed for joins or final output.
+        * Note: we must look at reltargetlist, not the attr_needed data, because
+        * attr_needed isn't computed for inheritance child rels.
         */
+       pull_varattnos((Node *) rel->reltargetlist, rel->relid, &attrs_used);
+
+       /* Add all the attributes used by restriction clauses. */
+       foreach(lc, rel->baserestrictinfo)
+       {
+               RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
+
+               pull_varattnos((Node *) rinfo->clause, rel->relid, &attrs_used);
+       }
+
+       /* Now, are any system columns requested from rel? */
        scan_plan->fsSystemCol = false;
-       for (i = rel->min_attr; i < 0; i++)
+       for (i = FirstLowInvalidHeapAttributeNumber + 1; i < 0; i++)
        {
-               if (!bms_is_empty(rel->attr_needed[i - rel->min_attr]))
+               if (bms_is_member(i - FirstLowInvalidHeapAttributeNumber, attrs_used))
                {
                        scan_plan->fsSystemCol = true;
                        break;
                }
        }
 
+       bms_free(attrs_used);
+
        return scan_plan;
 }