(2014/09/01 20:15), Etsuro Fujita wrote:
> While working on [1], I've found that postgres_fdw behaves oddly:
>
> postgres=# create foreign table ft (a int) server loopback options
> (table_name 't');
> CREATE FOREIGN TABLE
> postgres=# select tableoid, * from ft;
> tableoid | a
> ----------+---
> 16400 | 1
> (1 row)
>
> postgres=# select tableoid, * from ft where tableoid = 16400;
> tableoid | a
> ----------+---
> (0 rows)
> I think that one simple way of fixing such issues would be
> to consider unsafe to send to the remote a qual that contains any system
> columns.
I noticed the previous patch has overdone it. Attached is an updated
version of the patch.
Thanks,
PS:
> [1] https://commitfest.postgresql.org/action/patch_view?id=1386
I'll update the patch in [1] on top of this version.
Best regards,
Etsuro Fujita
*** a/contrib/postgres_fdw/deparse.c
--- b/contrib/postgres_fdw/deparse.c
***************
*** 252,257 **** foreign_expr_walker(Node *node,
--- 252,263 ----
if (var->varno == glob_cxt->foreignrel->relid &&
var->varlevelsup == 0)
{
+ /*
+ * System columns can't be sent 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;
*** 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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
