Handle restriction clause lists more uniformly in postgres_fdw.

Clauses in the lists retained by postgres_fdw during planning were
sometimes bare boolean clauses, sometimes RestrictInfos, and sometimes
a mixture of the two in the same list.  The comment about that situation
didn't come close to telling the full truth, either.  Aside from being
confusing, this had a couple of bad practical consequences:
* waste of planning cycles due to inability to cache per-clause selectivity
and cost estimates;
* sometimes, RestrictInfos would sneak into the fdw_private list of a
finished Plan node, causing failures if, for example, we tried to ship
the Plan tree to a parallel worker.
(It may well be that it's a bug in the parallel-query logic that we
would ever try to ship such a plan to a parallel worker, but in any
case this deserves to be cleaned up.)

To fix, rearrange so that clause lists in PgFdwRelationInfo are always
lists of RestrictInfos, and then strip the RestrictInfos at the last
minute when making a Plan node.  In passing do a bit of refactoring and
comment cleanup in postgresGetForeignPlan and foreign_join_ok.

Although the messiness here dates back at least to 9.6, there's no evidence
that it causes anything worse than wasted planning cycles in 9.6, so no
back-patch for now.

Per fuzz testing by Andreas Seltenreich.

Tom Lane and Ashutosh Bapat

Discussion: https://postgr.es/m/87tw5x4vcu.fsf@credativ.de

1 files changed, 3 insertions, 11 deletions

diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 57dbb798fdb..df75518f465 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -34,16 +34,8 @@ typedef struct PgFdwRelationInfo
 
 	/*
 	 * 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.
+	 * All entries in these lists should have RestrictInfo wrappers; that
+	 * improves efficiency of selectivity and cost estimation.
 	 */
 	List	   *remote_conds;
 	List	   *local_conds;
@@ -91,7 +83,7 @@ typedef struct PgFdwRelationInfo
 	RelOptInfo *outerrel;
 	RelOptInfo *innerrel;
 	JoinType	jointype;
-	List	   *joinclauses;
+	List	   *joinclauses;	/* List of RestrictInfo */
 
 	/* Grouping information */
 	List	   *grouped_tlist;