fix_safety_hazard_in_runcondition_qual_pushdown.patch

diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 479a694bca..6e8aa10a9c 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -52,14 +52,29 @@
 #include "utils/lsyscache.h"
 
 
+/* Bitmask flags for pushdown_safety_info.safetyFlags */
+#define TARGET_HAS_VOLATILE_FUNC		(1 << 0)
+#define TARGET_HAS_SET_FUNC				(1 << 1)
+#define TARGET_NOTIN_DISTINCTON_CLAUSE	(1 << 2)
+#define TARGET_NOTIN_PARTITIONBY_CLAUSE	(1 << 3)
+#define TARGET_TYPE_MISMATCH			(1 << 4)
+
 /* results of subquery_is_pushdown_safe */
 typedef struct pushdown_safety_info
 {
-	bool	   *unsafeColumns;	/* which output columns are unsafe to use */
+	unsigned char	   *safetyFlags;	/* bitmask flags for output columns */
 	bool		unsafeVolatile; /* don't push down volatile quals */
 	bool		unsafeLeaky;	/* don't push down leaky quals */
 } pushdown_safety_info;
 
+typedef enum pushdown_safe_type
+{
+	pushdown_unsafe,		/* unsafe to push qual into subquery */
+	pushdown_safe,			/* safe to push qual into subquery */
+	pushdown_windowclause_runcond /* unsafe, but may work as WindowClause run
+								   * condition */
+} pushdown_safe_type;
+
 /* These parameters are set by GUC */
 bool		enable_geqo = false;	/* just in case GUC doesn't set it */
 int			geqo_threshold;
@@ -136,9 +151,9 @@ static void check_output_expressions(Query *subquery,
 static void compare_tlist_datatypes(List *tlist, List *colTypes,
 									pushdown_safety_info *safetyInfo);
 static bool targetIsInAllPartitionLists(TargetEntry *tle, Query *query);
-static bool qual_is_pushdown_safe(Query *subquery, Index rti,
-								  RestrictInfo *rinfo,
-								  pushdown_safety_info *safetyInfo);
+static pushdown_safe_type qual_is_pushdown_safe(Query *subquery, Index rti,
+												RestrictInfo *rinfo,
+												pushdown_safety_info *safetyInfo);
 static void subquery_push_qual(Query *subquery,
 							   RangeTblEntry *rte, Index rti, Node *qual);
 static void recurse_push_qual(Node *setOp, Query *topquery,
@@ -2500,13 +2515,14 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
 	/*
 	 * Zero out result area for subquery_is_pushdown_safe, so that it can set
 	 * flags as needed while recursing.  In particular, we need a workspace
-	 * for keeping track of unsafe-to-reference columns.  unsafeColumns[i]
-	 * will be set true if we find that output column i of the subquery is
-	 * unsafe to use in a pushed-down qual.
+	 * for keeping track of the reasons why a qual is unsafe to push down.
+	 * These reasons are stored in the bits inside safetyFlags[i] when we
+	 * discover reasons that column i of the subquery is unsafe to be used as
+	 * a pushed-down qual.
 	 */
 	memset(&safetyInfo, 0, sizeof(safetyInfo));
-	safetyInfo.unsafeColumns = (bool *)
-		palloc0((list_length(subquery->targetList) + 1) * sizeof(bool));
+	safetyInfo.safetyFlags = (unsigned char *)
+		palloc0((list_length(subquery->targetList) + 1) * sizeof(unsigned char));
 
 	/*
 	 * If the subquery has the "security_barrier" flag, it means the subquery
@@ -2549,29 +2565,37 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
 			RestrictInfo *rinfo = (RestrictInfo *) lfirst(l);
 			Node	   *clause = (Node *) rinfo->clause;
 
-			if (!rinfo->pseudoconstant &&
-				qual_is_pushdown_safe(subquery, rti, rinfo, &safetyInfo))
+			if (!rinfo->pseudoconstant)
 			{
-				/* Push it down */
-				subquery_push_qual(subquery, rte, rti, clause);
-			}
-			else
-			{
-				/*
-				 * Since we can't push the qual down into the subquery, check
-				 * if it happens to reference a window function.  If so then
-				 * it might be useful to use for the WindowAgg's runCondition.
-				 */
-				if (!subquery->hasWindowFuncs ||
-					check_and_push_window_quals(subquery, rte, rti, clause,
-												&run_cond_attrs))
+				switch (qual_is_pushdown_safe(subquery, rti, rinfo, &safetyInfo))
 				{
-					/*
-					 * subquery has no window funcs or the clause is not a
-					 * suitable window run condition qual or it is, but the
-					 * original must also be kept in the upper query.
-					 */
-					upperrestrictlist = lappend(upperrestrictlist, rinfo);
+					case pushdown_safe:
+						/* Push it down */
+						subquery_push_qual(subquery, rte, rti, clause);
+						break;
+
+					case pushdown_windowclause_runcond:
+						/*
+						 * Since we can't push the qual down into the subquery, check
+						 * if it happens to reference a window function.  If so then
+						 * it might be useful to use for the WindowAgg's runCondition.
+						 */
+						if (!subquery->hasWindowFuncs ||
+							check_and_push_window_quals(subquery, rte, rti, clause,
+														&run_cond_attrs))
+						{
+							/*
+							 * subquery has no window funcs or the clause is not a
+							 * suitable window run condition qual or it is, but the
+							 * original must also be kept in the upper query.
+							 */
+							upperrestrictlist = lappend(upperrestrictlist, rinfo);
+						}
+						break;
+
+					case pushdown_unsafe:
+						upperrestrictlist = lappend(upperrestrictlist, rinfo);
+						break;
 				}
 			}
 		}
@@ -2579,7 +2603,7 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
 		/* We don't bother recomputing baserestrict_min_security */
 	}
 
-	pfree(safetyInfo.unsafeColumns);
+	pfree(safetyInfo.safetyFlags);
 
 	/*
 	 * The upper query might not use all the subquery's output columns; if
@@ -3517,13 +3541,13 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
  *
  * 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 safetyInfo->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.
+ * is found to be unsafe to reference, we set the reason for that inside
+ * safetyInfo->safetyFlags[k], but we don't reject the subquery overall since
+ * column k might not be referenced by some/all quals.  The safetyFlags[]
+ * 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.
  *
  * Note: pushing quals into a DISTINCT subquery is theoretically dubious:
  * we're effectively assuming that the quals cannot distinguish values that
@@ -3571,9 +3595,9 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
 
 	/*
 	 * 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.)
+	 * list, and mark the reasons why they're unsafe in safetyFlags[].
+	 * (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, safetyInfo);
@@ -3644,9 +3668,9 @@ recurse_pushdown_safe(Node *setOp, Query *topquery,
  *
  * 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:
+ * each output column of the subquery and set flags in safetyFlags[k] when we
+ * see 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
@@ -3684,40 +3708,40 @@ check_output_expressions(Query *subquery, pushdown_safety_info *safetyInfo)
 		if (tle->resjunk)
 			continue;			/* ignore resjunk columns */
 
-		/* We need not check further if output col is already known unsafe */
-		if (safetyInfo->unsafeColumns[tle->resno])
-			continue;
-
 		/* Functions returning sets are unsafe (point 1) */
 		if (subquery->hasTargetSRFs &&
+			(safetyInfo->safetyFlags[tle->resno] & TARGET_HAS_SET_FUNC) == 0 &&
 			expression_returns_set((Node *) tle->expr))
 		{
-			safetyInfo->unsafeColumns[tle->resno] = true;
+			safetyInfo->safetyFlags[tle->resno] |= TARGET_HAS_SET_FUNC;
 			continue;
 		}
 
 		/* Volatile functions are unsafe (point 2) */
-		if (contain_volatile_functions((Node *) tle->expr))
+		if ((safetyInfo->safetyFlags[tle->resno] & TARGET_HAS_VOLATILE_FUNC) == 0 &&
+			contain_volatile_functions((Node *) tle->expr))
 		{
-			safetyInfo->unsafeColumns[tle->resno] = true;
+			safetyInfo->safetyFlags[tle->resno] |= TARGET_HAS_VOLATILE_FUNC;
 			continue;
 		}
 
 		/* If subquery uses DISTINCT ON, check point 3 */
 		if (subquery->hasDistinctOn &&
+			(safetyInfo->safetyFlags[tle->resno] & TARGET_NOTIN_DISTINCTON_CLAUSE) == 0 &&
 			!targetIsInSortList(tle, InvalidOid, subquery->distinctClause))
 		{
 			/* non-DISTINCT column, so mark it unsafe */
-			safetyInfo->unsafeColumns[tle->resno] = true;
+			safetyInfo->safetyFlags[tle->resno] |= TARGET_NOTIN_DISTINCTON_CLAUSE;
 			continue;
 		}
 
 		/* If subquery uses window functions, check point 4 */
 		if (subquery->hasWindowFuncs &&
+			(safetyInfo->safetyFlags[tle->resno] & TARGET_NOTIN_DISTINCTON_CLAUSE) == 0 &&
 			!targetIsInAllPartitionLists(tle, subquery))
 		{
 			/* not present in all PARTITION BY clauses, so mark it unsafe */
-			safetyInfo->unsafeColumns[tle->resno] = true;
+			safetyInfo->safetyFlags[tle->resno] |= TARGET_NOTIN_PARTITIONBY_CLAUSE;
 			continue;
 		}
 	}
@@ -3729,8 +3753,8 @@ check_output_expressions(Query *subquery, pushdown_safety_info *safetyInfo)
  * 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.
+ * the top-level setop outputs.  We set the TARGET_TYPE_MISMATCH bit in
+ * safetyFlags[k] 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
@@ -3738,7 +3762,7 @@ check_output_expressions(Query *subquery, pushdown_safety_info *safetyInfo)
  *
  * tlist is a subquery tlist.
  * colTypes is an OID list of the top-level setop's output column types.
- * safetyInfo->unsafeColumns[] is the result array.
+ * safetyInfo is the pushdown_safety_info to set safetyFlags[] for.
  */
 static void
 compare_tlist_datatypes(List *tlist, List *colTypes,
@@ -3756,7 +3780,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))
-			safetyInfo->unsafeColumns[tle->resno] = true;
+			safetyInfo->safetyFlags[tle->resno] |= TARGET_TYPE_MISMATCH;
 		colType = lnext(colTypes, colType);
 	}
 	if (colType != NULL)
@@ -3816,28 +3840,28 @@ targetIsInAllPartitionLists(TargetEntry *tle, Query *query)
  * 5. rinfo's clause must not refer to any subquery output columns that were
  * found to be unsafe to reference by subquery_is_pushdown_safe().
  */
-static bool
+static pushdown_safe_type
 qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo,
 					  pushdown_safety_info *safetyInfo)
 {
-	bool		safe = true;
+	pushdown_safe_type		safe = pushdown_safe;
 	Node	   *qual = (Node *) rinfo->clause;
 	List	   *vars;
 	ListCell   *vl;
 
 	/* Refuse subselects (point 1) */
 	if (contain_subplans(qual))
-		return false;
+		return pushdown_unsafe;
 
 	/* Refuse volatile quals if we found they'd be unsafe (point 2) */
 	if (safetyInfo->unsafeVolatile &&
 		contain_volatile_functions((Node *) rinfo))
-		return false;
+		return pushdown_unsafe;
 
 	/* Refuse leaky quals if told to (point 3) */
 	if (safetyInfo->unsafeLeaky &&
 		contain_leaked_vars(qual))
-		return false;
+		return pushdown_unsafe;
 
 	/*
 	 * It would be unsafe to push down window function calls, but at least for
@@ -3866,7 +3890,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo,
 		 */
 		if (!IsA(var, Var))
 		{
-			safe = false;
+			safe = pushdown_unsafe;
 			break;
 		}
 
@@ -3878,7 +3902,7 @@ qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo,
 		 */
 		if (var->varno != rti)
 		{
-			safe = false;
+			safe = pushdown_unsafe;
 			break;
 		}
 
@@ -3888,15 +3912,26 @@ qual_is_pushdown_safe(Query *subquery, Index rti, RestrictInfo *rinfo,
 		/* Check point 4 */
 		if (var->varattno == 0)
 		{
-			safe = false;
+			safe = pushdown_unsafe;
 			break;
 		}
 
 		/* Check point 5 */
-		if (safetyInfo->unsafeColumns[var->varattno])
+		if (safetyInfo->safetyFlags[var->varattno] != 0)
 		{
-			safe = false;
-			break;
+			if (safetyInfo->safetyFlags[var->varattno] &
+				(TARGET_HAS_VOLATILE_FUNC|TARGET_HAS_SET_FUNC|
+				 TARGET_NOTIN_DISTINCTON_CLAUSE|TARGET_TYPE_MISMATCH))
+			{
+				safe = pushdown_unsafe;
+				break;
+			}
+			else
+			{
+				/* TARGET_NOTIN_PARTITIONBY_CLAUSE is ok for run conditions */
+				safe = pushdown_windowclause_runcond;
+				/* don't break, we might find another Var to be unsafe */
+			}
 		}
 	}