postgrespro
diff --git a/‎src/backend/optimizer/path/allpaths.c
Lines changed: 125 additions & 91 deletions b/‎src/backend/optimizer/path/allpaths.c
Lines changed: 125 additions & 91 deletions
@@ -82,13 +82,14 @@ static void set_worktable_pathlist(PlannerInfo *root, RelOptInfo *rel,
 					   RangeTblEntry *rte);
 static RelOptInfo *make_rel_from_joinlist(PlannerInfo *root, List *joinlist);
 static bool subquery_is_pushdown_safe(Query *subquery, Query *topquery,
-						  bool *differentTypes);
+						  bool *unsafeColumns);
 static bool recurse_pushdown_safe(Node *setOp, Query *topquery,
-					  bool *differentTypes);
+					  bool *unsafeColumns);
+static void check_output_expressions(Query *subquery, bool *unsafeColumns);
 static void compare_tlist_datatypes(List *tlist, List *colTypes,
-						bool *differentTypes);
+						bool *unsafeColumns);
 static bool qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
-					  bool *differentTypes);
+					  bool *unsafeColumns);
 static void subquery_push_qual(Query *subquery,
 				   RangeTblEntry *rte, Index rti, Node *qual);
 static void recurse_push_qual(Node *setOp, Query *topquery,
@@ -1021,7 +1022,7 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
 {
 	Query	   *parse = root->parse;
 	Query	   *subquery = rte->subquery;
-	bool	   *differentTypes;
+	bool	   *unsafeColumns;
 	double		tuple_fraction;
 	PlannerInfo *subroot;
 	List	   *pathkeys;
@@ -1033,8 +1034,12 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
 	 */
 	subquery = copyObject(subquery);
 
-	/* We need a workspace for keeping track of set-op type coercions */
-	differentTypes = (bool *)
+	/*
+	 * We need a workspace for keeping track of unsafe-to-reference columns.
+	 * unsafeColumns[i] is set TRUE if we've found that output column i of the
+	 * subquery is unsafe to use in a pushed-down qual.
+	 */
+	unsafeColumns = (bool *)
 		palloc0((list_length(subquery->targetList) + 1) * sizeof(bool));
 
 	/*
@@ -1063,7 +1068,7 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
 	 * push down a pushable qual, because it'd result in a worse plan?
 	 */
 	if (rel->baserestrictinfo != NIL &&
-		subquery_is_pushdown_safe(subquery, subquery, differentTypes))
+		subquery_is_pushdown_safe(subquery, subquery, unsafeColumns))
 	{
 		/* OK to consider pushing down individual quals */
 		List	   *upperrestrictlist = NIL;
@@ -1077,7 +1082,7 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
 			if (!rinfo->pseudoconstant &&
 				(!rte->security_barrier ||
 				 !contain_leaky_functions(clause)) &&
-				qual_is_pushdown_safe(subquery, rti, clause, differentTypes))
+				qual_is_pushdown_safe(subquery, rti, clause, unsafeColumns))
 			{
 				/* Push it down */
 				subquery_push_qual(subquery, rte, rti, clause);
@@ -1091,7 +1096,7 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
 		rel->baserestrictinfo = upperrestrictlist;
 	}
 
-	pfree(differentTypes);
+	pfree(unsafeColumns);
 
 	/*
 	 * We can safely pass the outer tuple_fraction down to the subquery if the
@@ -1485,17 +1490,19 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
  * 3. If the subquery contains EXCEPT or EXCEPT ALL set ops we cannot push
  * quals into it, because that could change the results.
  *
- * 4. For subqueries using UNION/UNION ALL/INTERSECT/INTERSECT ALL, we can
- * push quals into each component query, but the quals can only reference
- * subquery columns that suffer no type coercions in the set operation.
- * Otherwise there are possible semantic gotchas.  So, we check the
- * component queries to see if any of them have different output types;
- * differentTypes[k] is set true if column k has different type in any
- * component.
+ * In addition, we make several checks on the subquery's output columns
+ * to see if it is safe to reference them in pushed-down quals.  If output
+ * column k is found to be unsafe to reference, we set unsafeColumns[k] to
+ * TRUE, but we don't reject the subquery overall since column k might
+ * not be referenced by some/all quals.  The unsafeColumns[] array will be
+ * consulted later by qual_is_pushdown_safe().	It's better to do it this
+ * way than to make the checks directly in qual_is_pushdown_safe(), because
+ * when the subquery involves set operations we have to check the output
+ * expressions in each arm of the set op.
  */
 static bool
 subquery_is_pushdown_safe(Query *subquery, Query *topquery,
-						  bool *differentTypes)
+						  bool *unsafeColumns)
 {
 	SetOperationStmt *topop;
 
@@ -1507,13 +1514,22 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
 	if (subquery->hasWindowFuncs)
 		return false;
 
+	/*
+	 * If we're at a leaf query, check for unsafe expressions in its target
+	 * list, and mark any unsafe ones in unsafeColumns[].  (Non-leaf nodes in
+	 * setop trees have only simple Vars in their tlists, so no need to check
+	 * them.)
+	 */
+	if (subquery->setOperations == NULL)
+		check_output_expressions(subquery, unsafeColumns);
+
 	/* Are we at top level, or looking at a setop component? */
 	if (subquery == topquery)
 	{
 		/* Top level, so check any component queries */
 		if (subquery->setOperations != NULL)
 			if (!recurse_pushdown_safe(subquery->setOperations, topquery,
-									   differentTypes))
+									   unsafeColumns))
 				return false;
 	}
 	else
@@ -1526,7 +1542,7 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
 		Assert(topop && IsA(topop, SetOperationStmt));
 		compare_tlist_datatypes(subquery->targetList,
 								topop->colTypes,
-								differentTypes);
+								unsafeColumns);
 	}
 	return true;
 }
@@ -1536,7 +1552,7 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
  */
 static bool
 recurse_pushdown_safe(Node *setOp, Query *topquery,
-					  bool *differentTypes)
+					  bool *unsafeColumns)
 {
 	if (IsA(setOp, RangeTblRef))
 	{
@@ -1545,19 +1561,19 @@ recurse_pushdown_safe(Node *setOp, Query *topquery,
 		Query	   *subquery = rte->subquery;
 
 		Assert(subquery != NULL);
-		return subquery_is_pushdown_safe(subquery, topquery, differentTypes);
+		return subquery_is_pushdown_safe(subquery, topquery, unsafeColumns);
 	}
 	else if (IsA(setOp, SetOperationStmt))
 	{
 		SetOperationStmt *op = (SetOperationStmt *) setOp;
 
-		/* EXCEPT is no good */
+		/* EXCEPT is no good (point 3 for subquery_is_pushdown_safe) */
 		if (op->op == SETOP_EXCEPT)
 			return false;
 		/* Else recurse */
-		if (!recurse_pushdown_safe(op->larg, topquery, differentTypes))
+		if (!recurse_pushdown_safe(op->larg, topquery, unsafeColumns))
 			return false;
-		if (!recurse_pushdown_safe(op->rarg, topquery, differentTypes))
+		if (!recurse_pushdown_safe(op->rarg, topquery, unsafeColumns))
 			return false;
 	}
 	else
@@ -1569,17 +1585,92 @@ recurse_pushdown_safe(Node *setOp, Query *topquery,
 }
 
 /*
- * Compare tlist's datatypes against the list of set-operation result types.
- * For any items that are different, mark the appropriate element of
- * differentTypes[] to show that this column will have type conversions.
+ * check_output_expressions - check subquery's output expressions for safety
+ *
+ * There are several cases in which it's unsafe to push down an upper-level
+ * qual if it references a particular output column of a subquery.	We check
+ * each output column of the subquery and set unsafeColumns[k] to TRUE if
+ * that column is unsafe for a pushed-down qual to reference.  The conditions
+ * checked here are:
+ *
+ * 1. We must not push down any quals that refer to subselect outputs that
+ * return sets, else we'd introduce functions-returning-sets into the
+ * subquery's WHERE/HAVING quals.
+ *
+ * 2. We must not push down any quals that refer to subselect outputs that
+ * contain volatile functions, for fear of introducing strange results due
+ * to multiple evaluation of a volatile function.
+ *
+ * 3. If the subquery uses DISTINCT ON, we must not push down any quals that
+ * refer to non-DISTINCT output columns, because that could change the set
+ * of rows returned.  (This condition is vacuous for DISTINCT, because then
+ * there are no non-DISTINCT output columns, so we needn't check.  But note
+ * we are assuming that the qual can't distinguish values that the DISTINCT
+ * operator sees as equal.	This is a bit shaky but we have no way to test
+ * for the case, and it's unlikely enough that we shouldn't refuse the
+ * optimization just because it could theoretically happen.)
+ */
+static void
+check_output_expressions(Query *subquery, bool *unsafeColumns)
+{
+	ListCell   *lc;
+
+	foreach(lc, subquery->targetList)
+	{
+		TargetEntry *tle = (TargetEntry *) lfirst(lc);
+
+		if (tle->resjunk)
+			continue;			/* ignore resjunk columns */
+
+		/* We need not check further if output col is already known unsafe */
+		if (unsafeColumns[tle->resno])
+			continue;
+
+		/* Functions returning sets are unsafe (point 1) */
+		if (expression_returns_set((Node *) tle->expr))
+		{
+			unsafeColumns[tle->resno] = true;
+			continue;
+		}
+
+		/* Volatile functions are unsafe (point 2) */
+		if (contain_volatile_functions((Node *) tle->expr))
+		{
+			unsafeColumns[tle->resno] = true;
+			continue;
+		}
+
+		/* If subquery uses DISTINCT ON, check point 3 */
+		if (subquery->hasDistinctOn &&
+			!targetIsInSortList(tle, InvalidOid, subquery->distinctClause))
+		{
+			/* non-DISTINCT column, so mark it unsafe */
+			unsafeColumns[tle->resno] = true;
+			continue;
+		}
+	}
+}
+
+/*
+ * For subqueries using UNION/UNION ALL/INTERSECT/INTERSECT ALL, we can
+ * push quals into each component query, but the quals can only reference
+ * subquery columns that suffer no type coercions in the set operation.
+ * Otherwise there are possible semantic gotchas.  So, we check the
+ * component queries to see if any of them have output types different from
+ * the top-level setop outputs.  unsafeColumns[k] is set true if column k
+ * has different type in any component.
  *
  * We don't have to care about typmods here: the only allowed difference
  * between set-op input and output typmods is input is a specific typmod
  * and output is -1, and that does not require a coercion.
+ *
+ * tlist is a subquery tlist.
+ * colTypes is an OID list of the top-level setop's output column types.
+ * unsafeColumns[] is the result array.
  */
 static void
 compare_tlist_datatypes(List *tlist, List *colTypes,
-						bool *differentTypes)
+						bool *unsafeColumns)
 {
 	ListCell   *l;
 	ListCell   *colType = list_head(colTypes);
@@ -1593,7 +1684,7 @@ compare_tlist_datatypes(List *tlist, List *colTypes,
 		if (colType == NULL)
 			elog(ERROR, "wrong number of tlist entries");
 		if (exprType((Node *) tle->expr) != lfirst_oid(colType))
-			differentTypes[tle->resno] = true;
+			unsafeColumns[tle->resno] = true;
 		colType = lnext(colType);
 	}
 	if (colType != NULL)
@@ -1616,34 +1707,15 @@ compare_tlist_datatypes(List *tlist, List *colTypes,
  * (since there is no easy way to name that within the subquery itself).
  *
  * 3. The qual must not refer to any subquery output columns that were
- * found to have inconsistent types across a set operation tree by
- * subquery_is_pushdown_safe().
- *
- * 4. If the subquery uses DISTINCT ON, we must not push down any quals that
- * refer to non-DISTINCT output columns, because that could change the set
- * of rows returned.  (This condition is vacuous for DISTINCT, because then
- * there are no non-DISTINCT output columns, so we needn't check.  But note
- * we are assuming that the qual can't distinguish values that the DISTINCT
- * operator sees as equal.	This is a bit shaky but we have no way to test
- * for the case, and it's unlikely enough that we shouldn't refuse the
- * optimization just because it could theoretically happen.)
- *
- * 5. We must not push down any quals that refer to subselect outputs that
- * return sets, else we'd introduce functions-returning-sets into the
- * subquery's WHERE/HAVING quals.
- *
- * 6. We must not push down any quals that refer to subselect outputs that
- * contain volatile functions, for fear of introducing strange results due
- * to multiple evaluation of a volatile function.
+ * found to be unsafe to reference by subquery_is_pushdown_safe().
  */
 static bool
 qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
-					  bool *differentTypes)
+					  bool *unsafeColumns)
 {
 	bool		safe = true;
 	List	   *vars;
 	ListCell   *vl;
-	Bitmapset  *tested = NULL;
 
 	/* Refuse subselects (point 1) */
 	if (contain_subplans(qual))
@@ -1666,7 +1738,6 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
 	foreach(vl, vars)
 	{
 		Var		   *var = (Var *) lfirst(vl);
-		TargetEntry *tle;
 
 		/*
 		 * XXX Punt if we find any PlaceHolderVars in the restriction clause.
@@ -1682,6 +1753,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
 		}
 
 		Assert(var->varno == rti);
+		Assert(var->varattno >= 0);
 
 		/* Check point 2 */
 		if (var->varattno == 0)
@@ -1690,53 +1762,15 @@ qual_is_pushdown_safe(Query *subquery, Index rti, Node *qual,
 			break;
 		}
 
-		/*
-		 * We use a bitmapset to avoid testing the same attno more than once.
-		 * (NB: this only works because subquery outputs can't have negative
-		 * attnos.)
-		 */
-		if (bms_is_member(var->varattno, tested))
-			continue;
-		tested = bms_add_member(tested, var->varattno);
-
 		/* Check point 3 */
-		if (differentTypes[var->varattno])
-		{
-			safe = false;
-			break;
-		}
-
-		/* Must find the tlist element referenced by the Var */
-		tle = get_tle_by_resno(subquery->targetList, var->varattno);
-		Assert(tle != NULL);
-		Assert(!tle->resjunk);
-
-		/* If subquery uses DISTINCT ON, check point 4 */
-		if (subquery->hasDistinctOn &&
-			!targetIsInSortList(tle, InvalidOid, subquery->distinctClause))
-		{
-			/* non-DISTINCT column, so fail */
-			safe = false;
-			break;
-		}
-
-		/* Refuse functions returning sets (point 5) */
-		if (expression_returns_set((Node *) tle->expr))
-		{
-			safe = false;
-			break;
-		}
-
-		/* Refuse volatile functions (point 6) */
-		if (contain_volatile_functions((Node *) tle->expr))
+		if (unsafeColumns[var->varattno])
 		{
 			safe = false;
 			break;
 		}
 	}
 
 	list_free(vars);
-	bms_free(tested);
 
 	return safe;
 }