On 2016/05/02 22:06, Robert Haas wrote:
On Thu, Apr 28, 2016 at 7:59 AM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
On 2016/03/14 17:56, Ashutosh Bapat wrote:
On Mon, Mar 14, 2016 at 1:29 PM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp <mailto:fujita.ets...@lab.ntt.co.jp>> wrote:
/*
* Build the fdw_private list that will be available to the
executor.
* Items in the list must match order in enum
FdwScanPrivateIndex.
*/
fdw_private = list_make4(makeString(sql.data),
retrieved_attrs,
makeInteger(fpinfo->fetch_size),
makeInteger(foreignrel->umid));
I don't think it's correct to use makeInteger for the foreignrel's
umid.
As long as we are using makeInteger() and inVal() pair to set and
extract the values, it should be fine.
Yeah, but my concern about this is eg, print plan if debugging (ie,
debug_print_plan=on); the umid OID will be printed with the %ld specifier,
so in some platform, the OID might be printed wrongly. Maybe I'm missing
something, though.
That seems like a legitimate, if minor, complaint.
Here is a patch to fix this. That is basically the same as in [1], but
I rebased the patch against HEAD and removed list_make5 and its friends,
which were added just for the postgres_fdw DML pushdown.
Sorry for the delay. I was on vacation.
Best regards,
Etsuro Fujita
[1] http://www.postgresql.org/message-id/56e66f61.3070...@lab.ntt.co.jp
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
***************
*** 67,74 **** enum FdwScanPrivateIndex
FdwScanPrivateRetrievedAttrs,
/* Integer representing the desired fetch_size */
FdwScanPrivateFetchSize,
- /* Oid of user mapping to be used while connecting to the foreign server */
- FdwScanPrivateUserMappingOid,
/*
* String describing join i.e. names of relations being joined and types
--- 67,72 ----
***************
*** 1198,1208 **** postgresGetForeignPlan(PlannerInfo *root,
* Build the fdw_private list that will be available to the executor.
* Items in the list must match order in enum FdwScanPrivateIndex.
*/
! fdw_private = list_make5(makeString(sql.data),
remote_conds,
retrieved_attrs,
! makeInteger(fpinfo->fetch_size),
! makeInteger(foreignrel->umid));
if (foreignrel->reloptkind == RELOPT_JOINREL)
fdw_private = lappend(fdw_private,
makeString(fpinfo->relation_name->data));
--- 1196,1205 ----
* Build the fdw_private list that will be available to the executor.
* Items in the list must match order in enum FdwScanPrivateIndex.
*/
! fdw_private = list_make4(makeString(sql.data),
remote_conds,
retrieved_attrs,
! makeInteger(fpinfo->fetch_size));
if (foreignrel->reloptkind == RELOPT_JOINREL)
fdw_private = lappend(fdw_private,
makeString(fpinfo->relation_name->data));
***************
*** 1234,1240 **** postgresBeginForeignScan(ForeignScanState *node, int eflags)
--- 1231,1241 ----
ForeignScan *fsplan = (ForeignScan *) node->ss.ps.plan;
EState *estate = node->ss.ps.state;
PgFdwScanState *fsstate;
+ RangeTblEntry *rte;
+ Oid userid;
+ ForeignTable *table;
UserMapping *user;
+ int rtindex;
int numParams;
/*
***************
*** 1250,1285 **** postgresBeginForeignScan(ForeignScanState *node, int eflags)
node->fdw_state = (void *) fsstate;
/*
! * Obtain the foreign server where to connect and user mapping to use for
! * connection. For base relations we obtain this information from
! * catalogs. For join relations, this information is frozen at the time of
! * planning to ensure that the join is safe to pushdown. In case the
! * information goes stale between planning and execution, plan will be
! * invalidated and replanned.
*/
if (fsplan->scan.scanrelid > 0)
! {
! ForeignTable *table;
!
! /*
! * Identify which user to do the remote access as. This should match
! * what ExecCheckRTEPerms() does.
! */
! RangeTblEntry *rte = rt_fetch(fsplan->scan.scanrelid, estate->es_range_table);
! Oid userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
!
! fsstate->rel = node->ss.ss_currentRelation;
! table = GetForeignTable(RelationGetRelid(fsstate->rel));
!
! user = GetUserMapping(userid, table->serverid);
! }
else
! {
! Oid umid = intVal(list_nth(fsplan->fdw_private, FdwScanPrivateUserMappingOid));
! user = GetUserMappingById(umid);
! Assert(fsplan->fs_server == user->serverid);
! }
/*
* Get connection to the foreign server. Connection manager will
--- 1251,1274 ----
node->fdw_state = (void *) fsstate;
/*
! * Get the user mapping. This should match what ExecCheckRTEPerms() does.
! *
! * If scanning a foreign join, the planner ensured that joined relations
! * are foreign tables belonging to the same server and using the same
! * user mapping, so pick the lowest-numbered one as a representative.
! * Note that in that case the current user is the same as the one for
! * which the plan was created (see RevalidateCachedQuery), so this will
! * derive the same user to do the remote access as as in build_simple_rel.
*/
if (fsplan->scan.scanrelid > 0)
! rtindex = fsplan->scan.scanrelid;
else
! rtindex = bms_first_member(fsplan->fs_relids);
! rte = rt_fetch(rtindex, estate->es_range_table);
! userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
! table = GetForeignTable(rte->relid);
! user = GetUserMapping(userid, table->serverid);
/*
* Get connection to the foreign server. Connection manager will
***************
*** 1316,1324 **** postgresBeginForeignScan(ForeignScanState *node, int eflags)
--- 1305,1319 ----
* into local representation and error reporting during that process.
*/
if (fsplan->scan.scanrelid > 0)
+ {
+ fsstate->rel = node->ss.ss_currentRelation;
fsstate->tupdesc = RelationGetDescr(fsstate->rel);
+ }
else
+ {
+ fsstate->rel = NULL;
fsstate->tupdesc = node->ss.ss_ScanTupleSlot->tts_tupleDescriptor;
+ }
fsstate->attinmeta = TupleDescGetAttInMetadata(fsstate->tupdesc);
*** a/src/include/nodes/pg_list.h
--- b/src/include/nodes/pg_list.h
***************
*** 134,152 **** list_length(const List *l)
#define list_make2(x1,x2) lcons(x1, list_make1(x2))
#define list_make3(x1,x2,x3) lcons(x1, list_make2(x2, x3))
#define list_make4(x1,x2,x3,x4) lcons(x1, list_make3(x2, x3, x4))
- #define list_make5(x1,x2,x3,x4,x5) lcons(x1, list_make4(x2, x3, x4, x5))
#define list_make1_int(x1) lcons_int(x1, NIL)
#define list_make2_int(x1,x2) lcons_int(x1, list_make1_int(x2))
#define list_make3_int(x1,x2,x3) lcons_int(x1, list_make2_int(x2, x3))
#define list_make4_int(x1,x2,x3,x4) lcons_int(x1, list_make3_int(x2, x3, x4))
- #define list_make5_int(x1,x2,x3,x4,x5) lcons_int(x1, list_make4_int(x2, x3, x4, x5))
#define list_make1_oid(x1) lcons_oid(x1, NIL)
#define list_make2_oid(x1,x2) lcons_oid(x1, list_make1_oid(x2))
#define list_make3_oid(x1,x2,x3) lcons_oid(x1, list_make2_oid(x2, x3))
#define list_make4_oid(x1,x2,x3,x4) lcons_oid(x1, list_make3_oid(x2, x3, x4))
- #define list_make5_oid(x1,x2,x3,x4,x5) lcons_oid(x1, list_make4_oid(x2, x3, x4, x5))
/*
* foreach -
--- 134,149 ----
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
