diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 99629e602aac1b672d0bc05acca4f9574724c88e..c81efe93a47a3a8b9f9afd02a9041d66fea00164 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -103,6 +103,7 @@ static void check_output_expressions(Query *subquery,
pushdown_safety_info *safetyInfo);
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, Node *qual,
pushdown_safety_info *safetyInfo);
static void subquery_push_qual(Query *subquery,
@@ -1688,13 +1689,10 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
* 1. If the subquery has a LIMIT clause, we must not push down any quals,
* since that could change the set of rows returned.
*
- * 2. If the subquery contains any window functions, we can't push quals
- * into it, because that could change the results.
- *
- * 3. If the subquery contains EXCEPT or EXCEPT ALL set ops we cannot push
+ * 2. If the subquery contains EXCEPT or EXCEPT ALL set ops we cannot push
* quals into it, because that could change the results.
*
- * 4. If the subquery uses DISTINCT, we cannot push volatile quals into it.
+ * 3. If the subquery uses DISTINCT, we cannot push volatile quals into it.
* This is because upper-level quals should semantically be evaluated only
* once per distinct row, not once per original row, and if the qual is
* volatile then extra evaluations could change the results. (This issue
@@ -1702,6 +1700,12 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
* when those are present we push into HAVING not WHERE, so that the quals
* are still applied after aggregation.)
*
+ * 4. If the subquery contains window functions, we cannot push volatile quals
+ * into it. The issue here is a bit different from DISTINCT: a volatile qual
+ * might succeed for some rows of a window partition and fail for others,
+ * thereby changing the partition contents and thus the window functions'
+ * results for rows that remain.
+ *
* 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]
@@ -1723,6 +1727,18 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels)
* more than we save by eliminating rows before the DISTINCT step. But it
* would be very hard to estimate that at this stage, and in practice pushdown
* seldom seems to make things worse, so we ignore that problem too.
+ *
+ * Note: likewise, pushing quals into a subquery with window functions is a
+ * bit dubious: the quals might remove some rows of a window partition while
+ * leaving others, causing changes in the window functions' results for the
+ * surviving rows. We insist that such a qual reference only partitioning
+ * columns, but again that only protects us if the qual does not distinguish
+ * values that the partitioning equality operator sees as equal. The risks
+ * here are perhaps larger than for DISTINCT, since no de-duplication of rows
+ * occurs and thus there is no theoretical problem with such a qual. But
+ * we'll do this anyway because the potential performance benefits are very
+ * large, and we've seen no field complaints about the longstanding comparable
+ * behavior with DISTINCT.
*/
static bool
subquery_is_pushdown_safe(Query *subquery, Query *topquery,
@@ -1734,12 +1750,8 @@ subquery_is_pushdown_safe(Query *subquery, Query *topquery,
if (subquery->limitOffset != NULL || subquery->limitCount != NULL)
return false;
- /* Check point 2 */
- if (subquery->hasWindowFuncs)
- return false;
-
- /* Check point 4 */
- if (subquery->distinctClause)
+ /* Check points 3 and 4 */
+ if (subquery->distinctClause || subquery->hasWindowFuncs)
safetyInfo->unsafeVolatile = true;
/*
@@ -1795,7 +1807,7 @@ recurse_pushdown_safe(Node *setOp, Query *topquery,
{
SetOperationStmt *op = (SetOperationStmt *) setOp;
- /* EXCEPT is no good (point 3 for subquery_is_pushdown_safe) */
+ /* EXCEPT is no good (point 2 for subquery_is_pushdown_safe) */
if (op->op == SETOP_EXCEPT)
return false;
/* Else recurse */
@@ -1835,6 +1847,15 @@ recurse_pushdown_safe(Node *setOp, Query *topquery,
* there are no non-DISTINCT output columns, so we needn't check. Note that
* subquery_is_pushdown_safe already reported that we can't use volatile
* quals if there's DISTINCT or DISTINCT ON.)
+ *
+ * 4. If the subquery has any window functions, we must not push down quals
+ * that reference any output columns that are not listed in all the subquery's
+ * window PARTITION BY clauses. We can push down quals that use only
+ * partitioning columns because they should succeed or fail identically for
+ * every row of any one window partition, and totally excluding some
+ * partitions will not change a window function's results for remaining
+ * partitions. (Again, this also requires nonvolatile quals, but
+ * subquery_is_pushdown_safe handles that.)
*/
static void
check_output_expressions(Query *subquery, pushdown_safety_info *safetyInfo)
@@ -1874,6 +1895,15 @@ check_output_expressions(Query *subquery, pushdown_safety_info *safetyInfo)
safetyInfo->unsafeColumns[tle->resno] = true;
continue;
}
+
+ /* If subquery uses window functions, check point 4 */
+ if (subquery->hasWindowFuncs &&
+ !targetIsInAllPartitionLists(tle, subquery))
+ {
+ /* not present in all PARTITION BY clauses, so mark it unsafe */
+ safetyInfo->unsafeColumns[tle->resno] = true;
+ continue;
+ }
}
}
@@ -1917,6 +1947,31 @@ compare_tlist_datatypes(List *tlist, List *colTypes,
elog(ERROR, "wrong number of tlist entries");
}
+/*
+ * targetIsInAllPartitionLists
+ * True if the TargetEntry is listed in the PARTITION BY clause
+ * of every window defined in the query.
+ *
+ * It would be safe to ignore windows not actually used by any window
+ * function, but it's not easy to get that info at this stage; and it's
+ * unlikely to be useful to spend any extra cycles getting it, since
+ * unreferenced window definitions are probably infrequent in practice.
+ */
+static bool
+targetIsInAllPartitionLists(TargetEntry *tle, Query *query)
+{
+ ListCell *lc;
+
+ foreach(lc, query->windowClause)
+ {
+ WindowClause *wc = (WindowClause *) lfirst(lc);
+
+ if (!targetIsInSortList(tle, InvalidOid, wc->partitionClause))
+ return false;
+ }
+ return true;
+}
+
/*
* qual_is_pushdown_safe - is a particular qual safe to push down?
*
diff --git a/src/test/regress/expected/window.out b/src/test/regress/expected/window.out
index c2cc742c90382419f779449ed9a350b9316856ee..19f909f3d105087c2babcfa62ff3a77d442a3b03 100644
--- a/src/test/regress/expected/window.out
+++ b/src/test/regress/expected/window.out
@@ -1034,6 +1034,47 @@ FROM empsalary GROUP BY depname;
25100 | 1 | 22600 | develop
(3 rows)
+-- Test pushdown of quals into a subquery containing window functions
+-- pushdown is safe because all PARTITION BY clauses include depname:
+EXPLAIN (COSTS OFF)
+SELECT * FROM
+ (SELECT depname,
+ sum(salary) OVER (PARTITION BY depname) depsalary,
+ min(salary) OVER (PARTITION BY depname || 'A', depname) depminsalary
+ FROM empsalary) emp
+WHERE depname = 'sales';
+ QUERY PLAN
+---------------------------------------------------------------------
+ Subquery Scan on emp
+ -> WindowAgg
+ -> Sort
+ Sort Key: (((empsalary.depname)::text || 'A'::text))
+ -> WindowAgg
+ -> Seq Scan on empsalary
+ Filter: ((depname)::text = 'sales'::text)
+(7 rows)
+
+-- pushdown is unsafe because there's a PARTITION BY clause without depname:
+EXPLAIN (COSTS OFF)
+SELECT * FROM
+ (SELECT depname,
+ sum(salary) OVER (PARTITION BY enroll_date) enroll_salary,
+ min(salary) OVER (PARTITION BY depname) depminsalary
+ FROM empsalary) emp
+WHERE depname = 'sales';
+ QUERY PLAN
+-----------------------------------------------------------
+ Subquery Scan on emp
+ Filter: ((emp.depname)::text = 'sales'::text)
+ -> WindowAgg
+ -> Sort
+ Sort Key: empsalary.depname
+ -> WindowAgg
+ -> Sort
+ Sort Key: empsalary.enroll_date
+ -> Seq Scan on empsalary
+(9 rows)
+
-- cleanup
DROP TABLE empsalary;
-- test user-defined window function with named args and default args
diff --git a/src/test/regress/sql/window.sql b/src/test/regress/sql/window.sql
index 31c98eb27050b61163d917199a8f10b0f1cdee4d..e2a1a1cdd5147d2ab328d74c8130adc4feffdd79 100644
--- a/src/test/regress/sql/window.sql
+++ b/src/test/regress/sql/window.sql
@@ -272,6 +272,26 @@ SELECT sum(salary), row_number() OVER (ORDER BY depname), sum(
depname
FROM empsalary GROUP BY depname;
+-- Test pushdown of quals into a subquery containing window functions
+
+-- pushdown is safe because all PARTITION BY clauses include depname:
+EXPLAIN (COSTS OFF)
+SELECT * FROM
+ (SELECT depname,
+ sum(salary) OVER (PARTITION BY depname) depsalary,
+ min(salary) OVER (PARTITION BY depname || 'A', depname) depminsalary
+ FROM empsalary) emp
+WHERE depname = 'sales';
+
+-- pushdown is unsafe because there's a PARTITION BY clause without depname:
+EXPLAIN (COSTS OFF)
+SELECT * FROM
+ (SELECT depname,
+ sum(salary) OVER (PARTITION BY enroll_date) enroll_salary,
+ min(salary) OVER (PARTITION BY depname) depminsalary
+ FROM empsalary) emp
+WHERE depname = 'sales';
+
-- cleanup
DROP TABLE empsalary;