On 2016/05/10 16:56, Etsuro Fujita wrote:
Here is a patch to fix this.


I found that the previous patch handles the ForeignScan's fs_relids Bitmapset destructively. Also, I noticed that I removed some existing comments inadvertently. So, I'm attaching the updated patch to fix those things. I'll add this to the next CF. I think this should be addressed in advance of the release of 9.6, though.

Best regards,
Etsuro Fujita
*** 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;
  
  	/*
***************
*** 1256,1285 **** postgresBeginForeignScan(ForeignScanState *node, int eflags)
  	 * 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
--- 1257,1283 ----
  	 * 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.
+ 	 *
+ 	 * This should match what ExecCheckRTEPerms() does.
  	 */
  	if (fsplan->scan.scanrelid > 0)
+ 		rtindex = fsplan->scan.scanrelid;
+ 	else
  	{
  		/*
! 		 * It is ensured that foreign tables appearing in a foreign join
! 		 * belong to the same server and use the same user mapping, so pick
! 		 * the lowest-numbered one as a representative.
  		 */
! 		rtindex = -1;
! 		rtindex = bms_next_member(fsplan->fs_relids, rtindex);
! 		Assert(rtindex > 0);
  	}
! 	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)
--- 1314,1328 ----
  	 * 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

Reply via email to