From 906599f65f4353cd616d56171930457b86ddf709 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 22 Nov 2014 16:01:15 -0500 Subject: [PATCH] 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 --- src/backend/optimizer/plan/createplan.c | 26 ++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 91be71351bc..89b90521e81 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -20,6 +20,7 @@ #include #include "access/skey.h" +#include "access/sysattr.h" #include "foreign/fdwapi.h" #include "miscadmin.h" #include "nodes/makefuncs.h" @@ -1881,6 +1882,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... */ @@ -1928,17 +1931,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. */ - scan_plan->fsSystemCol = false; - for (i = rel->min_attr; i < 0; i++) + pull_varattnos((Node *) rel->reltargetlist, rel->relid, &attrs_used); + + /* Add all the attributes used by restriction clauses. */ + foreach(lc, rel->baserestrictinfo) { - if (!bms_is_empty(rel->attr_needed[i - rel->min_attr])) + 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 = FirstLowInvalidHeapAttributeNumber + 1; i < 0; i++) + { + if (bms_is_member(i - FirstLowInvalidHeapAttributeNumber, attrs_used)) { scan_plan->fsSystemCol = true; break; } } + bms_free(attrs_used); + return scan_plan; }