(2014/11/11 18:45), Etsuro Fujita wrote:
(2014/11/10 20:05), Ashutosh Bapat wrote:
Since two separate issues 1. using reltargetlist instead of attr_needed
and 2. system columns usage in FDW are being tackled here, we should
separate the patch into two one for each of the issues.
Agreed. Will split the patch into two.
Here are splitted patches:
fscan-reltargetlist.patch - patch for #1
postgres_fdw-syscol.patch - patch for #2
Thanks,
Best regards,
Etsuro Fujita
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
***************
*** 20,25 ****
--- 20,26 ----
#include <math.h>
#include "access/skey.h"
+ #include "access/sysattr.h"
#include "catalog/pg_class.h"
#include "foreign/fdwapi.h"
#include "miscadmin.h"
***************
*** 1945,1950 **** create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
--- 1946,1953 ----
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... */
***************
*** 1993,2008 **** create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
* bit of a kluge and might go away someday, so we intentionally leave it
* out of the API presented to FDWs.
*/
scan_plan->fsSystemCol = false;
for (i = rel->min_attr; i < 0; i++)
{
! if (!bms_is_empty(rel->attr_needed[i - rel->min_attr]))
{
scan_plan->fsSystemCol = true;
break;
}
}
return scan_plan;
}
--- 1996,2030 ----
* bit of a kluge and might go away someday, so we intentionally leave it
* out of the API presented to FDWs.
*/
+
+ /*
+ * Add 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);
+ }
+
+ /* Are any system columns requested from rel? */
scan_plan->fsSystemCol = false;
for (i = rel->min_attr; i < 0; i++)
{
! if (bms_is_member(i - FirstLowInvalidHeapAttributeNumber, attrs_used))
{
scan_plan->fsSystemCol = true;
break;
}
}
+ bms_free(attrs_used);
+
return scan_plan;
}
*** a/contrib/postgres_fdw/deparse.c
--- b/contrib/postgres_fdw/deparse.c
***************
*** 252,257 **** foreign_expr_walker(Node *node,
--- 252,265 ----
if (var->varno == glob_cxt->foreignrel->relid &&
var->varlevelsup == 0)
{
+ /*
+ * System columns can't be sent to remote.
+ *
+ * XXX: we should probably send ctid to remote.
+ */
+ if (var->varattno < 0)
+ return false;
+
/* Var belongs to foreign table */
collation = var->varcollid;
state = OidIsValid(collation) ? FDW_COLLATE_SAFE : FDW_COLLATE_NONE;
***************
*** 1261,1266 **** deparseVar(Var *node, deparse_expr_cxt *context)
--- 1269,1279 ----
if (node->varno == context->foreignrel->relid &&
node->varlevelsup == 0)
{
+ /*
+ * System columns shouldn't be examined here.
+ */
+ Assert(node->varattno >= 0);
+
/* Var belongs to foreign table */
deparseColumnRef(buf, node->varno, node->varattno, context->root);
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
