use-RestrictInfos-consistently-in-postgres_fdw.patch

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index c5149a0..1d5aa83 100644
*** a/contrib/postgres_fdw/deparse.c
--- b/contrib/postgres_fdw/deparse.c
*************** classifyConditions(PlannerInfo *root,
*** 210,216 ****
  
  	foreach(lc, input_conds)
  	{
! 		RestrictInfo *ri = (RestrictInfo *) lfirst(lc);
  
  		if (is_foreign_expr(root, baserel, ri->clause))
  			*remote_conds = lappend(*remote_conds, ri);
--- 210,216 ----
  
  	foreach(lc, input_conds)
  	{
! 		RestrictInfo *ri = lfirst_node(RestrictInfo, lc);
  
  		if (is_foreign_expr(root, baserel, ri->clause))
  			*remote_conds = lappend(*remote_conds, ri);
*************** build_tlist_to_deparse(RelOptInfo *forei
*** 869,874 ****
--- 869,875 ----
  {
  	List	   *tlist = NIL;
  	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
+ 	ListCell   *lc;
  
  	/*
  	 * For an upper relation, we have already built the target list while
*************** build_tlist_to_deparse(RelOptInfo *forei
*** 884,892 ****
  	tlist = add_to_flat_tlist(tlist,
  					   pull_var_clause((Node *) foreignrel->reltarget->exprs,
  									   PVC_RECURSE_PLACEHOLDERS));
! 	tlist = add_to_flat_tlist(tlist,
! 							  pull_var_clause((Node *) fpinfo->local_conds,
! 											  PVC_RECURSE_PLACEHOLDERS));
  
  	return tlist;
  }
--- 885,898 ----
  	tlist = add_to_flat_tlist(tlist,
  					   pull_var_clause((Node *) foreignrel->reltarget->exprs,
  									   PVC_RECURSE_PLACEHOLDERS));
! 	foreach(lc, fpinfo->local_conds)
! 	{
! 		RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
! 
! 		tlist = add_to_flat_tlist(tlist,
! 								  pull_var_clause((Node *) rinfo->clause,
! 												  PVC_RECURSE_PLACEHOLDERS));
! 	}
  
  	return tlist;
  }
*************** deparseSelectSql(List *tlist, bool is_su
*** 1049,1054 ****
--- 1055,1061 ----
   * "buf".
   *
   * quals is the list of clauses to be included in the WHERE clause.
+  * (These may or may not include RestrictInfo decoration.)
   */
  static void
  deparseFromExpr(List *quals, deparse_expr_cxt *context)
*************** deparseLockingClause(deparse_expr_cxt *c
*** 1262,1267 ****
--- 1269,1277 ----
   *
   * The conditions in the list are assumed to be ANDed. This function is used to
   * deparse WHERE clauses, JOIN .. ON clauses and HAVING clauses.
+  *
+  * Depending on the caller, the list elements might be either RestrictInfos
+  * or bare clauses.
   */
  static void
  appendConditions(List *exprs, deparse_expr_cxt *context)
*************** appendConditions(List *exprs, deparse_ex
*** 1278,1293 ****
  	{
  		Expr	   *expr = (Expr *) lfirst(lc);
  
! 		/*
! 		 * Extract clause from RestrictInfo, if required. See comments in
! 		 * declaration of PgFdwRelationInfo for details.
! 		 */
  		if (IsA(expr, RestrictInfo))
! 		{
! 			RestrictInfo *ri = (RestrictInfo *) expr;
! 
! 			expr = ri->clause;
! 		}
  
  		/* Connect expressions with "AND" and parenthesize each condition. */
  		if (!is_first)
--- 1288,1296 ----
  	{
  		Expr	   *expr = (Expr *) lfirst(lc);
  
! 		/* Extract clause from RestrictInfo, if required */
  		if (IsA(expr, RestrictInfo))
! 			expr = ((RestrictInfo *) expr)->clause;
  
  		/* Connect expressions with "AND" and parenthesize each condition. */
  		if (!is_first)
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 6670df5..28a945f 100644
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
*************** enum FdwScanPrivateIndex
*** 64,70 ****
  {
  	/* SQL statement to execute remotely (as a String node) */
  	FdwScanPrivateSelectSql,
! 	/* List of restriction clauses that can be executed remotely */
  	FdwScanPrivateRemoteConds,
  	/* Integer list of attribute numbers retrieved by the SELECT */
  	FdwScanPrivateRetrievedAttrs,
--- 64,71 ----
  {
  	/* SQL statement to execute remotely (as a String node) */
  	FdwScanPrivateSelectSql,
! 	/* List of qual clauses that can be executed remotely */
! 	/* (DO NOT try to use these at runtime; see postgresGetForeignPlan) */
  	FdwScanPrivateRemoteConds,
  	/* Integer list of attribute numbers retrieved by the SELECT */
  	FdwScanPrivateRetrievedAttrs,
*************** postgresGetForeignRelSize(PlannerInfo *r
*** 576,582 ****
  				   &fpinfo->attrs_used);
  	foreach(lc, fpinfo->local_conds)
  	{
! 		RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
  
  		pull_varattnos((Node *) rinfo->clause, baserel->relid,
  					   &fpinfo->attrs_used);
--- 577,583 ----
  				   &fpinfo->attrs_used);
  	foreach(lc, fpinfo->local_conds)
  	{
! 		RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
  
  		pull_varattnos((Node *) rinfo->clause, baserel->relid,
  					   &fpinfo->attrs_used);
*************** postgresGetForeignPlan(PlannerInfo *root
*** 1119,1132 ****
  	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
  	Index		scan_relid;
  	List	   *fdw_private;
- 	List	   *remote_conds = NIL;
  	List	   *remote_exprs = NIL;
  	List	   *local_exprs = NIL;
  	List	   *params_list = NIL;
  	List	   *retrieved_attrs;
  	StringInfoData sql;
  	ListCell   *lc;
- 	List	   *fdw_scan_tlist = NIL;
  
  	/*
  	 * For base relations, set scan_relid as the relid of the relation. For
--- 1120,1133 ----
  	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
  	Index		scan_relid;
  	List	   *fdw_private;
  	List	   *remote_exprs = NIL;
  	List	   *local_exprs = NIL;
  	List	   *params_list = NIL;
+ 	List	   *fdw_scan_tlist = NIL;
+ 	List	   *fdw_recheck_quals = NIL;
  	List	   *retrieved_attrs;
  	StringInfoData sql;
  	ListCell   *lc;
  
  	/*
  	 * For base relations, set scan_relid as the relid of the relation. For
*************** postgresGetForeignPlan(PlannerInfo *root
*** 1163,1171 ****
  	 *
  	 * This code must match "extract_actual_clauses(scan_clauses, false)"
  	 * except for the additional decision about remote versus local execution.
- 	 * Note however that we don't strip the RestrictInfo nodes from the
- 	 * remote_conds list, since appendWhereClause expects a list of
- 	 * RestrictInfos.
  	 */
  	foreach(lc, scan_clauses)
  	{
--- 1164,1169 ----
*************** postgresGetForeignPlan(PlannerInfo *root
*** 1176,1201 ****
  			continue;
  
  		if (list_member_ptr(fpinfo->remote_conds, rinfo))
- 		{
- 			remote_conds = lappend(remote_conds, rinfo);
  			remote_exprs = lappend(remote_exprs, rinfo->clause);
- 		}
  		else if (list_member_ptr(fpinfo->local_conds, rinfo))
  			local_exprs = lappend(local_exprs, rinfo->clause);
  		else if (is_foreign_expr(root, foreignrel, rinfo->clause))
- 		{
- 			remote_conds = lappend(remote_conds, rinfo);
  			remote_exprs = lappend(remote_exprs, rinfo->clause);
- 		}
  		else
  			local_exprs = lappend(local_exprs, rinfo->clause);
  	}
  
  	if (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
  	{
! 		/* For a join relation, get the conditions from fdw_private structure */
! 		remote_conds = fpinfo->remote_conds;
! 		local_exprs = fpinfo->local_conds;
  
  		/* Build the list of columns to be fetched from the foreign server. */
  		fdw_scan_tlist = build_tlist_to_deparse(foreignrel);
--- 1174,1198 ----
  			continue;
  
  		if (list_member_ptr(fpinfo->remote_conds, rinfo))
  			remote_exprs = lappend(remote_exprs, rinfo->clause);
  		else if (list_member_ptr(fpinfo->local_conds, rinfo))
  			local_exprs = lappend(local_exprs, rinfo->clause);
  		else if (is_foreign_expr(root, foreignrel, rinfo->clause))
  			remote_exprs = lappend(remote_exprs, rinfo->clause);
  		else
  			local_exprs = lappend(local_exprs, rinfo->clause);
  	}
  
  	if (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
  	{
! 		/*
! 		 * For a join relation, get the conditions from fdw_private structure.
! 		 * Note that we leave fdw_recheck_quals empty in this case, since
! 		 * there is no need for EPQ recheck at a join (and Vars or Aggrefs in
! 		 * the qual might not be available locally anyway).
! 		 */
! 		remote_exprs = extract_actual_clauses(fpinfo->remote_conds, false);
! 		local_exprs = extract_actual_clauses(fpinfo->local_conds, false);
  
  		/* Build the list of columns to be fetched from the foreign server. */
  		fdw_scan_tlist = build_tlist_to_deparse(foreignrel);
*************** postgresGetForeignPlan(PlannerInfo *root
*** 1238,1243 ****
--- 1235,1248 ----
  			}
  		}
  	}
+ 	else
+ 	{
+ 		/*
+ 		 * For a base-relation scan, we have to support EPQ recheck, which
+ 		 * should recheck all remote quals
+ 		 */
+ 		fdw_recheck_quals = remote_exprs;
+ 	}
  
  	/*
  	 * Build the query string to be sent for execution, and identify
*************** postgresGetForeignPlan(PlannerInfo *root
*** 1245,1251 ****
  	 */
  	initStringInfo(&sql);
  	deparseSelectStmtForRel(&sql, root, foreignrel, fdw_scan_tlist,
! 							remote_conds, best_path->path.pathkeys,
  							false, &retrieved_attrs, &params_list);
  
  	/*
--- 1250,1256 ----
  	 */
  	initStringInfo(&sql);
  	deparseSelectStmtForRel(&sql, root, foreignrel, fdw_scan_tlist,
! 							remote_exprs, best_path->path.pathkeys,
  							false, &retrieved_attrs, &params_list);
  
  	/*
*************** postgresGetForeignPlan(PlannerInfo *root
*** 1253,1259 ****
  	 * 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 (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
--- 1258,1264 ----
  	 * Items in the list must match order in enum FdwScanPrivateIndex.
  	 */
  	fdw_private = list_make4(makeString(sql.data),
! 							 remote_exprs,
  							 retrieved_attrs,
  							 makeInteger(fpinfo->fetch_size));
  	if (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
*************** postgresGetForeignPlan(PlannerInfo *root
*** 1266,1271 ****
--- 1271,1283 ----
  	 * Note that the remote parameter expressions are stored in the fdw_exprs
  	 * field of the finished plan node; we can't keep them in private state
  	 * because then they wouldn't be subject to later planner processing.
+ 	 *
+ 	 * We have some foreign qual conditions hidden away within fdw_private's
+ 	 * FdwScanPrivateRemoteConds item, which would be unsafe per the above
+ 	 * consideration. But those will only be used by postgresPlanDirectModify,
+ 	 * which may extract them to use in a rewritten plan.  We assume that
+ 	 * nothing will be done between here and there that would need to modify
+ 	 * those expressions.
  	 */
  	return make_foreignscan(tlist,
  							local_exprs,
*************** postgresGetForeignPlan(PlannerInfo *root
*** 1273,1279 ****
  							params_list,
  							fdw_private,
  							fdw_scan_tlist,
! 							remote_exprs,
  							outer_plan);
  }
  
--- 1285,1291 ----
  							params_list,
  							fdw_private,
  							fdw_scan_tlist,
! 							fdw_recheck_quals,
  							outer_plan);
  }
  
*************** postgresPlanDirectModify(PlannerInfo *ro
*** 2199,2205 ****
  	rel = heap_open(rte->relid, NoLock);
  
  	/*
! 	 * Extract the baserestrictinfo clauses that can be evaluated remotely.
  	 */
  	remote_conds = (List *) list_nth(fscan->fdw_private,
  									 FdwScanPrivateRemoteConds);
--- 2211,2219 ----
  	rel = heap_open(rte->relid, NoLock);
  
  	/*
! 	 * Extract the qual clauses that can be evaluated remotely.  (These are
! 	 * bare clauses not RestrictInfos, but deparse.c's appendConditions()
! 	 * doesn't care.)
  	 */
  	remote_conds = (List *) list_nth(fscan->fdw_private,
  									 FdwScanPrivateRemoteConds);
*************** foreign_join_ok(PlannerInfo *root, RelOp
*** 4094,4102 ****
  	if (fpinfo_o->local_conds || fpinfo_i->local_conds)
  		return false;
  
! 	/* Separate restrict list into join quals and quals on join relation */
  	if (IS_OUTER_JOIN(jointype))
! 		extract_actual_join_clauses(extra->restrictlist, &joinclauses, &otherclauses);
  	else
  	{
  		/*
--- 4108,4128 ----
  	if (fpinfo_o->local_conds || fpinfo_i->local_conds)
  		return false;
  
! 	/* Separate restrict list into join quals and pushed-down (other) quals */
  	if (IS_OUTER_JOIN(jointype))
! 	{
! 		/* Grovel through the clauses to separate into two lists */
! 		joinclauses = otherclauses = NIL;
! 		foreach(lc, extra->restrictlist)
! 		{
! 			RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
! 
! 			if (rinfo->is_pushed_down)
! 				otherclauses = lappend(otherclauses, rinfo);
! 			else
! 				joinclauses = lappend(joinclauses, rinfo);
! 		}
! 	}
  	else
  	{
  		/*
*************** foreign_join_ok(PlannerInfo *root, RelOp
*** 4106,4121 ****
  		 * a join down to the foreign server even if some of its join quals
  		 * are not safe to pushdown.
  		 */
! 		otherclauses = extract_actual_clauses(extra->restrictlist, false);
  		joinclauses = NIL;
  	}
  
  	/* Join quals must be safe to push down. */
  	foreach(lc, joinclauses)
  	{
! 		Expr	   *expr = (Expr *) lfirst(lc);
  
! 		if (!is_foreign_expr(root, joinrel, expr))
  			return false;
  	}
  
--- 4132,4147 ----
  		 * a join down to the foreign server even if some of its join quals
  		 * are not safe to pushdown.
  		 */
! 		otherclauses = extra->restrictlist;
  		joinclauses = NIL;
  	}
  
  	/* Join quals must be safe to push down. */
  	foreach(lc, joinclauses)
  	{
! 		RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
  
! 		if (!is_foreign_expr(root, joinrel, rinfo->clause))
  			return false;
  	}
  
*************** foreign_join_ok(PlannerInfo *root, RelOp
*** 4152,4163 ****
  	 */
  	foreach(lc, otherclauses)
  	{
! 		Expr	   *expr = (Expr *) lfirst(lc);
  
! 		if (!is_foreign_expr(root, joinrel, expr))
! 			fpinfo->local_conds = lappend(fpinfo->local_conds, expr);
  		else
! 			fpinfo->remote_conds = lappend(fpinfo->remote_conds, expr);
  	}
  
  	fpinfo->outerrel = outerrel;
--- 4178,4189 ----
  	 */
  	foreach(lc, otherclauses)
  	{
! 		RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
  
! 		if (is_foreign_expr(root, joinrel, rinfo->clause))
! 			fpinfo->remote_conds = lappend(fpinfo->remote_conds, rinfo);
  		else
! 			fpinfo->local_conds = lappend(fpinfo->local_conds, rinfo);
  	}
  
  	fpinfo->outerrel = outerrel;
*************** foreign_grouping_ok(PlannerInfo *root, R
*** 4631,4641 ****
  		foreach(lc, (List *) query->havingQual)
  		{
  			Expr	   *expr = (Expr *) lfirst(lc);
  
! 			if (!is_foreign_expr(root, grouped_rel, expr))
! 				fpinfo->local_conds = lappend(fpinfo->local_conds, expr);
  			else
! 				fpinfo->remote_conds = lappend(fpinfo->remote_conds, expr);
  		}
  	}
  
--- 4657,4681 ----
  		foreach(lc, (List *) query->havingQual)
  		{
  			Expr	   *expr = (Expr *) lfirst(lc);
+ 			RestrictInfo *rinfo;
  
! 			/*
! 			 * Currently, the core code doesn't wrap havingQuals in
! 			 * RestrictInfos, so we must make our own.
! 			 */
! 			Assert(!IsA(expr, RestrictInfo));
! 			rinfo = make_restrictinfo(expr,
! 									  true,
! 									  false,
! 									  false,
! 									  root->qual_security_level,
! 									  grouped_rel->relids,
! 									  NULL,
! 									  NULL);
! 			if (is_foreign_expr(root, grouped_rel, expr))
! 				fpinfo->remote_conds = lappend(fpinfo->remote_conds, rinfo);
  			else
! 				fpinfo->local_conds = lappend(fpinfo->local_conds, rinfo);
  		}
  	}
  
*************** foreign_grouping_ok(PlannerInfo *root, R
*** 4645,4653 ****
  	 */
  	if (fpinfo->local_conds)
  	{
  		ListCell   *lc;
! 		List	   *aggvars = pull_var_clause((Node *) fpinfo->local_conds,
! 											  PVC_INCLUDE_AGGREGATES);
  
  		foreach(lc, aggvars)
  		{
--- 4685,4701 ----
  	 */
  	if (fpinfo->local_conds)
  	{
+ 		List	   *aggvars = NIL;
  		ListCell   *lc;
! 
! 		foreach(lc, fpinfo->local_conds)
! 		{
! 			RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
! 
! 			aggvars = list_concat(aggvars,
! 								  pull_var_clause((Node *) rinfo->clause,
! 												  PVC_INCLUDE_AGGREGATES));
! 		}
  
  		foreach(lc, aggvars)
  		{
*************** foreign_grouping_ok(PlannerInfo *root, R
*** 4664,4670 ****
  				if (!is_foreign_expr(root, grouped_rel, expr))
  					return false;
  
! 				tlist = add_to_flat_tlist(tlist, aggvars);
  			}
  		}
  	}
--- 4712,4718 ----
  				if (!is_foreign_expr(root, grouped_rel, expr))
  					return false;
  
! 				tlist = add_to_flat_tlist(tlist, list_make1(expr));
  			}
  		}
  	}
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 57dbb79..df75518 100644
*** a/contrib/postgres_fdw/postgres_fdw.h
--- b/contrib/postgres_fdw/postgres_fdw.h
*************** typedef struct PgFdwRelationInfo
*** 34,49 ****
  
  	/*
  	 * Restriction clauses, divided into safe and unsafe to pushdown subsets.
! 	 *
! 	 * For a base foreign relation this is a list of clauses along-with
! 	 * RestrictInfo wrapper. Keeping RestrictInfo wrapper helps while dividing
! 	 * scan_clauses in postgresGetForeignPlan into safe and unsafe subsets.
! 	 * Also it helps in estimating costs since RestrictInfo caches the
! 	 * selectivity and qual cost for the clause in it.
! 	 *
! 	 * For a join relation, however, they are part of otherclause list
! 	 * obtained from extract_actual_join_clauses, which strips RestrictInfo
! 	 * construct. So, for a join relation they are list of bare clauses.
  	 */
  	List	   *remote_conds;
  	List	   *local_conds;
--- 34,41 ----
  
  	/*
  	 * Restriction clauses, divided into safe and unsafe to pushdown subsets.
! 	 * All entries in these lists should have RestrictInfo wrappers; that
! 	 * improves efficiency of selectivity and cost estimation.
  	 */
  	List	   *remote_conds;
  	List	   *local_conds;
*************** typedef struct PgFdwRelationInfo
*** 91,97 ****
  	RelOptInfo *outerrel;
  	RelOptInfo *innerrel;
  	JoinType	jointype;
! 	List	   *joinclauses;
  
  	/* Grouping information */
  	List	   *grouped_tlist;
--- 83,89 ----
  	RelOptInfo *outerrel;
  	RelOptInfo *innerrel;
  	JoinType	jointype;
! 	List	   *joinclauses;	/* List of RestrictInfo */
  
  	/* Grouping information */
  	List	   *grouped_tlist;