On 2016/09/08 19:55, Etsuro Fujita wrote:
On 2016/09/07 13:21, Ashutosh Bapat wrote:
    * with the patch:
    postgres=# explain verbose delete from ft1 using ft2 where ft1.a =
    ft2.a;
                                                             QUERY PLAN

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

     Delete on public.ft1  (cost=100.00..102.04 rows=1 width=38)
       ->  Foreign Delete  (cost=100.00..102.04 rows=1 width=38)
             Remote SQL: DELETE FROM public.t1 r1 USING (SELECT ROW(a,
    b), a FROM public.t2) ss1(c1, c2) WHERE ((r1.a = ss1.c2))
    (3 rows)

The underlying scan on t2 requires ROW(a,b) for locking the row for
update/share. But clearly it's not required if the full query is being
pushed down.

Is there a way we can detect that ROW(a,b) is useless
column (not used anywhere in the other parts of the query like
RETURNING, DELETE clause etc.) and eliminate it?

I don't have a clear solution for that yet, but I'll try to remove that
in the next version.

Similarly for a, it's
part of the targetlist of the underlying scan so that the WHERE clause
can be applied on it. But it's not needed if we are pushing down the
query. If we eliminate the targetlist of the query, we could construct a
remote query without having subquery in it, making it more readable.

Will try to do so also.

I addressed this by improving the deparse logic so that a remote query for performing an UPDATE/DELETE on a join directly on the remote can be created as proposed if possible. Attached is an updated version of the patch, which is created on top of the patch set [1]. The patch is still WIP (ie, needs more comments and regression tests, at least), but any comments would be gratefully appreciated.

Best regards,
Etsuro Fujita

[1] https://www.postgresql.org/message-id/11eafd10-d3f8-ac8a-b642-b0e65037c76b%40lab.ntt.co.jp
*** a/contrib/postgres_fdw/deparse.c
--- b/contrib/postgres_fdw/deparse.c
***************
*** 133,145 **** static void deparseTargetList(StringInfo buf,
  				  Bitmapset *attrs_used,
  				  bool qualify_col,
  				  List **retrieved_attrs);
! static void deparseExplicitTargetList(List *tlist, List **retrieved_attrs,
  						  deparse_expr_cxt *context);
  static void deparseReturningList(StringInfo buf, PlannerInfo *root,
  					 Index rtindex, Relation rel,
  					 bool trig_after_row,
  					 List *returningList,
  					 List **retrieved_attrs);
  static void deparseColumnRef(StringInfo buf, int varno, int varattno,
  				 PlannerInfo *root, bool qualify_col);
  static void deparseRelation(StringInfo buf, Relation rel);
--- 133,164 ----
  				  Bitmapset *attrs_used,
  				  bool qualify_col,
  				  List **retrieved_attrs);
! static void deparseExplicitTargetList(bool is_returning,
! 						  List *tlist,
! 						  List **retrieved_attrs,
  						  deparse_expr_cxt *context);
  static void deparseReturningList(StringInfo buf, PlannerInfo *root,
  					 Index rtindex, Relation rel,
  					 bool trig_after_row,
  					 List *returningList,
  					 List **retrieved_attrs);
+ static void deparseExplicitReturningList(List *rlist,
+ 							 List **retrieved_attrs,
+ 							 deparse_expr_cxt *context);
+ static List *makeExplicitReturningList(Index rtindex, Relation rel,
+ 						  List *returningList,
+ 						  List **fdw_scan_tlist,
+ 						  Relids *wholerows);
+ static void rewriteFromExprForRel(PlannerInfo *root, RelOptInfo *foreignrel,
+ 					  Index target_rel, List **target_conds, Relids wholerows);
+ static List *pullUpTargetConds(RelOptInfo *foreignrel, List *joinclauses,
+ 				  Index target_rel, List **target_conds);
+ static bool pullUpSubquery(PlannerInfo *root, RelOptInfo *foreignrel, JoinType jointype,
+ 			   Index target_rel, List **target_conds, Relids wholerows);
+ static bool isSimpleSubquery(PlannerInfo *root, RelOptInfo *baserel, JoinType jointype,
+ 				 Relids wholerows);
+ static List *getCleanSubqueryExprs(PlannerInfo *root, RelOptInfo *foreignrel,
+ 					  List *subquery_exprs, Relids wholerows);
  static void deparseColumnRef(StringInfo buf, int varno, int varattno,
  				 PlannerInfo *root, bool qualify_col);
  static void deparseRelation(StringInfo buf, Relation rel);
***************
*** 168,178 **** static void deparseSelectSql(List *tlist, List **retrieved_attrs,
  static void deparseLockingClause(deparse_expr_cxt *context);
  static void appendOrderByClause(List *pathkeys, deparse_expr_cxt *context);
  static void appendConditions(List *exprs, deparse_expr_cxt *context);
! static void deparseFromExprForRel(StringInfo buf, PlannerInfo *root,
! 					RelOptInfo *joinrel, bool use_alias, List **params_list);
  static void deparseFromExpr(List *quals, deparse_expr_cxt *context);
  static void deparseRangeTblRef(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
! 				   bool make_subquery, List **params_list);
  static void appendSubselectAlias(StringInfo buf, int tabno, int ncols);
  static void getSubselectAliasInfo(Expr *node, RelOptInfo *foreignrel,
  					  int *tabno, int *colno);
--- 187,197 ----
  static void deparseLockingClause(deparse_expr_cxt *context);
  static void appendOrderByClause(List *pathkeys, deparse_expr_cxt *context);
  static void appendConditions(List *exprs, deparse_expr_cxt *context);
! static void deparseFromExprForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
! 					  bool use_alias, Index target_rel, List **params_list);
  static void deparseFromExpr(List *quals, deparse_expr_cxt *context);
  static void deparseRangeTblRef(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
! 				   bool make_subquery, Index target_rel, List **params_list);
  static void appendSubselectAlias(StringInfo buf, int tabno, int ncols);
  static void getSubselectAliasInfo(Expr *node, RelOptInfo *foreignrel,
  					  int *tabno, int *colno);
***************
*** 1088,1094 **** deparseSelectSql(List *tlist, List **retrieved_attrs, deparse_expr_cxt *context)
  		tlist != NIL)
  	{
  		/* Use the input tlist */
! 		deparseExplicitTargetList(tlist, retrieved_attrs, context);
  	}
  	else
  	{
--- 1107,1113 ----
  		tlist != NIL)
  	{
  		/* Use the input tlist */
! 		deparseExplicitTargetList(false, tlist, retrieved_attrs, context);
  	}
  	else
  	{
***************
*** 1131,1137 **** deparseFromExpr(List *quals, deparse_expr_cxt *context)
  	appendStringInfoString(buf, " FROM ");
  	deparseFromExprForRel(buf, context->root, scanrel,
  						  (bms_num_members(scanrel->relids) > 1),
! 						  context->params_list);
  
  	/* Construct WHERE clause */
  	if (quals != NIL)
--- 1150,1156 ----
  	appendStringInfoString(buf, " FROM ");
  	deparseFromExprForRel(buf, context->root, scanrel,
  						  (bms_num_members(scanrel->relids) > 1),
! 						  (Index) 0, context->params_list);
  
  	/* Construct WHERE clause */
  	if (quals != NIL)
***************
*** 1407,1413 **** get_jointype_name(JoinType jointype)
   * from 1. It has same number of entries as tlist.
   */
  static void
! deparseExplicitTargetList(List *tlist, List **retrieved_attrs,
  						  deparse_expr_cxt *context)
  {
  	ListCell   *lc;
--- 1426,1434 ----
   * from 1. It has same number of entries as tlist.
   */
  static void
! deparseExplicitTargetList(bool is_returning,
! 						  List *tlist,
! 						  List **retrieved_attrs,
  						  deparse_expr_cxt *context)
  {
  	ListCell   *lc;
***************
*** 1425,1437 **** deparseExplicitTargetList(List *tlist, List **retrieved_attrs,
  
  		if (i > 0)
  			appendStringInfoString(buf, ", ");
  		deparseExpr((Expr *) tle->expr, context);
  
  		*retrieved_attrs = lappend_int(*retrieved_attrs, i + 1);
  		i++;
  	}
  
! 	if (i == 0)
  		appendStringInfoString(buf, "NULL");
  }
  
--- 1446,1461 ----
  
  		if (i > 0)
  			appendStringInfoString(buf, ", ");
+ 		else if (is_returning)
+ 			appendStringInfoString(buf, " RETURNING ");
+ 
  		deparseExpr((Expr *) tle->expr, context);
  
  		*retrieved_attrs = lappend_int(*retrieved_attrs, i + 1);
  		i++;
  	}
  
! 	if (i == 0 && !is_returning)
  		appendStringInfoString(buf, "NULL");
  }
  
***************
*** 1444,1450 **** deparseExplicitTargetList(List *tlist, List **retrieved_attrs,
   */
  static void
  deparseFromExprForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
! 					  bool use_alias, List **params_list)
  {
  	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
  
--- 1468,1474 ----
   */
  static void
  deparseFromExprForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
! 					  bool use_alias, Index target_rel, List **params_list)
  {
  	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
  
***************
*** 1452,1471 **** deparseFromExprForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
  	{
  		StringInfoData join_sql_o;
  		StringInfoData join_sql_i;
  
  		/* Deparse outer relation */
! 		initStringInfo(&join_sql_o);
! 		deparseRangeTblRef(&join_sql_o, root,
! 						   fpinfo->outerrel,
! 						   fpinfo->make_outerrel_subquery,
! 						   params_list);
  
  		/* Deparse inner relation */
! 		initStringInfo(&join_sql_i);
! 		deparseRangeTblRef(&join_sql_i, root,
! 						   fpinfo->innerrel,
! 						   fpinfo->make_innerrel_subquery,
! 						   params_list);
  
  		/*
  		 * For a join relation FROM clause entry is deparsed as
--- 1476,1527 ----
  	{
  		StringInfoData join_sql_o;
  		StringInfoData join_sql_i;
+ 		bool		deparse_outerrel = true;
+ 		bool		deparse_innerrel = true;
+ 
+ 		if (target_rel > 0)
+ 		{
+ 			if (fpinfo->outerrel->reloptkind == RELOPT_BASEREL &&
+ 				fpinfo->outerrel->relid == target_rel)
+ 				deparse_outerrel = false;
+ 			if (fpinfo->innerrel->reloptkind == RELOPT_BASEREL &&
+ 				fpinfo->innerrel->relid == target_rel)
+ 				deparse_innerrel = false;
+ 		}
  
  		/* Deparse outer relation */
! 		if (deparse_outerrel)
! 		{
! 			initStringInfo(&join_sql_o);
! 			deparseRangeTblRef(&join_sql_o, root,
! 							   fpinfo->outerrel,
! 							   fpinfo->make_outerrel_subquery,
! 							   target_rel,
! 							   params_list);
! 		}
! 		if (!deparse_innerrel)
! 		{
! 			Assert(deparse_outerrel);
! 			appendStringInfo(buf, "%s", join_sql_o.data);
! 			return;
! 		}
  
  		/* Deparse inner relation */
! 		if (deparse_innerrel)
! 		{
! 			initStringInfo(&join_sql_i);
! 			deparseRangeTblRef(&join_sql_i, root,
! 							   fpinfo->innerrel,
! 							   fpinfo->make_innerrel_subquery,
! 							   target_rel,
! 							   params_list);
! 		}
! 		if (!deparse_outerrel)
! 		{
! 			Assert(deparse_innerrel);
! 			appendStringInfo(buf, "%s", join_sql_i.data);
! 			return;
! 		}
  
  		/*
  		 * For a join relation FROM clause entry is deparsed as
***************
*** 1525,1531 **** deparseFromExprForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
   */
  static void
  deparseRangeTblRef(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
! 				   bool make_subquery, List **params_list)
  {
  	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
  
--- 1581,1587 ----
   */
  static void
  deparseRangeTblRef(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
! 				   bool make_subquery, Index target_rel, List **params_list)
  {
  	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
  
***************
*** 1535,1555 **** deparseRangeTblRef(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
  
  	if (make_subquery)
  	{
! 		List	   *tlist;
  		List	   *retrieved_attrs;
  
! 		tlist = make_tlist_from_pathtarget(foreignrel->reltarget);
  		Assert(tlist != NIL);
  		appendStringInfoChar(buf, '(');
  		deparseSelectStmtForRel(buf, root, foreignrel, tlist,
  								fpinfo->remote_conds, NIL,
  								&retrieved_attrs, params_list);
  		appendStringInfoChar(buf, ')');
  		appendSubselectAlias(buf, fpinfo->relation_index,
! 							 list_length(foreignrel->reltarget->exprs));
  	}
  	else
! 		deparseFromExprForRel(buf, root, foreignrel, true, params_list);
  }
  
  /*
--- 1591,1613 ----
  
  	if (make_subquery)
  	{
! 		List	   *tlist = NIL;
  		List	   *retrieved_attrs;
  
! 		tlist = add_to_flat_tlist(tlist, fpinfo->subquery_exprs);
  		Assert(tlist != NIL);
+ 		Assert(list_length(tlist) == list_length(fpinfo->subquery_exprs));
  		appendStringInfoChar(buf, '(');
  		deparseSelectStmtForRel(buf, root, foreignrel, tlist,
  								fpinfo->remote_conds, NIL,
  								&retrieved_attrs, params_list);
  		appendStringInfoChar(buf, ')');
  		appendSubselectAlias(buf, fpinfo->relation_index,
! 							 list_length(tlist));
  	}
  	else
! 		deparseFromExprForRel(buf, root, foreignrel, true, target_rel,
! 							  params_list);
  }
  
  /*
***************
*** 1598,1604 **** getSubselectAliasInfo(Expr *node, RelOptInfo *foreignrel,
  
  	/* Get the column number */
  	i = 1;
! 	foreach(lc, foreignrel->reltarget->exprs)
  	{
  		if (equal(lfirst(lc), (Node *) node))
  		{
--- 1656,1662 ----
  
  	/* Get the column number */
  	i = 1;
! 	foreach(lc, fpinfo->subquery_exprs)
  	{
  		if (equal(lfirst(lc), (Node *) node))
  		{
***************
*** 1805,1810 **** deparseUpdateSql(StringInfo buf, PlannerInfo *root,
--- 1863,1870 ----
  void
  deparseDirectUpdateSql(StringInfo buf, PlannerInfo *root,
  					   Index rtindex, Relation rel,
+ 					   RelOptInfo *foreignrel,
+ 					   List **fdw_scan_tlist,
  					   List *targetlist,
  					   List *targetAttrs,
  					   List *remote_conds,
***************
*** 1812,1832 **** deparseDirectUpdateSql(StringInfo buf, PlannerInfo *root,
  					   List *returningList,
  					   List **retrieved_attrs)
  {
- 	RelOptInfo *baserel = root->simple_rel_array[rtindex];
  	deparse_expr_cxt context;
  	int			nestlevel;
  	bool		first;
  	ListCell   *lc;
  
  	/* Set up context struct for recursion */
  	context.root = root;
! 	context.foreignrel = baserel;
! 	context.scanrel = baserel;
  	context.buf = buf;
  	context.params_list = params_list;
  
  	appendStringInfoString(buf, "UPDATE ");
  	deparseRelation(buf, rel);
  	appendStringInfoString(buf, " SET ");
  
  	/* Make sure any constants in the exprs are printed portably */
--- 1872,1906 ----
  					   List *returningList,
  					   List **retrieved_attrs)
  {
  	deparse_expr_cxt context;
+ 	List	   *rlist = NIL;
+ 	List	   *target_conds = NIL;
+ 	Relids		wholerows = NULL;
  	int			nestlevel;
  	bool		first;
  	ListCell   *lc;
  
+ 	if (foreignrel->reloptkind == RELOPT_JOINREL)
+ 	{
+ 		rlist = makeExplicitReturningList(rtindex, rel,
+ 										  returningList,
+ 										  fdw_scan_tlist,
+ 										  &wholerows);
+ 		rewriteFromExprForRel(root, foreignrel, rtindex,
+ 							  &target_conds, wholerows);
+ 	}
+ 
  	/* Set up context struct for recursion */
  	context.root = root;
! 	context.foreignrel = foreignrel;
! 	context.scanrel = foreignrel;
  	context.buf = buf;
  	context.params_list = params_list;
  
  	appendStringInfoString(buf, "UPDATE ");
  	deparseRelation(buf, rel);
+ 	if (foreignrel->reloptkind == RELOPT_JOINREL)
+ 		appendStringInfo(buf, " %s%d", REL_ALIAS_PREFIX, rtindex);
  	appendStringInfoString(buf, " SET ");
  
  	/* Make sure any constants in the exprs are printed portably */
***************
*** 1853,1866 **** deparseDirectUpdateSql(StringInfo buf, PlannerInfo *root,
  
  	reset_transmission_modes(nestlevel);
  
  	if (remote_conds)
  	{
  		appendStringInfo(buf, " WHERE ");
  		appendConditions(remote_conds, &context);
  	}
  
! 	deparseReturningList(buf, root, rtindex, rel, false,
! 						 returningList, retrieved_attrs);
  }
  
  /*
--- 1927,1951 ----
  
  	reset_transmission_modes(nestlevel);
  
+ 	if (foreignrel->reloptkind == RELOPT_JOINREL)
+ 	{
+ 		appendStringInfo(buf, " FROM ");
+ 		deparseFromExprForRel(buf, root, foreignrel, true, rtindex,
+ 							  params_list);
+ 		remote_conds = list_concat(list_copy(remote_conds), target_conds);
+ 	}
+ 
  	if (remote_conds)
  	{
  		appendStringInfo(buf, " WHERE ");
  		appendConditions(remote_conds, &context);
  	}
  
! 	if (foreignrel->reloptkind == RELOPT_JOINREL)
! 		deparseExplicitReturningList(rlist, retrieved_attrs, &context);
! 	else
! 		deparseReturningList(buf, root, rtindex, rel, false,
! 							 returningList, retrieved_attrs);
  }
  
  /*
***************
*** 1895,1917 **** deparseDeleteSql(StringInfo buf, PlannerInfo *root,
  void
  deparseDirectDeleteSql(StringInfo buf, PlannerInfo *root,
  					   Index rtindex, Relation rel,
  					   List *remote_conds,
  					   List **params_list,
  					   List *returningList,
  					   List **retrieved_attrs)
  {
- 	RelOptInfo *baserel = root->simple_rel_array[rtindex];
  	deparse_expr_cxt context;
  
  	/* Set up context struct for recursion */
  	context.root = root;
! 	context.foreignrel = baserel;
! 	context.scanrel = baserel;
  	context.buf = buf;
  	context.params_list = params_list;
  
  	appendStringInfoString(buf, "DELETE FROM ");
  	deparseRelation(buf, rel);
  
  	if (remote_conds)
  	{
--- 1980,2026 ----
  void
  deparseDirectDeleteSql(StringInfo buf, PlannerInfo *root,
  					   Index rtindex, Relation rel,
+ 					   RelOptInfo *foreignrel,
+ 					   List **fdw_scan_tlist,
  					   List *remote_conds,
  					   List **params_list,
  					   List *returningList,
  					   List **retrieved_attrs)
  {
  	deparse_expr_cxt context;
+ 	List	   *rlist = NIL;
+ 	List	   *target_conds = NIL;
+ 	Relids		wholerows = NULL;
+ 
+ 	if (foreignrel->reloptkind == RELOPT_JOINREL)
+ 	{
+ 		rlist = makeExplicitReturningList(rtindex, rel,
+ 										  returningList,
+ 										  fdw_scan_tlist,
+ 										  &wholerows);
+ 		rewriteFromExprForRel(root, foreignrel, rtindex,
+ 							  &target_conds, wholerows);
+ 	}
  
  	/* Set up context struct for recursion */
  	context.root = root;
! 	context.foreignrel = foreignrel;
! 	context.scanrel = foreignrel;
  	context.buf = buf;
  	context.params_list = params_list;
  
  	appendStringInfoString(buf, "DELETE FROM ");
  	deparseRelation(buf, rel);
+ 	if (foreignrel->reloptkind == RELOPT_JOINREL)
+ 		appendStringInfo(buf, " %s%d", REL_ALIAS_PREFIX, rtindex);
+ 
+ 	if (foreignrel->reloptkind == RELOPT_JOINREL)
+ 	{
+ 		appendStringInfo(buf, " USING ");
+ 		deparseFromExprForRel(buf, root, foreignrel, true, rtindex,
+ 							  params_list);
+ 		remote_conds = list_concat(list_copy(remote_conds), target_conds);
+ 	}
  
  	if (remote_conds)
  	{
***************
*** 1919,1926 **** deparseDirectDeleteSql(StringInfo buf, PlannerInfo *root,
  		appendConditions(remote_conds, &context);
  	}
  
! 	deparseReturningList(buf, root, rtindex, rel, false,
! 						 returningList, retrieved_attrs);
  }
  
  /*
--- 2028,2038 ----
  		appendConditions(remote_conds, &context);
  	}
  
! 	if (foreignrel->reloptkind == RELOPT_JOINREL)
! 		deparseExplicitReturningList(rlist, retrieved_attrs, &context);
! 	else
! 		deparseReturningList(buf, root, rtindex, rel, false,
! 							 returningList, retrieved_attrs);
  }
  
  /*
***************
*** 1960,1965 **** deparseReturningList(StringInfo buf, PlannerInfo *root,
--- 2072,2403 ----
  }
  
  /*
+  * Add a RETURNING clause, if needed, to an UPDATE/DELETE on a join.
+  */
+ static void
+ deparseExplicitReturningList(List *rlist,
+ 							 List **retrieved_attrs,
+ 							 deparse_expr_cxt *context)
+ {
+ 	deparseExplicitTargetList(true, rlist, retrieved_attrs, context);
+ }
+ 
+ /*
+  * Create a RETURNING list, if executing an UPDATE/DELETE on a join remotely.
+  */
+ static List *
+ makeExplicitReturningList(Index rtindex, Relation rel,
+ 						  List *returningList,
+ 						  List **fdw_scan_tlist,
+ 						  Relids *wholerows)
+ {
+ 	TupleDesc	tupdesc = RelationGetDescr(rel);
+ 	List	   *rlist;
+ 	List	   *vars;
+ 	ListCell   *lc;
+ 
+ 	*wholerows = NULL;
+ 
+ 	if (returningList == NIL)
+ 		return NIL;
+ 
+ 	vars = pull_var_clause((Node *) returningList, PVC_INCLUDE_PLACEHOLDERS);
+ 
+ 	foreach(lc, vars)
+ 	{
+ 		Var		   *var = (Var *) lfirst(lc);
+ 
+ 		if (IsA(var, Var) &&
+ 			var->varattno == InvalidAttrNumber)
+ 			*wholerows = bms_add_member(*wholerows, var->varno);
+ 	}
+ 
+ 	/* If so, we'll need all non-system columns of the result relation. */
+ 	rlist = NIL;
+ 	if (bms_is_member(rtindex, *wholerows))
+ 	{
+ 		int			i;
+ 
+ 		for (i = 1; i <= tupdesc->natts; i++)
+ 		{
+ 			Form_pg_attribute attr = tupdesc->attrs[i - 1];
+ 			Var		   *var;
+ 
+ 			/* Ignore dropped attributes. */
+ 			if (attr->attisdropped)
+ 				continue;
+ 
+ 			var = makeVar(rtindex,
+ 						  i,
+ 						  attr->atttypid,
+ 						  attr->atttypmod,
+ 						  attr->attcollation,
+ 						  0);
+ 
+ 			rlist = lappend(rlist,
+ 							makeTargetEntry((Expr *) var,
+ 											list_length(rlist) + 1,
+ 											NULL,
+ 											false));
+ 		}
+ 	}
+ 
+ 	/* Now add any remaining columns. */ 
+ 	foreach(lc, vars)
+ 	{
+ 		Var		   *var = (Var *) lfirst(lc);
+ 
+ 		if (IsA(var, Var) &&
+ 			var->varno == rtindex &&
+ 			var->varattno == InvalidAttrNumber)
+ 			continue;		/* don't need it */
+ 
+ 		if (tlist_member((Node *) var, rlist))
+ 			continue;		/* already got it */
+ 
+ 		rlist = lappend(rlist,
+ 						makeTargetEntry((Expr *) var,
+ 										list_length(rlist) + 1,
+ 										NULL,
+ 										false));
+ 	}
+ 
+ 	/* rewrite *fdw_scan_tlist */
+ 	if (rlist != NIL)
+ 	{
+ 		List	   *tlist = list_copy(rlist);
+ 
+ 		foreach(lc, *fdw_scan_tlist)
+ 		{
+ 			TargetEntry *tle = (TargetEntry *) lfirst(lc);
+ 
+ 			if (tlist_member((Node *) tle->expr, tlist))
+ 				continue;		/* already got it */
+ 
+ 			tlist = lappend(tlist,
+ 							makeTargetEntry(tle->expr,
+ 											list_length(tlist) + 1,
+ 											NULL,
+ 											false));
+ 		}
+ 
+ 		*fdw_scan_tlist = tlist;
+ 	}
+ 
+ 	list_free(vars);
+ 
+ 	return rlist;
+ }
+ 
+ static void
+ rewriteFromExprForRel(PlannerInfo *root, RelOptInfo *foreignrel,
+ 					  Index target_rel, List **target_conds, Relids wholerows)
+ {
+ 	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
+ 	RelOptInfo *outerrel = fpinfo->outerrel;
+ 	RelOptInfo *innerrel = fpinfo->innerrel;
+ 	JoinType	jointype = fpinfo->jointype;
+ 
+ 	Assert(foreignrel->reloptkind == RELOPT_JOINREL);
+ 
+ 	if (jointype == JOIN_INNER)
+ 	{
+ 		Assert(fpinfo->remote_conds == NIL);
+ 
+ 		fpinfo->joinclauses = pullUpTargetConds(foreignrel,
+ 												fpinfo->joinclauses,
+ 												target_rel,
+ 												target_conds);
+ 	}
+ 
+ 	if (fpinfo->make_outerrel_subquery)
+ 	{
+ 		fpinfo->make_outerrel_subquery = !pullUpSubquery(root,
+ 														 outerrel,
+ 														 jointype,
+ 														 target_rel,
+ 														 target_conds,
+ 														 wholerows);
+ 	}
+ 	if (fpinfo->make_innerrel_subquery)
+ 	{
+ 		fpinfo->make_innerrel_subquery = !pullUpSubquery(root,
+ 														 innerrel,
+ 														 jointype,
+ 														 target_rel,
+ 														 target_conds,
+ 														 wholerows);
+ 	}
+ 
+ 	if (outerrel->reloptkind == RELOPT_JOINREL)
+ 		rewriteFromExprForRel(root, outerrel, target_rel, target_conds,
+ 							  wholerows);
+ 	if (innerrel->reloptkind == RELOPT_JOINREL)
+ 		rewriteFromExprForRel(root, innerrel, target_rel, target_conds,
+ 							  wholerows);
+ }
+ 
+ static List *
+ pullUpTargetConds(RelOptInfo *foreignrel, List *joinclauses,
+ 				  Index target_rel, List **target_conds)
+ {
+ 	List	   *result = NIL;
+ 	ListCell   *lc;
+ 
+ 	if (!bms_is_member(target_rel, foreignrel->relids))
+ 		return joinclauses;
+ 
+ 	foreach(lc, joinclauses)
+ 	{
+ 		Node	   *clause = (Node *) lfirst(lc);
+ 		Relids		relids;
+ 
+ 		if (IsA(clause, RestrictInfo))
+ 		{
+ 			RestrictInfo *ri = (RestrictInfo *) clause;
+ 
+ 			clause = (Node *) ri->clause;
+ 		}
+ 
+ 		relids = pull_varnos(clause);
+ 
+ 		if (bms_is_member(target_rel, relids))
+ 			*target_conds = lappend(*target_conds, clause);
+ 		else
+ 			result = lappend(result, clause);
+ 	}
+ 	return result;
+ }
+ 
+ static bool
+ pullUpSubquery(PlannerInfo *root, RelOptInfo *foreignrel, JoinType jointype,
+ 			   Index target_rel, List **target_conds, Relids wholerows)
+ {
+ 	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
+ 
+ 	if (foreignrel->reloptkind == RELOPT_JOINREL)
+ 	{
+ 		Assert(!bms_is_member(target_rel, foreignrel->relids));
+ 		fpinfo->subquery_exprs = getCleanSubqueryExprs(root,
+ 													   foreignrel,
+ 													   fpinfo->subquery_exprs,
+ 													   wholerows);
+ 		return false;
+ 	}
+ 
+ 	Assert(foreignrel->reloptkind == RELOPT_BASEREL);
+ 
+ 	if (foreignrel->relid != target_rel)
+ 	{
+ 		if (isSimpleSubquery(root, foreignrel, jointype, wholerows))
+ 		{
+ 			fpinfo->subquery_exprs = NIL;
+ 			*target_conds = list_concat(*target_conds,
+ 										list_copy(fpinfo->remote_conds));
+ 			return true;
+ 		}
+ 
+ 		Assert(!bms_is_member(target_rel, foreignrel->relids));
+ 		fpinfo->subquery_exprs = getCleanSubqueryExprs(root,
+ 													   foreignrel,
+ 													   fpinfo->subquery_exprs,
+ 													   wholerows);
+ 		return false;
+ 	}
+ 
+ 	fpinfo->subquery_exprs = NIL;
+ 	*target_conds = list_concat(*target_conds,
+ 								list_copy(fpinfo->remote_conds));
+ 	return true;
+ }
+ 
+ static bool
+ isSimpleSubquery(PlannerInfo *root, RelOptInfo *baserel, JoinType jointype,
+ 				 Relids wholerows)
+ {
+ 	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) baserel->fdw_private;
+ 	int			i;
+ 	ListCell   *lc;
+ 
+ 	if (jointype == JOIN_FULL && fpinfo->remote_conds)
+ 		return false;
+ 
+ 	if (bms_is_member(baserel->relid, wholerows))
+ 		return false;
+ 
+ 	for (i = baserel->min_attr; i <= 0; i++)
+ 	{
+ 		Relids		relids;
+ 
+ 		if (i == SelfItemPointerAttributeNumber ||
+ 			i == ObjectIdAttributeNumber)
+ 			continue;
+ 
+ 		relids = baserel->attr_needed[i - baserel->min_attr];
+ 
+ 		if (i != 0)
+ 		{
+ 			if (!bms_is_empty(relids))
+ 				return false;
+ 		}
+ 		else
+ 		{
+ 			Assert(bms_is_member(0, relids));
+ 			relids = bms_copy(relids);
+ 			relids = bms_del_member(relids, 0);
+ 
+ 			if (bms_nonempty_difference(relids, baserel->relids))
+ 				return false;
+ 		}
+ 	}
+ 
+ 	foreach(lc, root->placeholder_list)
+ 	{
+ 		PlaceHolderInfo *phinfo = (PlaceHolderInfo *) lfirst(lc);
+ 
+ 		if (bms_nonempty_difference(phinfo->ph_needed, baserel->relids) &&
+ 			bms_is_subset(phinfo->ph_eval_at, baserel->relids))
+ 			return false;
+ 	}
+ 
+ 	return true;
+ }
+ 
+ static List *
+ getCleanSubqueryExprs(PlannerInfo *root, RelOptInfo *foreignrel,
+ 					  List *subquery_exprs, Relids wholerows)
+ {
+ 	List	   *result = NIL;
+ 	ListCell   *lc;
+ 
+ 	foreach(lc, subquery_exprs)
+ 	{
+ 		Var		   *var = (Var *) lfirst(lc);
+ 
+ 		if (IsA(var, Var) && var->varattno == 0)
+ 		{
+ 			if (bms_is_member(var->varno, wholerows))		
+ 				result = lappend(result, var);
+ 			else
+ 			{
+ 				RelOptInfo *baserel = root->simple_rel_array[var->varno];
+ 				Relids		relids = baserel->attr_needed[0 - baserel->min_attr];
+ 
+ 				Assert(bms_is_member(0, relids));
+ 				relids = bms_copy(relids);
+ 			    relids = bms_del_member(relids, 0);
+ 
+ 				if (bms_nonempty_difference(relids, foreignrel->relids))
+ 					result = lappend(result, var);
+ 			}
+ 		}
+ 		else
+ 			result = lappend(result, var);
+ 	}
+ 	return result;
+ }
+ 
+ /*
   * Construct SELECT statement to acquire size in blocks of given relation.
   *
   * Note: we use local definition of block size, not remote definition.
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
***************
*** 4110,4136 **** UPDATE ft2 SET c2 = c2 + 400, c3 = c3 || '_update7' WHERE c1 % 10 = 7 RETURNING
  
  EXPLAIN (verbose, costs off)
  UPDATE ft2 SET c2 = ft2.c2 + 500, c3 = ft2.c3 || '_update9', c7 = DEFAULT
!   FROM ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 9;                               -- can't be pushed down
!                                                                                                                                       QUERY PLAN                                                                                                                                      
! --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
   Update on public.ft2
!    Remote SQL: UPDATE "S 1"."T 1" SET c2 = $2, c3 = $3, c7 = $4 WHERE ctid = $1
!    ->  Foreign Scan
!          Output: ft2.c1, (ft2.c2 + 500), NULL::integer, (ft2.c3 || '_update9'::text), ft2.c4, ft2.c5, ft2.c6, 'ft2       '::character(10), ft2.c8, ft2.ctid, ft1.*
!          Relations: (public.ft2) INNER JOIN (public.ft1)
!          Remote SQL: SELECT r1."C 1", r1.c2, r1.c3, r1.c4, r1.c5, r1.c6, r1.c8, r1.ctid, s2.c1 FROM ("S 1"."T 1" r1 INNER JOIN (SELECT ROW("C 1", c2, c3, c4, c5, c6, c7, c8), "C 1" FROM "S 1"."T 1" WHERE ((("C 1" % 10) = 9))) s2(c1, c2) ON (((r1.c2 = s2.c2)))) FOR UPDATE OF r1
!          ->  Hash Join
!                Output: ft2.c1, ft2.c2, ft2.c3, ft2.c4, ft2.c5, ft2.c6, ft2.c8, ft2.ctid, ft1.*
!                Hash Cond: (ft2.c2 = ft1.c1)
!                ->  Foreign Scan on public.ft2
!                      Output: ft2.c1, ft2.c2, ft2.c3, ft2.c4, ft2.c5, ft2.c6, ft2.c8, ft2.ctid
!                      Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c8, ctid FROM "S 1"."T 1" FOR UPDATE
!                ->  Hash
!                      Output: ft1.*, ft1.c1
!                      ->  Foreign Scan on public.ft1
!                            Output: ft1.*, ft1.c1
!                            Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE ((("C 1" % 10) = 9))
! (17 rows)
  
  UPDATE ft2 SET c2 = ft2.c2 + 500, c3 = ft2.c3 || '_update9', c7 = DEFAULT
    FROM ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 9;
--- 4110,4122 ----
  
  EXPLAIN (verbose, costs off)
  UPDATE ft2 SET c2 = ft2.c2 + 500, c3 = ft2.c3 || '_update9', c7 = DEFAULT
!   FROM ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 9;                               -- can be pushed down
!                                                                                                    QUERY PLAN                                                                                                    
! -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
   Update on public.ft2
!    ->  Foreign Update
!          Remote SQL: UPDATE "S 1"."T 1" r1 SET c2 = (r1.c2 + 500), c3 = (r1.c3 || '_update9'::text), c7 = 'ft2       '::character(10) FROM "S 1"."T 1" r2 WHERE ((r1.c2 = r2."C 1")) AND (((r2."C 1" % 10) = 9))
! (3 rows)
  
  UPDATE ft2 SET c2 = ft2.c2 + 500, c3 = ft2.c3 || '_update9', c7 = DEFAULT
    FROM ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 9;
***************
*** 4253,4279 **** DELETE FROM ft2 WHERE c1 % 10 = 5 RETURNING c1, c4;
  (103 rows)
  
  EXPLAIN (verbose, costs off)
! DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;                -- can't be pushed down
!                                                                                                             QUERY PLAN                                                                                                            
! ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
   Delete on public.ft2
!    Remote SQL: DELETE FROM "S 1"."T 1" WHERE ctid = $1
!    ->  Foreign Scan
!          Output: ft2.ctid, ft1.*
!          Relations: (public.ft2) INNER JOIN (public.ft1)
!          Remote SQL: SELECT r1.ctid, s2.c1 FROM ("S 1"."T 1" r1 INNER JOIN (SELECT ROW("C 1", c2, c3, c4, c5, c6, c7, c8), "C 1" FROM "S 1"."T 1" WHERE ((("C 1" % 10) = 2))) s2(c1, c2) ON (((r1.c2 = s2.c2)))) FOR UPDATE OF r1
!          ->  Hash Join
!                Output: ft2.ctid, ft1.*
!                Hash Cond: (ft2.c2 = ft1.c1)
!                ->  Foreign Scan on public.ft2
!                      Output: ft2.ctid, ft2.c2
!                      Remote SQL: SELECT c2, ctid FROM "S 1"."T 1" FOR UPDATE
!                ->  Hash
!                      Output: ft1.*, ft1.c1
!                      ->  Foreign Scan on public.ft1
!                            Output: ft1.*, ft1.c1
!                            Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE ((("C 1" % 10) = 2))
! (17 rows)
  
  DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;
  SELECT c1,c2,c3,c4 FROM ft2 ORDER BY c1;
--- 4239,4251 ----
  (103 rows)
  
  EXPLAIN (verbose, costs off)
! DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;                -- can be pushed down
!                                                          QUERY PLAN                                                         
! ----------------------------------------------------------------------------------------------------------------------------
   Delete on public.ft2
!    ->  Foreign Delete
!          Remote SQL: DELETE FROM "S 1"."T 1" r1 USING "S 1"."T 1" r2 WHERE ((r1.c2 = r2."C 1")) AND (((r2."C 1" % 10) = 2))
! (3 rows)
  
  DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;
  SELECT c1,c2,c3,c4 FROM ft2 ORDER BY c1;
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
***************
*** 212,217 **** typedef struct PgFdwDirectModifyState
--- 212,223 ----
  	PGresult   *result;			/* result for query */
  	int			num_tuples;		/* # of result tuples */
  	int			next_tuple;		/* index of next one to return */
+ 	Relation	resultRel;		/* relcache entry for the target table */
+ 	TupleTableSlot *resultSlot;	/* slot for updated/deleted tuples */
+ 	AttrNumber *attnoMap;		/* array of resultRel attr numbers */
+ 	AttrNumber	ctidAttno;		/* attnum of ctid of result tuple */
+ 	AttrNumber	oidAttno;		/* attnum of oid of result tuple */
+ 	bool		hasSystemCols;	/* are there system columns of resultRel? */
  
  	/* working memory context */
  	MemoryContext temp_cxt;		/* context for per-tuple temporary data */
***************
*** 380,385 **** static void store_returning_result(PgFdwModifyState *fmstate,
--- 386,397 ----
  					   TupleTableSlot *slot, PGresult *res);
  static void execute_dml_stmt(ForeignScanState *node);
  static TupleTableSlot *get_returning_data(ForeignScanState *node);
+ static void init_returning_filter(PgFdwDirectModifyState *dmstate,
+ 					  List *fdw_scan_tlist,
+ 					  Index rtindex,
+ 					  EState *estate);
+ static TupleTableSlot *apply_returning_filter(PgFdwDirectModifyState *dmstate,
+ 					   TupleTableSlot *slot);
  static void prepare_query_params(PlanState *node,
  					 List *fdw_exprs,
  					 int numParams,
***************
*** 722,727 **** postgresGetForeignRelSize(PlannerInfo *root,
--- 734,742 ----
  
  	/* Set the relation index */
  	fpinfo->relation_index = baserel->relid;
+ 
+ 	/* Initialize the subquery output columns */
+ 	fpinfo->subquery_exprs = NIL;
  }
  
  /*
***************
*** 2162,2167 **** postgresPlanDirectModify(PlannerInfo *root,
--- 2177,2183 ----
  	Relation	rel;
  	StringInfoData sql;
  	ForeignScan *fscan;
+ 	RelOptInfo *foreignrel;
  	List	   *targetAttrs = NIL;
  	List	   *remote_conds;
  	List	   *params_list = NIL;
***************
*** 2193,2205 **** postgresPlanDirectModify(PlannerInfo *root,
  		return false;
  
  	/*
- 	 * We can't handle an UPDATE or DELETE on a foreign join for now.
- 	 */
- 	fscan = (ForeignScan *) subplan;
- 	if (fscan->scan.scanrelid == 0)
- 		return false;
- 
- 	/*
  	 * It's unsafe to update a foreign table directly, if any expressions to
  	 * assign to the target columns are unsafe to evaluate remotely.
  	 */
--- 2209,2214 ----
***************
*** 2238,2243 **** postgresPlanDirectModify(PlannerInfo *root,
--- 2247,2254 ----
  	/*
  	 * Ok, rewrite subplan so as to modify the foreign table directly.
  	 */
+ 	fscan = (ForeignScan *) subplan;
+ 
  	initStringInfo(&sql);
  
  	/*
***************
*** 2247,2252 **** postgresPlanDirectModify(PlannerInfo *root,
--- 2258,2275 ----
  	rel = heap_open(rte->relid, NoLock);
  
  	/*
+ 	 * Get a rel for this foreign table or join.
+ 	 */
+ 	if (fscan->scan.scanrelid == 0)
+ 	{
+ 		/* We should have a rel for this foreign join. */
+ 		foreignrel = find_join_rel(root, fscan->fs_relids);
+ 		Assert(foreignrel);
+ 	}
+ 	else
+ 		foreignrel = find_base_rel(root, resultRelation);
+ 
+ 	/*
  	 * Extract the baserestrictinfo clauses that can be evaluated remotely.
  	 */
  	remote_conds = (List *) list_nth(fscan->fdw_private,
***************
*** 2265,2270 **** postgresPlanDirectModify(PlannerInfo *root,
--- 2288,2295 ----
  	{
  		case CMD_UPDATE:
  			deparseDirectUpdateSql(&sql, root, resultRelation, rel,
+ 								   foreignrel,
+ 								   &fscan->fdw_scan_tlist,
  								   ((Plan *) fscan)->targetlist,
  								   targetAttrs,
  								   remote_conds, &params_list,
***************
*** 2272,2277 **** postgresPlanDirectModify(PlannerInfo *root,
--- 2297,2304 ----
  			break;
  		case CMD_DELETE:
  			deparseDirectDeleteSql(&sql, root, resultRelation, rel,
+ 								   foreignrel,
+ 								   &fscan->fdw_scan_tlist,
  								   remote_conds, &params_list,
  								   returningList, &retrieved_attrs);
  			break;
***************
*** 2299,2304 **** postgresPlanDirectModify(PlannerInfo *root,
--- 2326,2337 ----
  									retrieved_attrs,
  									makeInteger(plan->canSetTag));
  
+ 	/*
+ 	 * We don't need the outer subplan.
+ 	 */
+ 	if (fscan->scan.scanrelid == 0)
+ 		fscan->scan.plan.lefttree = NULL;
+ 
  	heap_close(rel, NoLock);
  	return true;
  }
***************
*** 2313,2318 **** postgresBeginDirectModify(ForeignScanState *node, int eflags)
--- 2346,2352 ----
  	ForeignScan *fsplan = (ForeignScan *) node->ss.ps.plan;
  	EState	   *estate = node->ss.ps.state;
  	PgFdwDirectModifyState *dmstate;
+ 	Index		rtindex;
  	RangeTblEntry *rte;
  	Oid			userid;
  	ForeignTable *table;
***************
*** 2335,2345 **** postgresBeginDirectModify(ForeignScanState *node, int eflags)
  	 * Identify which user to do the remote access as.  This should match what
  	 * ExecCheckRTEPerms() does.
  	 */
! 	rte = rt_fetch(fsplan->scan.scanrelid, estate->es_range_table);
  	userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
  
  	/* Get info about foreign table. */
! 	dmstate->rel = node->ss.ss_currentRelation;
  	table = GetForeignTable(RelationGetRelid(dmstate->rel));
  	user = GetUserMapping(userid, table->serverid);
  
--- 2369,2383 ----
  	 * Identify which user to do the remote access as.  This should match what
  	 * ExecCheckRTEPerms() does.
  	 */
! 	rtindex = estate->es_result_relation_info->ri_RangeTableIndex;
! 	rte = rt_fetch(rtindex, estate->es_range_table);
  	userid = rte->checkAsUser ? rte->checkAsUser : GetUserId();
  
  	/* Get info about foreign table. */
! 	if (fsplan->scan.scanrelid > 0)
! 		dmstate->rel = node->ss.ss_currentRelation;
! 	else
! 		dmstate->rel = ExecOpenScanRelation(estate, rtindex, eflags);
  	table = GetForeignTable(RelationGetRelid(dmstate->rel));
  	user = GetUserMapping(userid, table->serverid);
  
***************
*** 2349,2354 **** postgresBeginDirectModify(ForeignScanState *node, int eflags)
--- 2387,2401 ----
  	 */
  	dmstate->conn = GetConnection(user, false);
  
+ 	if (fsplan->scan.scanrelid == 0)
+ 	{
+ 		/* Save info about target table. */
+ 		dmstate->resultRel = dmstate->rel;
+ 
+ 		/* rel should be NULL if foreign join. */
+ 		dmstate->rel = NULL;
+ 	}
+ 
  	/* Initialize state variable */
  	dmstate->num_tuples = -1;	/* -1 means not set yet */
  
***************
*** 2369,2375 **** postgresBeginDirectModify(ForeignScanState *node, int eflags)
  
  	/* Prepare for input conversion of RETURNING results. */
  	if (dmstate->has_returning)
! 		dmstate->attinmeta = TupleDescGetAttInMetadata(RelationGetDescr(dmstate->rel));
  
  	/*
  	 * Prepare for processing of parameters used in remote query, if any.
--- 2416,2436 ----
  
  	/* Prepare for input conversion of RETURNING results. */
  	if (dmstate->has_returning)
! 	{
! 		TupleDesc	tupdesc;
! 
! 		if (fsplan->scan.scanrelid > 0)
! 			tupdesc = RelationGetDescr(dmstate->rel);
! 		else
! 			tupdesc = node->ss.ss_ScanTupleSlot->tts_tupleDescriptor;
! 
! 		dmstate->attinmeta = TupleDescGetAttInMetadata(tupdesc);
! 
! 		/* Initialize a filter to extract an updated/deleted tuple. */
! 		if (fsplan->scan.scanrelid == 0)
! 			init_returning_filter(dmstate, fsplan->fdw_scan_tlist,
! 								  rtindex, estate);
! 	}
  
  	/*
  	 * Prepare for processing of parameters used in remote query, if any.
***************
*** 2450,2455 **** postgresEndDirectModify(ForeignScanState *node)
--- 2511,2520 ----
  	ReleaseConnection(dmstate->conn);
  	dmstate->conn = NULL;
  
+ 	/* close the result relation. */
+ 	if (dmstate->resultRel)
+ 		ExecCloseScanRelation(dmstate->resultRel);
+ 
  	/* MemoryContext will be deleted automatically. */
  }
  
***************
*** 3377,3382 **** get_returning_data(ForeignScanState *node)
--- 3442,3448 ----
  	EState	   *estate = node->ss.ps.state;
  	ResultRelInfo *resultRelInfo = estate->es_result_relation_info;
  	TupleTableSlot *slot = node->ss.ss_ScanTupleSlot;
+ 	TupleTableSlot *resultSlot;
  
  	Assert(resultRelInfo->ri_projectReturning);
  
***************
*** 3394,3400 **** get_returning_data(ForeignScanState *node)
--- 3460,3469 ----
  	 * "UPDATE/DELETE .. RETURNING 1" for example.)
  	 */
  	if (!dmstate->has_returning)
+ 	{
  		ExecStoreAllNullTuple(slot);
+ 		resultSlot = slot;
+ 	}
  	else
  	{
  		/*
***************
*** 3410,3416 **** get_returning_data(ForeignScanState *node)
  												dmstate->rel,
  												dmstate->attinmeta,
  												dmstate->retrieved_attrs,
! 												NULL,
  												dmstate->temp_cxt);
  			ExecStoreTuple(newtup, slot, InvalidBuffer, false);
  		}
--- 3479,3485 ----
  												dmstate->rel,
  												dmstate->attinmeta,
  												dmstate->retrieved_attrs,
! 												node,
  												dmstate->temp_cxt);
  			ExecStoreTuple(newtup, slot, InvalidBuffer, false);
  		}
***************
*** 3421,3436 **** get_returning_data(ForeignScanState *node)
  			PG_RE_THROW();
  		}
  		PG_END_TRY();
  	}
  	dmstate->next_tuple++;
  
  	/* Make slot available for evaluation of the local query RETURNING list. */
! 	resultRelInfo->ri_projectReturning->pi_exprContext->ecxt_scantuple = slot;
  
  	return slot;
  }
  
  /*
   * Prepare for processing of parameters used in remote query.
   */
  static void
--- 3490,3677 ----
  			PG_RE_THROW();
  		}
  		PG_END_TRY();
+ 
+ 		/* Get the updated/deleted tuple. */
+ 		if (dmstate->rel)
+ 			resultSlot = slot;
+ 		else
+ 			resultSlot = apply_returning_filter(dmstate, slot);
  	}
  	dmstate->next_tuple++;
  
  	/* Make slot available for evaluation of the local query RETURNING list. */
! 	resultRelInfo->ri_projectReturning->pi_exprContext->ecxt_scantuple = resultSlot;
  
  	return slot;
  }
  
  /*
+  * Initialize a filter to extract an updated/deleted tuple from a scan tuple.
+  */
+ static void
+ init_returning_filter(PgFdwDirectModifyState *dmstate,
+ 					  List *fdw_scan_tlist,
+ 					  Index rtindex,
+ 					  EState *estate)
+ {
+ 	TupleDesc	resultTupType = RelationGetDescr(dmstate->resultRel);
+ 	ListCell   *lc;
+ 	int			i;
+ 
+ 	/* Make a new slot for storing an output tuple. */
+ 	dmstate->resultSlot = ExecInitExtraTupleSlot(estate);
+ 	ExecSetSlotDescriptor(dmstate->resultSlot, resultTupType);
+ 
+ 	/*
+ 	 * Calculate the mapping between the fdw_scan_tlist list's entries and
+ 	 * the updated/deleted tuple's attributes.
+ 	 *
+ 	 * The "map" is an array of the resultRel attribute numbers, i.e. one
+ 	 * entry for every attribute of the updated/deleted tuple.  The value of
+ 	 * this entry is the index of the corresponding entry in fdw_scan_tlist.
+ 	 * We store zero for any attributes that don't have the corresponding
+ 	 * entries in fdw_scan_tlist (i.e. attributes that won't be retrieved
+ 	 * from the remote server), marking that a NULL is needed in the output
+ 	 * tuple.
+ 	 *
+ 	 * Also get the index of the entry for ctid/oid of the updated/deleted
+ 	 * tuple if any.
+ 	 */
+ 	dmstate->attnoMap = (AttrNumber *) palloc0(resultTupType->natts * sizeof(AttrNumber));
+ 
+ 	dmstate->ctidAttno = dmstate->oidAttno = 0;
+ 
+ 	i = 1;
+ 	dmstate->hasSystemCols = false;
+ 	foreach(lc, fdw_scan_tlist)
+ 	{
+ 		if (list_member_int(dmstate->retrieved_attrs, i))
+ 		{
+ 			TargetEntry *tle = (TargetEntry *) lfirst(lc);
+ 			Var		   *var = (Var *) tle->expr;
+ 
+ 			if (IsA(var, Var) && var->varno == rtindex)
+ 			{
+ 				int			attrno = var->varattno;
+ 
+ 				if (attrno < 0)
+ 				{
+ 					if (attrno == SelfItemPointerAttributeNumber)
+ 						dmstate->ctidAttno = i;
+ 					else if (attrno == ObjectIdAttributeNumber)
+ 						dmstate->oidAttno = i;
+ 					dmstate->hasSystemCols = true;
+ 				}
+ 				else
+ 				{
+ 					Assert(attrno > 0);
+ 					dmstate->attnoMap[attrno - 1] = i;
+ 				}
+ 			}
+ 		}
+ 		i++;
+ 	}
+ }
+ 
+ /*
+  * Extract and return an updated/deleted tuple from a scan tuple.
+  */
+ static TupleTableSlot *
+ apply_returning_filter(PgFdwDirectModifyState *dmstate,
+ 					   TupleTableSlot *slot)
+ {
+ 	TupleTableSlot *resultSlot = dmstate->resultSlot;
+ 	TupleDesc	resultTupType = RelationGetDescr(dmstate->resultRel);
+ 	HeapTuple	resultTup;
+ 	Datum	   *values;
+ 	bool	   *isnull;
+ 	Datum	   *old_values;
+ 	bool	   *old_isnull;
+ 	int			i;
+ 
+ 	/*
+ 	 * Extract all the values of the scan tuple.
+ 	 */
+ 	slot_getallattrs(slot);
+ 	old_values = slot->tts_values;
+ 	old_isnull = slot->tts_isnull;
+ 
+ 	/*
+ 	 * Prepare to build a result tuple.
+ 	 */
+ 	ExecClearTuple(resultSlot);
+ 	values = resultSlot->tts_values;
+ 	isnull = resultSlot->tts_isnull;
+ 
+ 	/*
+ 	 * Transpose data into proper fields of the result tuple.
+ 	 */
+ 	for (i = 0; i < resultTupType->natts; i++)
+ 	{
+ 		int			j = dmstate->attnoMap[i];
+ 
+ 		if (j == 0)
+ 		{
+ 			values[i] = (Datum) 0;
+ 			isnull[i] = true;
+ 		}
+ 		else
+ 		{
+ 			values[i] = old_values[j - 1];
+ 			isnull[i] = old_isnull[j - 1];
+ 		}
+ 	}
+ 
+ 	/*
+ 	 * Build the virtual result tuple.
+ 	 */
+ 	ExecStoreVirtualTuple(resultSlot);
+ 
+ 	/*
+ 	 * If we have any system columns to return, install it in t_self.
+ 	 */
+ 	if (dmstate->hasSystemCols)
+ 	{
+ 		resultTup = ExecMaterializeSlot(resultSlot);
+ 
+ 		/*
+ 		 * xmin, xmax, cmin, and tableoid
+ 		 *
+ 		 * Note: we currently don't allow the result relation to appear on
+ 		 * the nullable side of an outer join.
+ 		 */
+ 		HeapTupleHeaderSetXmin(resultTup->t_data, InvalidTransactionId);
+ 		HeapTupleHeaderSetXmax(resultTup->t_data, InvalidTransactionId);
+ 		HeapTupleHeaderSetCmin(resultTup->t_data, InvalidTransactionId);
+ 
+ 		resultTup->t_tableOid = RelationGetRelid(dmstate->resultRel);
+ 
+ 		/* ctid */
+ 		if (dmstate->ctidAttno)
+ 		{
+ 			ItemPointer ctid = NULL;
+ 
+ 			ctid = (ItemPointer) DatumGetPointer(old_values[dmstate->ctidAttno - 1]);
+ 			resultTup->t_self = *ctid;
+ 		}
+ 
+ 		/* oid */
+ 		if (dmstate->oidAttno)
+ 		{
+ 			Oid			oid = InvalidOid;
+ 
+ 			oid = DatumGetObjectId(old_values[dmstate->oidAttno - 1]);
+ 			HeapTupleSetOid(resultTup, oid);
+ 		}
+ 	}
+ 
+ 	/*
+ 	 * And return the result tuple.
+ 	 */
+ 	return resultSlot;
+ }
+ 
+ /*
   * Prepare for processing of parameters used in remote query.
   */
  static void
***************
*** 4293,4303 **** foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
--- 4534,4550 ----
  	 * that relation covered by the subquery for later use of deparser.
  	 */
  	if (fpinfo->make_outerrel_subquery)
+ 	{
  		fpinfo->subquery_rels = bms_add_members(fpinfo->subquery_rels,
  												outerrel->relids);
+ 		fpinfo_o->subquery_exprs = outerrel->reltarget->exprs;
+ 	}
  	if (fpinfo->make_innerrel_subquery)
+ 	{
  		fpinfo->subquery_rels = bms_add_members(fpinfo->subquery_rels,
  												innerrel->relids);
+ 		fpinfo_i->subquery_exprs = innerrel->reltarget->exprs;
+ 	}
  
  	/*
  	 * For an inner join, all restrictions can be treated alike. Treating the
***************
*** 4420,4425 **** foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
--- 4667,4675 ----
  	fpinfo->relation_index =
  		list_length(root->parse->rtable) + list_length(root->join_rel_list);
  
+ 	/* Initialize the subquery output columns */
+ 	fpinfo->subquery_exprs = NIL;
+ 
  	return true;
  }
  
***************
*** 4943,4953 **** make_tuple_from_result_row(PGresult *res,
  		tupdesc = RelationGetDescr(rel);
  	else
  	{
- 		PgFdwScanState *fdw_sstate;
- 
  		Assert(fsstate);
! 		fdw_sstate = (PgFdwScanState *) fsstate->fdw_state;
! 		tupdesc = fdw_sstate->tupdesc;
  	}
  
  	values = (Datum *) palloc0(tupdesc->natts * sizeof(Datum));
--- 5193,5200 ----
  		tupdesc = RelationGetDescr(rel);
  	else
  	{
  		Assert(fsstate);
! 		tupdesc = fsstate->ss.ss_ScanTupleSlot->tts_tupleDescriptor;
  	}
  
  	values = (Datum *) palloc0(tupdesc->natts * sizeof(Datum));
*** a/contrib/postgres_fdw/postgres_fdw.h
--- b/contrib/postgres_fdw/postgres_fdw.h
***************
*** 131,136 **** typedef struct PgFdwRelationInfo
--- 131,139 ----
  	 * deparsing the relation as a subquery.
  	 */
  	int			relation_index;
+ 
+ 	/* output columns of the subquery corresponding to the relation */
+ 	List	   *subquery_exprs;
  } PgFdwRelationInfo;
  
  /* in postgres_fdw.c */
***************
*** 173,178 **** extern void deparseUpdateSql(StringInfo buf, PlannerInfo *root,
--- 176,183 ----
  				 List **retrieved_attrs);
  extern void deparseDirectUpdateSql(StringInfo buf, PlannerInfo *root,
  					   Index rtindex, Relation rel,
+ 					   RelOptInfo *foreignrel,
+ 					   List **fdw_scan_tlist,
  					   List *targetlist,
  					   List *targetAttrs,
  					   List *remote_conds,
***************
*** 185,190 **** extern void deparseDeleteSql(StringInfo buf, PlannerInfo *root,
--- 190,197 ----
  				 List **retrieved_attrs);
  extern void deparseDirectDeleteSql(StringInfo buf, PlannerInfo *root,
  					   Index rtindex, Relation rel,
+ 					   RelOptInfo *foreignrel,
+ 					   List **fdw_scan_tlist,
  					   List *remote_conds,
  					   List **params_list,
  					   List *returningList,
*** a/contrib/postgres_fdw/sql/postgres_fdw.sql
--- b/contrib/postgres_fdw/sql/postgres_fdw.sql
***************
*** 1008,1021 **** UPDATE ft2 SET c2 = c2 + 400, c3 = c3 || '_update7' WHERE c1 % 10 = 7 RETURNING
  UPDATE ft2 SET c2 = c2 + 400, c3 = c3 || '_update7' WHERE c1 % 10 = 7 RETURNING *;
  EXPLAIN (verbose, costs off)
  UPDATE ft2 SET c2 = ft2.c2 + 500, c3 = ft2.c3 || '_update9', c7 = DEFAULT
!   FROM ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 9;                               -- can't be pushed down
  UPDATE ft2 SET c2 = ft2.c2 + 500, c3 = ft2.c3 || '_update9', c7 = DEFAULT
    FROM ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 9;
  EXPLAIN (verbose, costs off)
    DELETE FROM ft2 WHERE c1 % 10 = 5 RETURNING c1, c4;                               -- can be pushed down
  DELETE FROM ft2 WHERE c1 % 10 = 5 RETURNING c1, c4;
  EXPLAIN (verbose, costs off)
! DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;                -- can't be pushed down
  DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;
  SELECT c1,c2,c3,c4 FROM ft2 ORDER BY c1;
  EXPLAIN (verbose, costs off)
--- 1008,1021 ----
  UPDATE ft2 SET c2 = c2 + 400, c3 = c3 || '_update7' WHERE c1 % 10 = 7 RETURNING *;
  EXPLAIN (verbose, costs off)
  UPDATE ft2 SET c2 = ft2.c2 + 500, c3 = ft2.c3 || '_update9', c7 = DEFAULT
!   FROM ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 9;                               -- can be pushed down
  UPDATE ft2 SET c2 = ft2.c2 + 500, c3 = ft2.c3 || '_update9', c7 = DEFAULT
    FROM ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 9;
  EXPLAIN (verbose, costs off)
    DELETE FROM ft2 WHERE c1 % 10 = 5 RETURNING c1, c4;                               -- can be pushed down
  DELETE FROM ft2 WHERE c1 % 10 = 5 RETURNING c1, c4;
  EXPLAIN (verbose, costs off)
! DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;                -- can be pushed down
  DELETE FROM ft2 USING ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 2;
  SELECT c1,c2,c3,c4 FROM ft2 ORDER BY c1;
  EXPLAIN (verbose, costs off)
-- 
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