Re: postgres_fdw behaves oddly

Uh, where are we on this patch?

---------------------------------------------------------------------------

On Mon, Sep 8, 2014 at 09:30:32PM +0900, Etsuro Fujita wrote:
> (2014/09/02 18:55), Etsuro Fujita wrote:
> > (2014/09/01 20:15), Etsuro Fujita wrote:
> >> While working on [1], I've found that postgres_fdw behaves oddly:
>
> I've revised the patch a bit further. Please find attached a patch.
>
> Thanks,
>
> Best regards,
> Etsuro Fujita

> *** 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);
> }
> *** 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;
> }
>

>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +