mirror of
https://github.com/postgres/postgres.git
synced 2025-07-27 12:41:57 +03:00
Fix bugs with degenerate window ORDER BY clauses in GROUPS/RANGE mode.
nodeWindowAgg.c failed to cope with the possibility that no ordering columns are defined in the window frame for GROUPS mode or RANGE OFFSET mode, leading to assertion failures or odd errors, as reported by Masahiko Sawada and Lukas Eder. In RANGE OFFSET mode, an ordering column is really required, so add an Assert about that. In GROUPS mode, the code would work, except that the node initialization code wasn't in sync with the execution code about when to set up tuplestore read pointers and spare slots. Fix the latter for consistency's sake (even though I think the changes described below make the out-of-sync cases unreachable for now). Per SQL spec, a single ordering column is required for RANGE OFFSET mode, and at least one ordering column is required for GROUPS mode. The parser enforced the former but not the latter; add a check for that. We were able to reach the no-ordering-column cases even with fully spec compliant queries, though, because the planner would drop partitioning and ordering columns from the generated plan if they were redundant with earlier columns according to the redundant-pathkey logic, for instance "PARTITION BY x ORDER BY y" in the presence of a "WHERE x=y" qual. While in principle that's an optimization that could save some pointless comparisons at runtime, it seems unlikely to be meaningful in the real world. I think this behavior was not so much an intentional optimization as a side-effect of an ancient decision to construct the plan node's ordering-column info by reverse-engineering the PathKeys of the input path. If we give up redundant-column removal then it takes very little code to generate the plan node info directly from the WindowClause, ensuring that we have the expected number of ordering columns in all cases. (If anyone does complain about this, the planner could perhaps be taught to remove redundant columns only when it's safe to do so, ie *not* in RANGE OFFSET mode. But I doubt anyone ever will.) With these changes, the WindowAggPath.winpathkeys field is not used for anything anymore, so remove it. The test cases added here are not actually very interesting given the removal of the redundant-column-removal logic, but they would represent important corner cases if anyone ever tries to put that back. Tom Lane and Masahiko Sawada. Back-patch to v11 where RANGE OFFSET and GROUPS modes were added. Discussion: https://postgr.es/m/CAD21AoDrWqycq-w_+Bx1cjc+YUhZ11XTj9rfxNiNDojjBx8Fjw@mail.gmail.com Discussion: https://postgr.es/m/153086788677.17476.8002640580496698831@wrigleys.postgresql.org
This commit is contained in:
Tom Lane
@ -2834,6 +2834,101 @@ SELECT count(*) OVER (PARTITION BY four) FROM (SELECT * FROM tenk1 UNION ALL SEL
|
||||
-------
|
||||
(0 rows)
|
||||
|
||||
-- check some degenerate cases
|
||||
create temp table t1 (f1 int, f2 int8);
|
||||
insert into t1 values (1,1),(1,2),(2,2);
|
||||
select f1, sum(f1) over (partition by f1
|
||||
range between 1 preceding and 1 following)
|
||||
from t1 where f1 = f2; -- error, must have order by
|
||||
ERROR: RANGE with offset PRECEDING/FOLLOWING requires exactly one ORDER BY column
|
||||
LINE 1: select f1, sum(f1) over (partition by f1
|
||||
^
|
||||
explain (costs off)
|
||||
select f1, sum(f1) over (partition by f1 order by f2
|
||||
range between 1 preceding and 1 following)
|
||||
from t1 where f1 = f2;
|
||||
QUERY PLAN
|
||||
---------------------------------
|
||||
WindowAgg
|
||||
-> Sort
|
||||
Sort Key: f1
|
||||
-> Seq Scan on t1
|
||||
Filter: (f1 = f2)
|
||||
(5 rows)
|
||||
|
||||
select f1, sum(f1) over (partition by f1 order by f2
|
||||
range between 1 preceding and 1 following)
|
||||
from t1 where f1 = f2;
|
||||
f1 | sum
|
||||
----+-----
|
||||
1 | 1
|
||||
2 | 2
|
||||
(2 rows)
|
||||
|
||||
select f1, sum(f1) over (partition by f1, f1 order by f2
|
||||
range between 2 preceding and 1 preceding)
|
||||
from t1 where f1 = f2;
|
||||
f1 | sum
|
||||
----+-----
|
||||
1 |
|
||||
2 |
|
||||
(2 rows)
|
||||
|
||||
select f1, sum(f1) over (partition by f1, f2 order by f2
|
||||
range between 1 following and 2 following)
|
||||
from t1 where f1 = f2;
|
||||
f1 | sum
|
||||
----+-----
|
||||
1 |
|
||||
2 |
|
||||
(2 rows)
|
||||
|
||||
select f1, sum(f1) over (partition by f1
|
||||
groups between 1 preceding and 1 following)
|
||||
from t1 where f1 = f2; -- error, must have order by
|
||||
ERROR: GROUPS mode requires an ORDER BY clause
|
||||
LINE 1: select f1, sum(f1) over (partition by f1
|
||||
^
|
||||
explain (costs off)
|
||||
select f1, sum(f1) over (partition by f1 order by f2
|
||||
groups between 1 preceding and 1 following)
|
||||
from t1 where f1 = f2;
|
||||
QUERY PLAN
|
||||
---------------------------------
|
||||
WindowAgg
|
||||
-> Sort
|
||||
Sort Key: f1
|
||||
-> Seq Scan on t1
|
||||
Filter: (f1 = f2)
|
||||
(5 rows)
|
||||
|
||||
select f1, sum(f1) over (partition by f1 order by f2
|
||||
groups between 1 preceding and 1 following)
|
||||
from t1 where f1 = f2;
|
||||
f1 | sum
|
||||
----+-----
|
||||
1 | 1
|
||||
2 | 2
|
||||
(2 rows)
|
||||
|
||||
select f1, sum(f1) over (partition by f1, f1 order by f2
|
||||
groups between 2 preceding and 1 preceding)
|
||||
from t1 where f1 = f2;
|
||||
f1 | sum
|
||||
----+-----
|
||||
1 |
|
||||
2 |
|
||||
(2 rows)
|
||||
|
||||
select f1, sum(f1) over (partition by f1, f2 order by f2
|
||||
groups between 1 following and 2 following)
|
||||
from t1 where f1 = f2;
|
||||
f1 | sum
|
||||
----+-----
|
||||
1 |
|
||||
2 |
|
||||
(2 rows)
|
||||
|
||||
-- ordering by a non-integer constant is allowed
|
||||
SELECT rank() OVER (ORDER BY length('abc'));
|
||||
rank
|
||||
|
||||
@ -795,6 +795,44 @@ WINDOW w AS (ORDER BY x groups between 1 preceding and 1 following);
|
||||
-- with UNION
|
||||
SELECT count(*) OVER (PARTITION BY four) FROM (SELECT * FROM tenk1 UNION ALL SELECT * FROM tenk2)s LIMIT 0;
|
||||
|
||||
-- check some degenerate cases
|
||||
create temp table t1 (f1 int, f2 int8);
|
||||
insert into t1 values (1,1),(1,2),(2,2);
|
||||
|
||||
select f1, sum(f1) over (partition by f1
|
||||
range between 1 preceding and 1 following)
|
||||
from t1 where f1 = f2; -- error, must have order by
|
||||
explain (costs off)
|
||||
select f1, sum(f1) over (partition by f1 order by f2
|
||||
range between 1 preceding and 1 following)
|
||||
from t1 where f1 = f2;
|
||||
select f1, sum(f1) over (partition by f1 order by f2
|
||||
range between 1 preceding and 1 following)
|
||||
from t1 where f1 = f2;
|
||||
select f1, sum(f1) over (partition by f1, f1 order by f2
|
||||
range between 2 preceding and 1 preceding)
|
||||
from t1 where f1 = f2;
|
||||
select f1, sum(f1) over (partition by f1, f2 order by f2
|
||||
range between 1 following and 2 following)
|
||||
from t1 where f1 = f2;
|
||||
|
||||
select f1, sum(f1) over (partition by f1
|
||||
groups between 1 preceding and 1 following)
|
||||
from t1 where f1 = f2; -- error, must have order by
|
||||
explain (costs off)
|
||||
select f1, sum(f1) over (partition by f1 order by f2
|
||||
groups between 1 preceding and 1 following)
|
||||
from t1 where f1 = f2;
|
||||
select f1, sum(f1) over (partition by f1 order by f2
|
||||
groups between 1 preceding and 1 following)
|
||||
from t1 where f1 = f2;
|
||||
select f1, sum(f1) over (partition by f1, f1 order by f2
|
||||
groups between 2 preceding and 1 preceding)
|
||||
from t1 where f1 = f2;
|
||||
select f1, sum(f1) over (partition by f1, f2 order by f2
|
||||
groups between 1 following and 2 following)
|
||||
from t1 where f1 = f2;
|
||||
|
||||
-- ordering by a non-integer constant is allowed
|
||||
SELECT rank() OVER (ORDER BY length('abc'));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user