Subject: Re: postgres_fdw: support parameterized foreign joins

From: Arthur Zakirov <[email-masked]>
Date: Tue, 4 Apr 2017 18:55:14 +0300

On 23.03.2017 15:45, Etsuro Fujita wrote:
>
> Done.  Also, I added regression tests and revised code and comments a
> bit.  (As for create_foreignscan_path(), I haven't done anything about
> that yet.)  Please find attached a new version created on top of [1].

Thank you!

I didn't notice that it is necessary to apply the patch 
"epqpath-for-foreignjoin".

I have a few comments.

>    * innersortkeys are the sort pathkeys for the inner side of the mergejoin
> +  * req_outer_list is a list of sensible parameterizations for the join rel
>    */

I think it would be better if the comment explained what type is stored 
in req_outer_list. So the following comment would be good:

"req_outer_list is a list of Relids of sensible parameterizations for 
the join rel"

>
> ! 			Assert(foreignrel->reloptkind == RELOPT_JOINREL);
> !

Here the new macro IS_JOIN_REL() can be used.

> ! 			/* Get the remote and local conditions */
> ! 			remote_conds = list_concat(list_copy(remote_param_join_conds),
> ! 									   fpinfo->remote_conds);
> ! 			local_param_join_exprs =
> ! 				get_actual_clauses(local_param_join_conds);
> ! 			local_exprs = list_concat(list_copy(local_param_join_exprs),
> ! 									  fpinfo->local_conds);

Is this code correct? 'remote_conds' and 'local_exprs' are initialized 
above when 'scan_clauses' is separated. Maybe better to use 
'remote_conds' and 'local_exprs' instead of 'fpinfo->remote_conds' and 
'fpinfo->local_conds' respectively?

And the last. The patch needs rebasing because new macroses 
IS_JOIN_REL() and IS_UPPER_REL() were added. And the patch is applied 
with errors.

-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

-- 
Sent via pgsql-hackers mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

---

From: Etsuro Fujita <[email-masked]>
Date: Wed, 5 Apr 2017 18:20:42 +0900

Hi Arthur,

> I have a few comments.

Thank you for the review!

> "req_outer_list is a list of Relids of sensible parameterizations for

Done.

> Here the new macro IS_JOIN_REL() can be used.

Done.

>> !             remote_conds =
>> list_concat(list_copy(remote_param_join_conds),
>> !                                        fpinfo->remote_conds);
>> !             local_exprs =
>> list_concat(list_copy(local_param_join_exprs),
>> !                                       fpinfo->local_conds);
>

Let me explain.  As described in the comment in postgresGetForeignPlan:

     if (IS_SIMPLE_REL(foreignrel))
         scan_relid = foreignrel->relid;
     else
     {
         scan_relid = 0;

         /*
          * create_scan_plan() and create_foreignscan_plan() pass
          * rel->baserestrictinfo + parameterization clauses through
          * scan_clauses, but for a join or upper relation, there should 
be no
          * scan_clauses.
          */
         Assert(!scan_clauses);
     }

scan_clauses=NIL for a join relation.  So, for a join relation we use 
fpinfo->remote_conds and fpinfo->local_conds, instead.  (Note that those 
conditions are created at path creation time, ie, 
postgresGetForeignJoinPaths.  See foreign_join_ok.)

> IS_JOIN_REL() and IS_UPPER_REL() were added. And the patch is applied

Rebased.

Attached is an updated version created on top of the latest patch 
"epqpath-for-foreignjoin" [1].

Other changes:
* Added a bit more regression tests with FOR UPDATE clause to see if 
CreateLocalJoinPath works well for parameterized foreign join paths.
* Added/revised comments a bit.

Best regards,
Etsuro Fujita

[1] 
https://www.postgresql.org/message-id/424933d7-d6bb-4b8f-4e44-1fea212af083%40lab.ntt.co.jp
*** a/contrib/postgres_fdw/deparse.c
--- b/contrib/postgres_fdw/deparse.c
***************
*** 865,871 **** deparse_type_name(Oid type_oid, int32 typemod)
   * foreign server.
   */
  List *
! build_tlist_to_deparse(RelOptInfo *foreignrel)
  {
  	List	   *tlist = NIL;
  	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
--- 865,871 ----
   * foreign server.
   */
  List *
! build_tlist_to_deparse(RelOptInfo *foreignrel, List *local_param_conds)
  {
  	List	   *tlist = NIL;
  	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
***************
*** 887,892 **** build_tlist_to_deparse(RelOptInfo *foreignrel)
--- 887,896 ----
  	tlist = add_to_flat_tlist(tlist,
  							  pull_var_clause((Node *) fpinfo->local_conds,
  											  PVC_RECURSE_PLACEHOLDERS));
+ 	if (local_param_conds)
+ 		tlist = add_to_flat_tlist(tlist,
+ 								  pull_var_clause((Node *) local_param_conds,
+ 												  PVC_RECURSE_PLACEHOLDERS));
  
  	return tlist;
  }
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
***************
*** 2283,2288 **** SELECT ft5, ft5.c1, ft5.c2, ft5.c3, ft4.c1, ft4.c2 FROM ft5 left join ft4 on ft5
--- 2283,2343 ----
   (30,31,AAA030) | 30 | 31 | AAA030 | 30 | 31
  (4 rows)
  
+ -- parameterized remote path for foreign join
+ EXPLAIN (VERBOSE, COSTS OFF)
+   SELECT * FROM "S 1"."T 1" a LEFT JOIN (ft1 b INNER JOIN ft2 c ON (b.c1 = c.c1)) ON (c.c1 = a.c2) WHERE a."C 1" = 47;
+                                                                                                                           QUERY PLAN                                                                                                                           
+ ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+  Nested Loop Left Join
+    Output: a."C 1", a.c2, a.c3, a.c4, a.c5, a.c6, a.c7, a.c8, b.c1, b.c2, b.c3, b.c4, b.c5, b.c6, b.c7, b.c8, c.c1, c.c2, c.c3, c.c4, c.c5, c.c6, c.c7, c.c8
+    ->  Index Scan using t1_pkey on "S 1"."T 1" a
+          Output: a."C 1", a.c2, a.c3, a.c4, a.c5, a.c6, a.c7, a.c8
+          Index Cond: (a."C 1" = 47)
+    ->  Foreign Scan
+          Output: b.c1, b.c2, b.c3, b.c4, b.c5, b.c6, b.c7, b.c8, c.c1, c.c2, c.c3, c.c4, c.c5, c.c6, c.c7, c.c8
+          Relations: (public.ft1 b) INNER JOIN (public.ft2 c)
+          Remote SQL: SELECT r2."C 1", r2.c2, r2.c3, r2.c4, r2.c5, r2.c6, r2.c7, r2.c8, r3."C 1", r3.c2, r3.c3, r3.c4, r3.c5, r3.c6, r3.c7, r3.c8 FROM ("S 1"."T 1" r2 INNER JOIN "S 1"."T 1" r3 ON (((r2."C 1" = r3."C 1")))) WHERE ((r3."C 1" = $1::integer))
+ (9 rows)
+ 
+ SELECT * FROM "S 1"."T 1" a LEFT JOIN (ft1 b INNER JOIN ft2 c ON (b.c1 = c.c1)) ON (c.c1 = a.c2) WHERE a."C 1" = 47;
+  C 1 | c2 |  c3   |              c4              |            c5            | c6 |     c7     | c8  | c1 | c2 |  c3   |              c4              |            c5            | c6 |     c7     | c8  | c1 | c2 |  c3   |              c4              |            c5            | c6 |     c7     | c8  
+ -----+----+-------+------------------------------+--------------------------+----+------------+-----+----+----+-------+------------------------------+--------------------------+----+------------+-----+----+----+-------+------------------------------+--------------------------+----+------------+-----
+   47 |  7 | 00047 | Tue Feb 17 00:00:00 1970 PST | Tue Feb 17 00:00:00 1970 | 7  | 7          | foo |  7 |  7 | 00007 | Thu Jan 08 00:00:00 1970 PST | Thu Jan 08 00:00:00 1970 | 7  | 7          | foo |  7 |  7 | 00007 | Thu Jan 08 00:00:00 1970 PST | Thu Jan 08 00:00:00 1970 | 7  | 7          | foo
+ (1 row)
+ 
+ -- with FOR UPDATE/SHARE
+ EXPLAIN (VERBOSE, COSTS OFF)
+   SELECT * FROM "S 1"."T 1" a LEFT JOIN (ft1 b INNER JOIN ft2 c ON (b.c1 = c.c1)) ON (c.c1 = a.c2) WHERE a."C 1" = 57 FOR UPDATE OF a;
+                                                                                                                                                                                                                                          QUERY PLAN                                                                                                                                                                                                                                          
+ ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+  LockRows
+    Output: a."C 1", a.c2, a.c3, a.c4, a.c5, a.c6, a.c7, a.c8, b.c1, b.c2, b.c3, b.c4, b.c5, b.c6, b.c7, b.c8, c.c1, c.c2, c.c3, c.c4, c.c5, c.c6, c.c7, c.c8, a.ctid, b.*, c.*
+    ->  Nested Loop Left Join
+          Output: a."C 1", a.c2, a.c3, a.c4, a.c5, a.c6, a.c7, a.c8, b.c1, b.c2, b.c3, b.c4, b.c5, b.c6, b.c7, b.c8, c.c1, c.c2, c.c3, c.c4, c.c5, c.c6, c.c7, c.c8, a.ctid, b.*, c.*
+          ->  Index Scan using t1_pkey on "S 1"."T 1" a
+                Output: a."C 1", a.c2, a.c3, a.c4, a.c5, a.c6, a.c7, a.c8, a.ctid
+                Index Cond: (a."C 1" = 57)
+          ->  Foreign Scan
+                Output: b.c1, b.c2, b.c3, b.c4, b.c5, b.c6, b.c7, b.c8, b.*, c.c1, c.c2, c.c3, c.c4, c.c5, c.c6, c.c7, c.c8, c.*
+                Relations: (public.ft1 b) INNER JOIN (public.ft2 c)
+                Remote SQL: SELECT r2."C 1", r2.c2, r2.c3, r2.c4, r2.c5, r2.c6, r2.c7, r2.c8, CASE WHEN (r2.*)::text IS NOT NULL THEN ROW(r2."C 1", r2.c2, r2.c3, r2.c4, r2.c5, r2.c6, r2.c7, r2.c8) END, r3."C 1", r3.c2, r3.c3, r3.c4, r3.c5, r3.c6, r3.c7, r3.c8, CASE WHEN (r3.*)::text IS NOT NULL THEN ROW(r3."C 1", r3.c2, r3.c3, r3.c4, r3.c5, r3.c6, r3.c7, r3.c8) END FROM ("S 1"."T 1" r2 INNER JOIN "S 1"."T 1" r3 ON (((r2."C 1" = r3."C 1")))) WHERE ((r3."C 1" = $1::integer))
+                ->  Nested Loop
+                      Output: b.c1, b.c2, b.c3, b.c4, b.c5, b.c6, b.c7, b.c8, b.*, c.c1, c.c2, c.c3, c.c4, c.c5, c.c6, c.c7, c.c8, c.*
+                      Join Filter: ((c.c1 = a.c2) AND (b.c1 = c.c1))
+                      ->  Foreign Scan on public.ft1 b
+                            Output: b.c1, b.c2, b.c3, b.c4, b.c5, b.c6, b.c7, b.c8, b.*
+                            Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1"
+                      ->  Foreign Scan on public.ft2 c
+                            Output: c.c1, c.c2, c.c3, c.c4, c.c5, c.c6, c.c7, c.c8, c.*
+                            Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1"
+ (20 rows)
+ 
+ SELECT * FROM "S 1"."T 1" a LEFT JOIN (ft1 b INNER JOIN ft2 c ON (b.c1 = c.c1)) ON (c.c1 = a.c2) WHERE a."C 1" = 57 FOR UPDATE OF a;
+  C 1 | c2 |  c3   |              c4              |            c5            | c6 |     c7     | c8  | c1 | c2 |  c3   |              c4              |            c5            | c6 |     c7     | c8  | c1 | c2 |  c3   |              c4              |            c5            | c6 |     c7     | c8  
+ -----+----+-------+------------------------------+--------------------------+----+------------+-----+----+----+-------+------------------------------+--------------------------+----+------------+-----+----+----+-------+------------------------------+--------------------------+----+------------+-----
+   57 |  7 | 00057 | Fri Feb 27 00:00:00 1970 PST | Fri Feb 27 00:00:00 1970 | 7  | 7          | foo |  7 |  7 | 00007 | Thu Jan 08 00:00:00 1970 PST | Thu Jan 08 00:00:00 1970 | 7  | 7          | foo |  7 |  7 | 00007 | Thu Jan 08 00:00:00 1970 PST | Thu Jan 08 00:00:00 1970 | 7  | 7          | foo
+ (1 row)
+ 
  -- check join pushdown in situations where multiple userids are involved
  CREATE ROLE regress_view_owner;
  CREATE USER MAPPING FOR regress_view_owner SERVER loopback;
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
***************
*** 413,421 **** static List *get_useful_pathkeys_for_relation(PlannerInfo *root,
--- 413,427 ----
  static List *get_useful_ecs_for_relation(PlannerInfo *root, RelOptInfo *rel);
  static void add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel,
  								Path *epq_path);
+ static void add_parameterized_paths_for_rel(PlannerInfo *root,
+ 											RelOptInfo *joinrel,
+ 											List *req_outer_list);
  static void add_foreign_grouping_paths(PlannerInfo *root,
  						   RelOptInfo *input_rel,
  						   RelOptInfo *grouped_rel);
+ static List *build_joinrel_param_join_conds(PlannerInfo *root,
+ 							   RelOptInfo *joinrel,
+ 							   Relids required_outer);

  /*
***************
*** 1141,1149 **** postgresGetForeignPlan(PlannerInfo *root,
  		/*
  		 * create_scan_plan() and create_foreignscan_plan() pass
  		 * rel->baserestrictinfo + parameterization clauses through
! 		 * scan_clauses. For a join rel->baserestrictinfo is NIL and we are
! 		 * not considering parameterization right now, so there should be no
! 		 * scan_clauses for a joinrel and upper rel either.
  		 */
  		Assert(!scan_clauses);
  	}
--- 1147,1154 ----
  		/*
  		 * create_scan_plan() and create_foreignscan_plan() pass
  		 * rel->baserestrictinfo + parameterization clauses through
! 		 * scan_clauses, but for a join or upper relation, there should be no
! 		 * scan_clauses.
  		 */
  		Assert(!scan_clauses);
  	}
***************
*** 1193,1204 **** postgresGetForeignPlan(PlannerInfo *root,
  
  	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);
  
  		/*
  		 * Ensure that the outer plan produces a tuple whose descriptor
--- 1198,1256 ----
  
  	if (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
  	{
! 		/*
! 		 * Get the remote/local conditions from fdw_private structure.  Also,
! 		 * build the list of columns to be fetched from the foreign server.
! 		 */
! 		if (best_path->path.param_info)
! 		{
! 			List	   *param_join_conds;
! 			List	   *remote_param_join_conds;
! 			List	   *local_param_join_conds;
! 			List	   *local_param_join_exprs;
  
! 			Assert(IS_JOIN_REL(foreignrel));
! 
! 			/*
! 			 * For a parameterized foreign join, we also need to enforce all
! 			 * join clauses available from the outer relation(s).
! 			 */
! 
! 			/* Identify all join clauses that are movable to this rel. */
! 			param_join_conds =
! 				build_joinrel_param_join_conds(root, foreignrel,
! 										PATH_REQ_OUTER((Path *) best_path));
! 
! 			/*
! 			 * param_join_conds might contain both clauses that are safe to
! 			 * send across, and clauses that aren't.
! 			 */
! 			classifyConditions(root, foreignrel,
! 							   param_join_conds,
! 							   &remote_param_join_conds,
! 							   &local_param_join_conds);
! 
! 			/* Get the remote and local conditions */
! 			remote_conds = list_concat(list_copy(remote_param_join_conds),
! 									   fpinfo->remote_conds);
! 			local_param_join_exprs =
! 				get_actual_clauses(local_param_join_conds);
! 			local_exprs = list_concat(list_copy(local_param_join_exprs),
! 									  fpinfo->local_conds);
! 
! 			/* Build the list of the columns */
! 			fdw_scan_tlist = build_tlist_to_deparse(foreignrel,
! 													local_param_join_conds);
! 		}
! 		else
! 		{
! 			/* Get the remote and local conditions */
! 			remote_conds = fpinfo->remote_conds;
! 			local_exprs = fpinfo->local_conds;
! 
! 			/* Build the list of the columns */
! 			fdw_scan_tlist = build_tlist_to_deparse(foreignrel, NIL);
! 		}
  
  		/*
  		 * Ensure that the outer plan produces a tuple whose descriptor
***************
*** 2536,2549 **** estimate_path_cost_size(PlannerInfo *root,
  
  		/* Build the list of columns to be fetched from the foreign server. */
  		if (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
! 			fdw_scan_tlist = build_tlist_to_deparse(foreignrel);
  		else
  			fdw_scan_tlist = NIL;
  
  		/*
  		 * The complete list of remote conditions includes everything from
! 		 * baserestrictinfo plus any extra join_conds relevant to this
! 		 * particular path.
  		 */
  		remote_conds = list_concat(list_copy(remote_param_join_conds),
  								   fpinfo->remote_conds);
--- 2588,2603 ----
  
  		/* Build the list of columns to be fetched from the foreign server. */
  		if (IS_JOIN_REL(foreignrel) || IS_UPPER_REL(foreignrel))
! 			fdw_scan_tlist = build_tlist_to_deparse(foreignrel,
! 													local_param_join_conds);
  		else
  			fdw_scan_tlist = NIL;
  
  		/*
  		 * The complete list of remote conditions includes everything from
! 		 * baserestrictinfo (or top-level WHERE clauses in case of a join
! 		 * relation) plus any extra join_conds relevant to this particular
! 		 * path.
  		 */
  		remote_conds = list_concat(list_copy(remote_param_join_conds),
  								   fpinfo->remote_conds);
***************
*** 4163,4168 **** foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
--- 4217,4223 ----
  	fpinfo->outerrel = outerrel;
  	fpinfo->innerrel = innerrel;
  	fpinfo->jointype = jointype;
+ 	fpinfo->extra = extra;
  
  	/*
  	 * By default, both the input relations are not required to be deparsed
***************
*** 4370,4375 **** add_paths_with_pathkeys_for_rel(PlannerInfo *root, RelOptInfo *rel,
--- 4425,4643 ----
  	}
  }
  
+ static void
+ add_parameterized_paths_for_rel(PlannerInfo *root, RelOptInfo *joinrel,
+ 								List *req_outer_list)
+ {
+ 	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) joinrel->fdw_private;
+ 	RelOptInfo *outerrel = fpinfo->outerrel;
+ 	RelOptInfo *innerrel = fpinfo->innerrel;
+ 	ListCell   *lc;
+ 
+ 	/*
+ 	 * If we're not using remote estimates, we can't do anything.
+ 	 */
+ 	if (!fpinfo->use_remote_estimate)
+ 		return;
+ 
+ 	/*
+ 	 * Build a path for each outer relation in the given req_outer_list.
+ 	 */
+ 	foreach(lc, req_outer_list)
+ 	{
+ 		Relids		required_outer = (Relids) lfirst(lc);
+ 		ForeignPath *path;
+ 		Path	   *epq_path;
+ 		List	   *param_join_conds;
+ 		double		rows;
+ 		int			width;
+ 		Cost		startup_cost;
+ 		Cost		total_cost;
+ 		ParamPathInfo *param_info;
+ 		bool		found;
+ 		ListCell   *lc2;
+ 
+ 		Assert(!bms_is_empty(required_outer));
+ 
+ 		/* If we have already considered the required_outer, skip it */
+ 		found = false;
+ 		foreach(lc2, fpinfo->req_outer_list)
+ 		{
+ 			Relids		considered_outer = (Relids) lfirst(lc2);
+ 
+ 			if (bms_equal(considered_outer, required_outer))
+ 			{
+ 				found = true;
+ 				break;
+ 			}
+ 		}
+ 		if (found)
+ 			continue;
+ 
+ 		/* Save the required_outer to our req_outer_list */
+ 		fpinfo->req_outer_list = lappend(fpinfo->req_outer_list,
+ 										 required_outer);
+ 
+ 		/* Make an alternative local join path */
+ 		if (root->parse->commandType == CMD_DELETE ||
+ 			root->parse->commandType == CMD_UPDATE ||
+ 			root->rowMarks)
+ 		{
+ 			Path	   *outer_path = outerrel->cheapest_total_path;
+ 			Path	   *inner_path = innerrel->cheapest_total_path;
+ 
+ 			/* The cheapest total paths should be unparameterized */
+ 			Assert(outer_path->param_info == NULL);
+ 			Assert(inner_path->param_info == NULL);
+ 
+ 			/* Create a parameterized local join path */
+ 			epq_path = CreateLocalJoinPath(root, joinrel,
+ 										   outer_path,
+ 										   inner_path,
+ 										   required_outer,
+ 										   fpinfo->jointype,
+ 										   fpinfo->extra);
+ 			if (!epq_path)
+ 			{
+ 				elog(DEBUG3, "could not push down foreign join because a local path suitable for EPQ checks could not be created");
+ 				continue;
+ 			}
+ 		}
+ 		else
+ 			epq_path = NULL;
+ 
+ 		/* Identify all join clauses that are movable to this rel */
+ 		param_join_conds = build_joinrel_param_join_conds(root, joinrel,
+ 														  required_outer);
+ 
+ 		/* Check if there are safe-to-send-to-remote join clauses */
+ 		found = false;
+ 		foreach(lc2, param_join_conds)
+ 		{
+ 			RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc2);
+ 
+ 			if (is_foreign_expr(root, joinrel, rinfo->clause))
+ 			{
+ 				found = true;
+ 				break;
+ 			}
+ 		}
+ 		if (!found)
+ 			continue;
+ 
+ 		/* Get a cost estimate from the remote */
+ 		estimate_path_cost_size(root, joinrel, param_join_conds, NIL,
+ 								&rows, &width, &startup_cost, &total_cost);
+ 
+ 		/* Find or build the joinrel ParamPathInfo */
+ 		found = false;
+ 		foreach(lc2, joinrel->ppilist)
+ 		{
+ 			ParamPathInfo *ppi = (ParamPathInfo *) lfirst(lc2);
+ 
+ 			if (bms_equal(ppi->ppi_req_outer, required_outer))
+ 			{
+ 				param_info = ppi;
+ 				found = true;
+ 				break;
+ 			}
+ 		}
+ 		if (!found)
+ 		{
+ 			param_info = makeNode(ParamPathInfo);
+ 			param_info->ppi_req_outer = required_outer;
+ 			param_info->ppi_rows = 0;
+ 			param_info->ppi_clauses = NIL;
+ 			joinrel->ppilist = lappend(joinrel->ppilist, param_info);
+ 		}
+ 		param_info->ppi_rows = rows;
+ 
+ 		/*
+ 		 * If we found the ParamPathInfo, paths in the pathlist might have
+ 		 * looked at ppi_rows, so fix that to ensure that those match our
+ 		 * idea of the rowcount.
+ 		 */
+ 		if (found)
+ 		{
+ 			foreach(lc2, joinrel->pathlist)
+ 			{
+ 				Path	   *old_path = (Path *) lfirst(lc2);
+ 
+ 				if (bms_subset_compare(PATH_REQ_OUTER(old_path),
+ 									   required_outer) == BMS_EQUAL)
+ 				{
+ 					/*
+ 					 * Update the rows of the path
+ 					 *
+ 					 * In principle we might need to re-estimate the cost and
+ 					 * re-add the path to the pathlist because the cost has
+ 					 * been calculated using the old value of ppi_rows, but
+ 					 * that would be expensive.  However, even if we did so,
+ 					 * the path is likely to be dominated by the path created
+ 					 * below unless joinrel->consider_param_startup is true,
+ 					 * so it seems OK to live with the approximation.
+ 					 */
+ 					old_path->rows = rows;
+ 				}
+ 			}
+ 		}
+ 
+ 		/* Make the path */
+ 		path = create_foreignscan_path(root,
+ 									   joinrel,
+ 									   NULL,	/* default pathtarget */
+ 									   rows,
+ 									   startup_cost,
+ 									   total_cost,
+ 									   NIL,		/* no pathkeys */
+ 									   param_info->ppi_req_outer,
+ 									   epq_path,
+ 									   NIL);	/* no fdw_private */
+ 		add_path(joinrel, (Path *) path);
+ 	}
+ }
+ 
+ /*
+  * Build a list of join clauses that are movable to the given join relation
+  * for the given parameterization.
+  */
+ static List *
+ build_joinrel_param_join_conds(PlannerInfo *root,
+ 							   RelOptInfo *joinrel,
+ 							   Relids required_outer)
+ {
+ 	List	   *result;
+ 	Relids		join_and_outer;
+ 	ListCell   *lc;
+ 
+ 	/*
+ 	 * Identify all join clauses that are movable to this join rel given this
+ 	 * parameterization.
+ 	 */
+ 	join_and_outer = bms_union(joinrel->relids, required_outer);
+ 	result = NIL;
+ 	foreach(lc, joinrel->joininfo)
+ 	{
+ 		RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
+ 
+ 		if (join_clause_is_movable_into(rinfo,
+ 										joinrel->relids,
+ 										join_and_outer))
+ 			result = lappend(result, rinfo);
+ 	}
+ 
+ 	/*
+ 	 * Add in join clauses generated by EquivalenceClasses, too
+ 	 */
+ 	result = list_concat(result,
+ 						 generate_join_implied_equalities(root,
+ 														  join_and_outer,
+ 														  required_outer,
+ 														  joinrel));
+ 
+ 	return result;
+ }
+ 
  /*
   * postgresGetForeignJoinPaths
   *		Add possible ForeignPath to joinrel, if join is safe to push down.
***************
*** 4392,4414 **** postgresGetForeignJoinPaths(PlannerInfo *root,
  								 * EvalPlanQual gets triggered. */
  
  	/*
! 	 * Skip if this join combination has been considered already.
  	 */
! 	if (joinrel->fdw_private)
  		return;
  
  	/*
! 	 * Create unfinished PgFdwRelationInfo entry which is used to indicate
! 	 * that the join relation is already considered, so that we won't waste
! 	 * time in judging safety of join pushdown and adding the same paths again
! 	 * if found safe. Once we know that this join can be pushed down, we fill
! 	 * the entry.
  	 */
! 	fpinfo = (PgFdwRelationInfo *) palloc0(sizeof(PgFdwRelationInfo));
! 	fpinfo->pushdown_safe = false;
! 	joinrel->fdw_private = fpinfo;
! 	/* attrs_used is only for base relations. */
! 	fpinfo->attrs_used = NULL;
  
  	/*
  	 * If there is a possibility that EvalPlanQual will be executed, we need
--- 4660,4694 ----
  								 * EvalPlanQual gets triggered. */
  
  	/*
! 	 * If this is not the first call and pushdown_safe is already true, all
! 	 * we need to do is consider parameterized paths for the join relation.
  	 */
! 	if (joinrel->fdw_private &&
! 		((PgFdwRelationInfo *) joinrel->fdw_private)->pushdown_safe)
! 	{
! 		add_parameterized_paths_for_rel(root, joinrel, extra->req_outer_list);
  		return;
+ 	}
  
  	/*
! 	 * If this is the first call, create the PgFdwRelationInfo entry.  Once
! 	 * we know that the join can be pushed down, we fill in the entry.
  	 */
! 	if (!joinrel->fdw_private)
! 	{
! 		fpinfo = (PgFdwRelationInfo *) palloc0(sizeof(PgFdwRelationInfo));
! 		fpinfo->pushdown_safe = false;
! 		/* attrs_used is only for base relations. */
! 		fpinfo->attrs_used = NULL;
! 		/* Initialize req_outer_list to NIL. */
! 		fpinfo->req_outer_list = NIL;
! 		joinrel->fdw_private = fpinfo;
! 	}
! 	else
! 	{
! 		fpinfo = (PgFdwRelationInfo *) joinrel->fdw_private;
! 		Assert(fpinfo->pushdown_safe == false);
! 	}
  
  	/*
  	 * If there is a possibility that EvalPlanQual will be executed, we need
***************
*** 4514,4520 **** postgresGetForeignJoinPaths(PlannerInfo *root,
  	/* Consider pathkeys for the join relation */
  	add_paths_with_pathkeys_for_rel(root, joinrel, epq_path);
  
! 	/* XXX Consider parameterized paths for the join relation */
  }
  
  /*
--- 4794,4801 ----
  	/* Consider pathkeys for the join relation */
  	add_paths_with_pathkeys_for_rel(root, joinrel, epq_path);
  
! 	/* Consider parameterized paths for the join relation */
! 	add_parameterized_paths_for_rel(root, joinrel, extra->req_outer_list);
  }
  
  /*
*** a/contrib/postgres_fdw/postgres_fdw.h
--- b/contrib/postgres_fdw/postgres_fdw.h
***************
*** 91,97 **** typedef struct PgFdwRelationInfo
--- 91,100 ----
  	RelOptInfo *outerrel;
  	RelOptInfo *innerrel;
  	JoinType	jointype;
+ 	JoinPathExtraData *extra;
  	List	   *joinclauses;
+ 	List	   *req_outer_list;	/* outer relations considered so far to build
+ 								 * parameterized foreign join paths */
  
  	/* Grouping information */
  	List	   *grouped_tlist;
***************
*** 172,178 **** extern void deparseAnalyzeSql(StringInfo buf, Relation rel,
  				  List **retrieved_attrs);
  extern void deparseStringLiteral(StringInfo buf, const char *val);
  extern Expr *find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel);
! extern List *build_tlist_to_deparse(RelOptInfo *foreignrel);
  extern void deparseSelectStmtForRel(StringInfo buf, PlannerInfo *root,
  						RelOptInfo *foreignrel, List *tlist,
  						List *remote_conds, List *pathkeys, bool is_subquery,
--- 175,182 ----
  				  List **retrieved_attrs);
  extern void deparseStringLiteral(StringInfo buf, const char *val);
  extern Expr *find_em_expr_for_rel(EquivalenceClass *ec, RelOptInfo *rel);
! extern List *build_tlist_to_deparse(RelOptInfo *foreignrel,
! 									List *local_param_conds);
  extern void deparseSelectStmtForRel(StringInfo buf, PlannerInfo *root,
  						RelOptInfo *foreignrel, List *tlist,
  						List *remote_conds, List *pathkeys, bool is_subquery,
*** a/contrib/postgres_fdw/sql/postgres_fdw.sql
--- b/contrib/postgres_fdw/sql/postgres_fdw.sql
***************
*** 533,538 **** EXPLAIN (VERBOSE, COSTS OFF)
--- 533,547 ----
  SELECT ft5, ft5.c1, ft5.c2, ft5.c3, ft4.c1, ft4.c2 FROM ft5 left join ft4 on ft5.c1 = ft4.c1 WHERE ft4.c1 BETWEEN 10 and 30 ORDER BY ft5.c1, ft4.c1;
  SELECT ft5, ft5.c1, ft5.c2, ft5.c3, ft4.c1, ft4.c2 FROM ft5 left join ft4 on ft5.c1 = ft4.c1 WHERE ft4.c1 BETWEEN 10 and 30 ORDER BY ft5.c1, ft4.c1;
  
+ -- parameterized remote path for foreign join
+ EXPLAIN (VERBOSE, COSTS OFF)
+   SELECT * FROM "S 1"."T 1" a LEFT JOIN (ft1 b INNER JOIN ft2 c ON (b.c1 = c.c1)) ON (c.c1 = a.c2) WHERE a."C 1" = 47;
+ SELECT * FROM "S 1"."T 1" a LEFT JOIN (ft1 b INNER JOIN ft2 c ON (b.c1 = c.c1)) ON (c.c1 = a.c2) WHERE a."C 1" = 47;
+ -- with FOR UPDATE/SHARE
+ EXPLAIN (VERBOSE, COSTS OFF)
+   SELECT * FROM "S 1"."T 1" a LEFT JOIN (ft1 b INNER JOIN ft2 c ON (b.c1 = c.c1)) ON (c.c1 = a.c2) WHERE a."C 1" = 57 FOR UPDATE OF a;
+ SELECT * FROM "S 1"."T 1" a LEFT JOIN (ft1 b INNER JOIN ft2 c ON (b.c1 = c.c1)) ON (c.c1 = a.c2) WHERE a."C 1" = 57 FOR UPDATE OF a;
+ 
  -- check join pushdown in situations where multiple userids are involved
  CREATE ROLE regress_view_owner;
  CREATE USER MAPPING FOR regress_view_owner SERVER loopback;
*** a/src/backend/optimizer/path/joinpath.c
--- b/src/backend/optimizer/path/joinpath.c
***************
*** 121,126 **** add_paths_to_joinrel(PlannerInfo *root,
--- 121,127 ----
  	extra.mergeclauses = NIL;
  	extra.outersortkeys = NIL;
  	extra.innersortkeys = NIL;
+ 	extra.req_outer_list = NIL;
  
  	/*
  	 * Find potential mergejoin clauses.  We can skip this if we are not
***************
*** 334,339 **** try_nestloop_path(PlannerInfo *root,
--- 335,345 ----
  		return;
  	}
  
+ 	/* Save required_outer for possible use by the FDW */
+ 	if (extra->consider_foreignjoin && required_outer)
+ 		extra->req_outer_list = lappend(extra->req_outer_list,
+ 										bms_copy(required_outer));
+ 
  	/*
  	 * Do a precheck to quickly eliminate obviously-inferior paths.  We
  	 * calculate a cheap lower bound on the path's cost and then use
***************
*** 477,482 **** try_mergejoin_path(PlannerInfo *root,
--- 483,493 ----
  		return;
  	}
  
+ 	/* Save required_outer for possible use by the FDW */
+ 	if (extra->consider_foreignjoin && required_outer)
+ 		extra->req_outer_list = lappend(extra->req_outer_list,
+ 										bms_copy(required_outer));
+ 
  	/*
  	 * If the given paths are already well enough ordered, we can skip doing
  	 * an explicit sort.
***************
*** 623,628 **** try_hashjoin_path(PlannerInfo *root,
--- 634,644 ----
  		return;
  	}
  
+ 	/* Save required_outer for possible use by the FDW */
+ 	if (extra->consider_foreignjoin && required_outer)
+ 		extra->req_outer_list = lappend(extra->req_outer_list,
+ 										bms_copy(required_outer));
+ 
  	/*
  	 * See comments in try_nestloop_path().  Also note that hashjoin paths
  	 * never have any output pathkeys, per comments in create_hashjoin_path.
*** a/src/include/nodes/relation.h
--- b/src/include/nodes/relation.h
***************
*** 2159,2164 **** typedef struct SemiAntiJoinFactors
--- 2159,2166 ----
   * mergeclauses are the RestrictInfos to use as merge clauses in a mergejoin
   * outersortkeys are the sort pathkeys for the outer side of the mergejoin
   * innersortkeys are the sort pathkeys for the inner side of the mergejoin
+  * req_outer_list is a list of Relids of sensible parameterizations for the
+  *		join rel
   */
  typedef struct JoinPathExtraData
  {
***************
*** 2173,2178 **** typedef struct JoinPathExtraData
--- 2175,2181 ----
  	List	   *mergeclauses;
  	List	   *outersortkeys;
  	List	   *innersortkeys;
+ 	List	   *req_outer_list;
  } JoinPathExtraData;
  
  /*

---

From: Arthur Zakirov <[email-masked]>
Date: Fri, 7 Apr 2017 16:54:17 +0300

Thank you!

 > postgresGetForeignJoinPaths.  See foreign_join_ok.)

Oh, I see. I've missed that condition.

Marked the patch as "Ready for Commiter". But the patch should be 
commited only after the patch [1].

1.

---

From: David Steele <[email-masked]>
Date: Sat, 8 Apr 2017 10:16:25 -0400

> commited only after the patch [1].

This submission has been moved to CF 2017-07.

-- 
-David
[email-masked]

---

From: Etsuro Fujita <[email-masked]>
Date: Tue, 11 Apr 2017 17:55:42 +0900

> commited only after the patch [1].

Thanks for reviewing!  I'll continue to work on this for PG11.

Best regards,
Etsuro Fujita