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 this is because (a) the qual that contains tableoid can be
sent to the remote as shown in the EXPLAIN output:
postgres=# explain verbose select tableoid, * from ft where tableoid =
16400;
QUERY PLAN
----------------------------------------------------------------------
Foreign Scan on public.ft (cost=100.00..193.20 rows=2560 width=8)
Output: tableoid, a
Remote SQL: SELECT a FROM public.t WHERE ((tableoid = 16400::oid))
Planning time: 0.110 ms
(4 rows)
and because (b) the tableoid value can be differs between the local and
the remote. 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 (though we should probably give special treatment to quals
containing only ctid). With the modification of postgres_fdw, we get
the right result:
postgres=# select tableoid, * from ft where tableoid = 16400;
tableoid | a
----------+---
16400 | 1
(1 row)
However, it's not complete enough. Here is another surising result
(note no tableoid column in the select list):
postgres=# select * from ft where tableoid = 16400;
a
---
(0 rows)
I think that this is because create_foreignscan_plan doesn't refer to
rel->baserestrictinfo when detecting whether any system columns are
requested. By the additional modification of create_foreignscan_plan,
we get the right result:
postgres=# select * from ft where tableoid = 16400;
a
---
1
(1 row)
I'd also like to propose to change the function so as to make reference
to rel->reltargetlist, not to attr_needed, to match the code with other
places. Please find attached a patch.
Thanks,
[1] https://commitfest.postgresql.org/action/patch_view?id=1386
Best regards,
Etsuro Fujita
*** a/contrib/postgres_fdw/deparse.c
--- b/contrib/postgres_fdw/deparse.c
***************
*** 243,248 **** foreign_expr_walker(Node *node,
--- 243,254 ----
Var *var = (Var *) node;
/*
+ * System columns can't be sent to remote.
+ */
+ if (var->varattno < 0)
+ return false;
+
+ /*
* If the Var is from the foreign table, we consider its
* collation (if any) safe to use. If it is from another
* table, we treat its collation the same way as we would a
*** 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
