Skip to content

Commit

Permalink
[CALCITE-5863] Calcite rejects valid query with multiple ORDER BY col…
Browse files Browse the repository at this point in the history
…umns and constant RANGE bounds in window functions
  • Loading branch information
itiels authored and LakeShen committed Nov 1, 2023
1 parent 2ddc605 commit 919f50d
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 2 deletions.
14 changes: 12 additions & 2 deletions core/src/main/java/org/apache/calcite/sql/SqlWindow.java
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,9 @@ public boolean isAllowPartial() {
// SQL03 7.10 Rule 11a
if (orderList.size() > 0) {
// if order by is a compound list then range not allowed
if (orderList.size() > 1 && !isRows()) {
if (orderList.size() > 1
&& !isRows()
&& !onlySymbolBounds(lowerBound, upperBound)) {
throw validator.newValidationError(isRows,
RESOURCE.compoundOrderByProhibitsRange());
}
Expand All @@ -625,7 +627,9 @@ public boolean isAllowPartial() {
// requires an ORDER BY clause if frame is logical(RANGE)
// We relax this requirement if the table appears to be
// sorted already
if (!isRows() && !SqlValidatorUtil.containsMonotonic(scope)) {
if (!onlySymbolBounds(lowerBound, upperBound)
&& !isRows()
&& !SqlValidatorUtil.containsMonotonic(scope)) {
throw validator.newValidationError(this,
RESOURCE.overMissingOrderBy());
}
Expand Down Expand Up @@ -660,6 +664,12 @@ public boolean isAllowPartial() {
}
}

private boolean onlySymbolBounds(@Nullable SqlNode lowerBound, @Nullable SqlNode upperBound) {
return lowerBound != null && upperBound != null
&& (isCurrentRow(lowerBound) || isUnboundedPreceding(lowerBound))
&& (isCurrentRow(upperBound) || isUnboundedFollowing(upperBound));
}

private static void validateFrameBoundary(
@Nullable SqlNode bound,
boolean isRows,
Expand Down
16 changes: 16 additions & 0 deletions core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4016,6 +4016,22 @@ RelOptFixture checkDynamicFunctions(boolean treatDynamicCallsAsConstant) {
.check();
}

@Test void testReduceConstantsWithMultipleOrderByWindow() {
final String sql = "select col1, col2\n"
+ "from (\n"
+ " select empno,\n"
+ " sum(100) over (order by deptno, empno range between current row and unbounded following) as col1,\n"
+ " sum(100) over (partition by sal, deptno order by deptno, empno range between unbounded preceding and unbounded following) as col2\n"
+ " from emp where sal = 5000)";

sql(sql)
.withRule(CoreRules.PROJECT_TO_LOGICAL_PROJECT_AND_WINDOW,
CoreRules.PROJECT_MERGE,
CoreRules.PROJECT_WINDOW_TRANSPOSE,
CoreRules.WINDOW_REDUCE_EXPRESSIONS)
.check();
}

@Test void testEmptyFilterProjectUnion() {
// Plan should be same as for
// select * from (values (30, 3)) as t(x, y)");
Expand Down
34 changes: 34 additions & 0 deletions core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3251,6 +3251,40 @@ void testWinPartClause() {
+ "rows 2 preceding )").ok();
winExp2("sum(sal) over (order by deptno range 2.0 preceding)").ok();

// compound order by with literal bounds
winExp2("sum(sal) over "
+ "(order by sal,deptno "
+ "range between current row and unbounded following)").ok();
winExp2("sum(sal) over "
+ "(order by sal,deptno "
+ "range between current row and current row)").ok();
winExp2("sum(sal) over "
+ "(order by sal,deptno "
+ "range between unbounded preceding and current row)").ok();
winExp2("sum(sal) over "
+ "(order by sal,deptno "
+ "range between unbounded preceding and unbounded following)").ok();

// Range without order by with only literal bounds
winExp2("sum(sal) over "
+ "(partition by sal,deptno "
+ "range between unbounded preceding and unbounded following)").ok();

// RANGE with non-difference type order by and literal bounds
winExp2("sum(sal) over "
+ "(order by ename "
+ "range between unbounded preceding and current row)").ok();

// Range without order by with only preceding / following should fail
winExp2("sum(sal) over "
+ "^(partition by sal "
+ "range between 3 preceding and unbounded following)^")
.fails("Window specification must contain an ORDER BY clause");
winExp2("sum(sal) over "
+ "^(partition by sal "
+ "range between current row and 3 following)^")
.fails("Window specification must contain an ORDER BY clause");

// Failure mode tests
winExp2("sum(sal) over (order by deptno "
+ "rows between ^UNBOUNDED FOLLOWING^ and unbounded preceding)")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12387,6 +12387,32 @@ LogicalProject($0=[$2], $1=[$3], $2=[$4])
LogicalProject(COL1=[SUM(100) OVER (PARTITION BY $7, $5 ORDER BY $5)], COL2=[SUM(100) OVER (PARTITION BY $5 ORDER BY $7)], COL3=[SUM($5) OVER (PARTITION BY $7 ORDER BY $5)])
LogicalFilter(condition=[=($5, 5000)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
</TestCase>
<TestCase name="testReduceConstantsWithMultipleOrderByWindow">
<Resource name="sql">
<![CDATA[select col1, col2
from (
select empno,
sum(100) over (order by deptno, empno range between current row and unbounded following) as col1,
sum(100) over (partition by sal, deptno order by deptno, empno range between unbounded preceding and unbounded following) as col2
from emp where sal = 5000)]]>
</Resource>
<Resource name="planBefore">
<![CDATA[
LogicalProject(COL1=[SUM(100) OVER (ORDER BY $7, $0 RANGE BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING)], COL2=[SUM(100) OVER (PARTITION BY $5, $7 ORDER BY $7, $0 RANGE BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING)])
LogicalFilter(condition=[=($5, 5000)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
<Resource name="planAfter">
<![CDATA[
LogicalProject($0=[$3], $1=[$4])
LogicalWindow(window#0=[window(order by [2, 0] range between CURRENT ROW and UNBOUNDED FOLLOWING aggs [SUM($3)])], window#1=[window(partition {2} order by [2, 0] range between UNBOUNDED PRECEDING and UNBOUNDED FOLLOWING aggs [SUM($3)])])
LogicalProject(EMPNO=[$0], SAL=[$5], DEPTNO=[$7])
LogicalFilter(condition=[=($5, 5000)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
</TestCase>
Expand Down
18 changes: 18 additions & 0 deletions core/src/test/resources/sql/misc.iq
Original file line number Diff line number Diff line change
Expand Up @@ -2618,4 +2618,22 @@ SELECT

!ok

!use post

# [CALCITE-5863] Incorrect validation with range and multiple order by columns
SELECT sum("salary")
OVER (ORDER BY "salary", "deptno" RANGE BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING)
FROM "hr"."emps";
+---------+
| EXPR$0 |
+---------+
| 11500.0 |
| 21500.0 |
| 29500.0 |
| 36500.0 |
+---------+
(4 rows)

!ok

# End misc.iq

0 comments on commit 919f50d

Please sign in to comment.