Disable support for partitionwise joins in problematic cases.

Commit f49842d, which added support for partitionwise joins, built the
child's tlist by applying adjust_appendrel_attrs() to the parent's.  So in
the case where the parent's included a whole-row Var for the parent, the
child's contained a ConvertRowtypeExpr.  To cope with that, that commit
added code to the planner, such as setrefs.c, but some code paths still
assumed that the tlist for a scan (or join) rel would only include Vars
and PlaceHolderVars, which was true before that commit, causing errors:

* When creating an explicit sort node for an input path for a mergejoin
  path for a child join, prepare_sort_from_pathkeys() threw the 'could not
  find pathkey item to sort' error.
* When deparsing a relation participating in a pushed down child join as a
  subquery in contrib/postgres_fdw, get_relation_column_alias_ids() threw
  the 'unexpected expression in subquery output' error.
* When performing set_plan_references() on a local join plan generated by
  contrib/postgres_fdw for EvalPlanQual support for a pushed down child
  join, fix_join_expr() threw the 'variable not found in subplan target
  lists' error.

To fix these, two approaches have been proposed: one by Ashutosh Bapat and
one by me.  While the former keeps building the child's tlist with a
ConvertRowtypeExpr, the latter builds it with a whole-row Var for the
child not to violate the planner assumption, and tries to fix it up later,
But both approaches need more work, so refuse to generate partitionwise
join paths when whole-row Vars are involved, instead.  We don't need to
handle ConvertRowtypeExprs in the child's tlists for now, so this commit
also removes the changes to the planner.

Previously, partitionwise join computed attr_needed data for each child
separately, and built the child join's tlist using that data, which also
required an extra step for adding PlaceHolderVars to that tlist, but it
would be more efficient to build it from the parent join's tlist through
the adjust_appendrel_attrs() transformation.  So this commit builds that
list that way, and simplifies build_joinrel_tlist() and placeholder.c as
well as part of set_append_rel_size() to basically what they were before
partitionwise join went in.

Back-patch to PG11 where partitionwise join was introduced.

Report by Rajkumar Raghuwanshi.  Analysis by Ashutosh Bapat, who also
provided some of regression tests.  Patch by me, reviewed by Robert Haas.

Discussion: https://postgr.es/m/CAKcux6ktu-8tefLWtQuuZBYFaZA83vUzuRd7c1YHC-yEWyYFpg@mail.gmail.com

1 files changed, 64 insertions, 61 deletions

diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index c69740eda6c..39f5729b915 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -57,6 +57,11 @@ static void add_join_rel(PlannerInfo *root, RelOptInfo *joinrel);
 static void build_joinrel_partition_info(RelOptInfo *joinrel,
 							 RelOptInfo *outer_rel, RelOptInfo *inner_rel,
 							 List *restrictlist, JoinType jointype);
+static void build_child_join_reltarget(PlannerInfo *root,
+						   RelOptInfo *parentrel,
+						   RelOptInfo *childrel,
+						   int nappinfos,
+						   AppendRelInfo **appinfos);
 
 
 /*
@@ -188,6 +193,7 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent)
 	rel->baserestrict_min_security = UINT_MAX;
 	rel->joininfo = NIL;
 	rel->has_eclass_joins = false;
+	rel->consider_partitionwise_join = false; /* might get changed later */
 	rel->part_scheme = NULL;
 	rel->nparts = 0;
 	rel->boundinfo = NULL;
@@ -602,6 +608,7 @@ build_join_rel(PlannerInfo *root,
 	joinrel->baserestrict_min_security = UINT_MAX;
 	joinrel->joininfo = NIL;
 	joinrel->has_eclass_joins = false;
+	joinrel->consider_partitionwise_join = false; /* might get changed later */
 	joinrel->top_parent_relids = NULL;
 	joinrel->part_scheme = NULL;
 	joinrel->nparts = 0;
@@ -732,6 +739,9 @@ build_child_join_rel(PlannerInfo *root, RelOptInfo *outer_rel,
 	/* Only joins between "other" relations land here. */
 	Assert(IS_OTHER_REL(outer_rel) && IS_OTHER_REL(inner_rel));
 
+	/* The parent joinrel should have consider_partitionwise_join set. */
+	Assert(parent_joinrel->consider_partitionwise_join);
+
 	joinrel->reloptkind = RELOPT_OTHER_JOINREL;
 	joinrel->relids = bms_union(outer_rel->relids, inner_rel->relids);
 	joinrel->rows = 0;
@@ -773,6 +783,7 @@ build_child_join_rel(PlannerInfo *root, RelOptInfo *outer_rel,
 	joinrel->baserestrictcost.per_tuple = 0;
 	joinrel->joininfo = NIL;
 	joinrel->has_eclass_joins = false;
+	joinrel->consider_partitionwise_join = false; /* might get changed later */
 	joinrel->top_parent_relids = NULL;
 	joinrel->part_scheme = NULL;
 	joinrel->nparts = 0;
@@ -789,14 +800,13 @@ build_child_join_rel(PlannerInfo *root, RelOptInfo *outer_rel,
 	/* Compute information relevant to foreign relations. */
 	set_foreign_rel_properties(joinrel, outer_rel, inner_rel);
 
-	/* Build targetlist */
-	build_joinrel_tlist(root, joinrel, outer_rel);
-	build_joinrel_tlist(root, joinrel, inner_rel);
-	/* Add placeholder variables. */
-	add_placeholders_to_child_joinrel(root, joinrel, parent_joinrel);
+	appinfos = find_appinfos_by_relids(root, joinrel->relids, &nappinfos);
+
+	/* Set up reltarget struct */
+	build_child_join_reltarget(root, parent_joinrel, joinrel,
+							   nappinfos, appinfos);
 
 	/* Construct joininfo list. */
-	appinfos = find_appinfos_by_relids(root, joinrel->relids, &nappinfos);
 	joinrel->joininfo = (List *) adjust_appendrel_attrs(root,
 														(Node *) parent_joinrel->joininfo,
 														nappinfos,
@@ -826,7 +836,6 @@ build_child_join_rel(PlannerInfo *root, RelOptInfo *outer_rel,
 	/* Child joinrel is parallel safe if parent is parallel safe. */
 	joinrel->consider_parallel = parent_joinrel->consider_parallel;
 
-
 	/* Set estimates of the child-joinrel's size. */
 	set_joinrel_size_estimates(root, joinrel, outer_rel, inner_rel,
 							   sjinfo, restrictlist);
@@ -895,15 +904,9 @@ static void
 build_joinrel_tlist(PlannerInfo *root, RelOptInfo *joinrel,
 					RelOptInfo *input_rel)
 {
-	Relids		relids;
+	Relids		relids = joinrel->relids;
 	ListCell   *vars;
 
-	/* attrs_needed refers to parent relids and not those of a child. */
-	if (joinrel->top_parent_relids)
-		relids = joinrel->top_parent_relids;
-	else
-		relids = joinrel->relids;
-
 	foreach(vars, input_rel->reltarget->exprs)
 	{
 		Var		   *var = (Var *) lfirst(vars);
@@ -919,54 +922,23 @@ build_joinrel_tlist(PlannerInfo *root, RelOptInfo *joinrel,
 
 		/*
 		 * Otherwise, anything in a baserel or joinrel targetlist ought to be
-		 * a Var. Children of a partitioned table may have ConvertRowtypeExpr
-		 * translating whole-row Var of a child to that of the parent.
-		 * Children of an inherited table or subquery child rels can not
-		 * directly participate in a join, so other kinds of nodes here.
+		 * a Var.  (More general cases can only appear in appendrel child
+		 * rels, which will never be seen here.)
 		 */
-		if (IsA(var, Var))
-		{
-			baserel = find_base_rel(root, var->varno);
-			ndx = var->varattno - baserel->min_attr;
-		}
-		else if (IsA(var, ConvertRowtypeExpr))
-		{
-			ConvertRowtypeExpr *child_expr = (ConvertRowtypeExpr *) var;
-			Var		   *childvar = (Var *) child_expr->arg;
-
-			/*
-			 * Child's whole-row references are converted to look like those
-			 * of parent using ConvertRowtypeExpr. There can be as many
-			 * ConvertRowtypeExpr decorations as the depth of partition tree.
-			 * The argument to the deepest ConvertRowtypeExpr is expected to
-			 * be a whole-row reference of the child.
-			 */
-			while (IsA(childvar, ConvertRowtypeExpr))
-			{
-				child_expr = (ConvertRowtypeExpr *) childvar;
-				childvar = (Var *) child_expr->arg;
-			}
-			Assert(IsA(childvar, Var) &&childvar->varattno == 0);
-
-			baserel = find_base_rel(root, childvar->varno);
-			ndx = 0 - baserel->min_attr;
-		}
-		else
+		if (!IsA(var, Var))
 			elog(ERROR, "unexpected node type in rel targetlist: %d",
 				 (int) nodeTag(var));
 
+		/* Get the Var's original base rel */
+		baserel = find_base_rel(root, var->varno);
 
-		/* Is the target expression still needed above this joinrel? */
+		/* Is it still needed above this joinrel? */
+		ndx = var->varattno - baserel->min_attr;
 		if (bms_nonempty_difference(baserel->attr_needed[ndx], relids))
 		{
 			/* Yup, add it to the output */
 			joinrel->reltarget->exprs = lappend(joinrel->reltarget->exprs, var);
-
-			/*
-			 * Vars have cost zero, so no need to adjust reltarget->cost. Even
-			 * if it's a ConvertRowtypeExpr, it will be computed only for the
-			 * base relation, costing nothing for a join.
-			 */
+			/* Vars have cost zero, so no need to adjust reltarget->cost */
 			joinrel->reltarget->width += baserel->attr_widths[ndx];
 		}
 	}
@@ -1626,16 +1598,18 @@ build_joinrel_partition_info(RelOptInfo *joinrel, RelOptInfo *outer_rel,
 
 	/*
 	 * We can only consider this join as an input to further partitionwise
-	 * joins if (a) the input relations are partitioned, (b) the partition
-	 * schemes match, and (c) we can identify an equi-join between the
-	 * partition keys.  Note that if it were possible for
-	 * have_partkey_equi_join to return different answers for the same joinrel
-	 * depending on which join ordering we try first, this logic would break.
-	 * That shouldn't happen, though, because of the way the query planner
-	 * deduces implied equalities and reorders the joins. Please see
-	 * optimizer/README for details.
+	 * joins if (a) the input relations are partitioned and have
+	 * consider_partitionwise_join=true, (b) the partition schemes match, and
+	 * (c) we can identify an equi-join between the partition keys.  Note that
+	 * if it were possible for have_partkey_equi_join to return different
+	 * answers for the same joinrel depending on which join ordering we try
+	 * first, this logic would break.  That shouldn't happen, though, because
+	 * of the way the query planner deduces implied equalities and reorders
+	 * the joins.  Please see optimizer/README for details.
 	 */
 	if (!IS_PARTITIONED_REL(outer_rel) || !IS_PARTITIONED_REL(inner_rel) ||
+		!outer_rel->consider_partitionwise_join ||
+		!inner_rel->consider_partitionwise_join ||
 		outer_rel->part_scheme != inner_rel->part_scheme ||
 		!have_partkey_equi_join(joinrel, outer_rel, inner_rel,
 								jointype, restrictlist))
@@ -1687,6 +1661,12 @@ build_joinrel_partition_info(RelOptInfo *joinrel, RelOptInfo *outer_rel,
 	joinrel->part_rels =
 		(RelOptInfo **) palloc0(sizeof(RelOptInfo *) * joinrel->nparts);
 
+	/*
+	 * Set the consider_partitionwise_join flag.
+	 */
+	Assert(outer_rel->consider_partitionwise_join);
+	Assert(inner_rel->consider_partitionwise_join);
+	joinrel->consider_partitionwise_join = true;
 
 	/*
 	 * Construct partition keys for the join.
@@ -1768,3 +1748,26 @@ build_joinrel_partition_info(RelOptInfo *joinrel, RelOptInfo *outer_rel,
 		joinrel->nullable_partexprs[cnt] = nullable_partexpr;
 	}
 }
+
+/*
+ * build_child_join_reltarget
+ *	  Set up a child-join relation's reltarget from a parent-join relation.
+ */
+static void
+build_child_join_reltarget(PlannerInfo *root,
+						   RelOptInfo *parentrel,
+						   RelOptInfo *childrel,
+						   int nappinfos,
+						   AppendRelInfo **appinfos)
+{
+	/* Build the targetlist */
+	childrel->reltarget->exprs = (List *)
+		adjust_appendrel_attrs(root,
+							   (Node *) parentrel->reltarget->exprs,
+							   nappinfos, appinfos);
+
+	/* Set the cost and width fields */
+	childrel->reltarget->cost.startup = parentrel->reltarget->cost.startup;
+	childrel->reltarget->cost.per_tuple = parentrel->reltarget->cost.per_tuple;
+	childrel->reltarget->width = parentrel->reltarget->width;
+}