dont_remove_subquery_columns_used_by_wclause_runconditions_v2.patch

text/plain

Filename: dont_remove_subquery_columns_used_by_wclause_runconditions_v2.patch
Type: text/plain
Part: 0
Message: Re: BUG #17495: Regression in 15beta1 when filtering subquery including row_number window function

Patch

Format: unified
Series: patch v2
File+
src/backend/optimizer/path/allpaths.c 32 11
src/test/regress/expected/window.out 19 0
src/test/regress/sql/window.sql 10 0
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 7ac116a791..2e12ec7d77 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -142,7 +142,8 @@ static void subquery_push_qual(Query *subquery,
 							   RangeTblEntry *rte, Index rti, Node *qual);
 static void recurse_push_qual(Node *setOp, Query *topquery,
 							  RangeTblEntry *rte, Index rti, Node *qual);
-static void remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel);
+static void remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel,
+										   Bitmapset *extra_used_attrs);
 
 
 /*
@@ -2177,14 +2178,16 @@ has_multiple_baserels(PlannerInfo *root)
  * the run condition will handle all of the required filtering.
  *
  * Returns true if 'opexpr' was found to be useful and was added to the
- * WindowClauses runCondition. We also set *keep_original accordingly.
+ * WindowClauses runCondition.  We also set *keep_original accordingly and add
+ * 'attno' to *run_cond_attrs offset by FirstLowInvalidHeapAttributeNumber.
  * If the 'opexpr' cannot be used then we set *keep_original to true and
  * return false.
  */
 static bool
 find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
 						   AttrNumber attno, WindowFunc *wfunc, OpExpr *opexpr,
-						   bool wfunc_left, bool *keep_original)
+						   bool wfunc_left, bool *keep_original,
+						   Bitmapset **run_cond_attrs)
 {
 	Oid			prosupport;
 	Expr	   *otherexpr;
@@ -2356,6 +2359,9 @@ find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
 
 		wclause->runCondition = lappend(wclause->runCondition, newexpr);
 
+		/* record that this attno was used in a run condition */
+		*run_cond_attrs = bms_add_member(*run_cond_attrs,
+										 attno - FirstLowInvalidHeapAttributeNumber);
 		return true;
 	}
 
@@ -2369,13 +2375,16 @@ find_window_run_conditions(Query *subquery, RangeTblEntry *rte, Index rti,
  *		WindowClause as a 'runCondition' qual.  These, when present, allow
  *		some unnecessary work to be skipped during execution.
  *
+ * 'run_cond_attrs' will be populated with all targetlist resnos of subquery
+ * targets that we pushed window quals for.
+ *
  * Returns true if the caller still must keep the original qual or false if
  * the caller can safely ignore the original qual because the WindowAgg node
  * will use the runCondition to stop returning tuples.
  */
 static bool
 check_and_push_window_quals(Query *subquery, RangeTblEntry *rte, Index rti,
-							Node *clause)
+							Node *clause, Bitmapset **run_cond_attrs)
 {
 	OpExpr	   *opexpr = (OpExpr *) clause;
 	bool		keep_original = true;
@@ -2403,7 +2412,8 @@ check_and_push_window_quals(Query *subquery, RangeTblEntry *rte, Index rti,
 		WindowFunc *wfunc = (WindowFunc *) tle->expr;
 
 		if (find_window_run_conditions(subquery, rte, rti, tle->resno, wfunc,
-									   opexpr, true, &keep_original))
+									   opexpr, true, &keep_original,
+									   run_cond_attrs))
 			return keep_original;
 	}
 
@@ -2415,7 +2425,8 @@ check_and_push_window_quals(Query *subquery, RangeTblEntry *rte, Index rti,
 		WindowFunc *wfunc = (WindowFunc *) tle->expr;
 
 		if (find_window_run_conditions(subquery, rte, rti, tle->resno, wfunc,
-									   opexpr, false, &keep_original))
+									   opexpr, false, &keep_original,
+									   run_cond_attrs))
 			return keep_original;
 	}
 
@@ -2444,6 +2455,7 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
 	pushdown_safety_info safetyInfo;
 	double		tuple_fraction;
 	RelOptInfo *sub_final_rel;
+	Bitmapset  *run_cond_attrs = NULL;
 	ListCell   *lc;
 
 	/*
@@ -2526,7 +2538,8 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
 				 * it might be useful to use for the WindowAgg's runCondition.
 				 */
 				if (!subquery->hasWindowFuncs ||
-					check_and_push_window_quals(subquery, rte, rti, clause))
+					check_and_push_window_quals(subquery, rte, rti, clause,
+												&run_cond_attrs))
 				{
 					/*
 					 * subquery has no window funcs or the clause is not a
@@ -2545,9 +2558,11 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel,
 
 	/*
 	 * The upper query might not use all the subquery's output columns; if
-	 * not, we can simplify.
+	 * not, we can simplify.  Pass the attributes that were pushed down into
+	 * WindowAgg run conditions to ensure we don't accidentally think those
+	 * are unused.
 	 */
-	remove_unused_subquery_outputs(subquery, rel);
+	remove_unused_subquery_outputs(subquery, rel, run_cond_attrs);
 
 	/*
 	 * We can safely pass the outer tuple_fraction down to the subquery if the
@@ -3945,14 +3960,20 @@ recurse_push_qual(Node *setOp, Query *topquery,
  * compute expressions, but because deletion of output columns might allow
  * optimizations such as join removal to occur within the subquery.
  *
+ * extra_used_attrs can passed as non-NULL to mark any columns that we should
+ * not remove.  This parameter is modifed by the function, so callers should
+ * make a copy if they need to use the passed in Bitmapset after calling this
+ * function.
+ * 
  * To avoid affecting column numbering in the targetlist, we don't physically
  * remove unused tlist entries, but rather replace their expressions with NULL
  * constants.  This is implemented by modifying subquery->targetList.
  */
 static void
-remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel)
+remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel,
+							   Bitmapset *extra_used_attrs)
 {
-	Bitmapset  *attrs_used = NULL;
+	Bitmapset  *attrs_used = extra_used_attrs;
 	ListCell   *lc;
 
 	/*
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index d78b4c463c..433a0bb025 100644
--- a/src/test/regress/expected/window.out
+++ b/src/test/regress/expected/window.out
@@ -3589,6 +3589,25 @@ WHERE rn < 3;
      3 | sales     |  2
 (6 rows)
 
+-- ensure that "unused" subquery columns are not removed when the column only
+-- exists in the run condition
+EXPLAIN (COSTS OFF)
+SELECT empno, depname FROM
+  (SELECT empno,
+          depname,
+          row_number() OVER (PARTITION BY depname ORDER BY empno) rn
+   FROM empsalary) emp
+WHERE rn < 3;
+                         QUERY PLAN                         
+------------------------------------------------------------
+ Subquery Scan on emp
+   ->  WindowAgg
+         Run Condition: (row_number() OVER (?) < 3)
+         ->  Sort
+               Sort Key: empsalary.depname, empsalary.empno
+               ->  Seq Scan on empsalary
+(6 rows)
+
 -- likewise with count(empno) instead of row_number()
 EXPLAIN (COSTS OFF)
 SELECT * FROM
diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
index 967b9413de..a504e46e40 100644
--- a/src/test/regress/sql/window.sql
+++ b/src/test/regress/sql/window.sql
@@ -1121,6 +1121,16 @@ SELECT * FROM
    FROM empsalary) emp
 WHERE rn < 3;
 
+-- ensure that "unused" subquery columns are not removed when the column only
+-- exists in the run condition
+EXPLAIN (COSTS OFF)
+SELECT empno, depname FROM
+  (SELECT empno,
+          depname,
+          row_number() OVER (PARTITION BY depname ORDER BY empno) rn
+   FROM empsalary) emp
+WHERE rn < 3;
+
 -- likewise with count(empno) instead of row_number()
 EXPLAIN (COSTS OFF)
 SELECT * FROM