From: Tom Lane Date: Sat, 28 Jun 2014 06:08:08 +0000 (-0700) Subject: Allow pushdown of WHERE quals into subqueries with window functions. X-Git-Tag: REL9_5_ALPHA1~1795 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d222585a9f7a18f2d793785c82be4c877b90c461;p=postgresql Allow pushdown of WHERE quals into subqueries with window functions. We can allow this even without any specific knowledge of the semantics of the window function, so long as pushed-down quals will either accept every row in a given window partition, or reject every such row. Because window functions act only within a partition, such a case can't result in changing the window functions' outputs for any surviving row. Eliminating entire partitions in this way obviously can reduce the cost of the window-function computations substantially. The fly in the ointment is that it's hard to be entirely sure whether this is true for an arbitrary qual condition. This patch allows pushdown if (a) the qual references only partitioning columns, and (b) the qual contains no volatile functions. We are at risk of incorrect results if the qual can produce different answers for values that the partitioning equality operator sees as equal. While it's not hard to invent cases for which that can happen, it seems to seldom be a problem in practice, since no one has complained about a similar assumption that we've had for many years with respect to DISTINCT. The potential performance gains seem to be worth the risk. David Rowley, reviewed by Vik Fearing; some credit is due also to Thomas Mayer who did considerable preliminary investigation. --- diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 99629e602a..c81efe93a4 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 c2cc742c90..19f909f3d1 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 31c98eb270..e2a1a1cdd5 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;