Skip to content

Commit 9999d08

Browse files
committed
fix(calcite): Default SQL aggregate window frame to running window
Aggregate window functions with ORDER BY but no explicit frame defaulted to the whole partition, so COUNT(DISTINCT x) OVER(ORDER BY y) returned the total for every row instead of a running value. Default SQL aggregates to RANGE UNBOUNDED PRECEDING .. CURRENT ROW, matching the SQL standard and the non-Calcite engine's peer semantics (ranking functions ignore frames). PPL windows carry no ORDER BY, so are unaffected. Signed-off-by: Chen Dai <daichen@amazon.com>
1 parent cf14aba commit 9999d08

5 files changed

Lines changed: 59 additions & 3 deletions

File tree

api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,33 @@ GROUP BY department HAVING MAX(age) > 30 AND MIN(age) < 50
424424
""");
425425
}
426426

427+
@Test
428+
public void testCountDistinctWindowWithOrderBy() {
429+
// No frame printed: RANGE .. CURRENT ROW is Calcite's default for ORDER BY.
430+
givenQuery(
431+
"""
432+
SELECT department, COUNT(DISTINCT name) OVER(ORDER BY department) FROM catalog.employees
433+
""")
434+
.assertPlan(
435+
"""
436+
LogicalProject(department=[$3], COUNT(DISTINCT name) OVER(ORDER BY department)=[COUNT(DISTINCT $1) OVER (ORDER BY $3 NULLS FIRST)])
437+
LogicalTableScan(table=[[catalog, employees]])
438+
""");
439+
}
440+
441+
@Test
442+
public void testSumWindowWithPartitionAndOrderBy() {
443+
givenQuery(
444+
"""
445+
SELECT name, SUM(age) OVER(PARTITION BY department ORDER BY age) FROM catalog.employees
446+
""")
447+
.assertPlan(
448+
"""
449+
LogicalProject(name=[$1], SUM(age) OVER(PARTITION BY department ORDER BY age)=[SUM($2) OVER (PARTITION BY $3 ORDER BY $2 NULLS FIRST)])
450+
LogicalTableScan(table=[[catalog, employees]])
451+
""");
452+
}
453+
427454
@Test
428455
public void testWindowOrderByDefaultsNullsFirst() {
429456
// Window function ORDER BY without explicit NULLS FIRST/LAST defaults to NULLS FIRST,

core/src/main/java/org/opensearch/sql/ast/expression/WindowBound.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
package org.opensearch.sql.ast.expression;
77

8+
import lombok.EqualsAndHashCode;
89
import lombok.Getter;
910

1011
public abstract class WindowBound {
@@ -25,6 +26,7 @@ public boolean isPreceding() {
2526
}
2627
}
2728

29+
@EqualsAndHashCode(callSuper = false)
2830
public static class CurrentRowWindowBound extends WindowBound {
2931
CurrentRowWindowBound() {}
3032

core/src/main/java/org/opensearch/sql/ast/expression/WindowFrame.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ public static WindowFrame toCurrentRow() {
4040
AstDSL.stringLiteral("CURRENT ROW"));
4141
}
4242

43+
public static WindowFrame rangeToCurrentRow() {
44+
return WindowFrame.of(
45+
FrameType.RANGE,
46+
AstDSL.stringLiteral("UNBOUNDED PRECEDING"),
47+
AstDSL.stringLiteral("CURRENT ROW"));
48+
}
49+
4350
public static WindowFrame of(FrameType type, String lower, String upper) {
4451
return WindowFrame.of(type, AstDSL.stringLiteral(lower), AstDSL.stringLiteral(upper));
4552
}

sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,14 @@ public UnresolvedExpression visitWindowFunctionClause(WindowFunctionClauseContex
222222
.map(item -> ImmutablePair.of(createSortOption(item), visit(item.expression())))
223223
.collect(Collectors.toList());
224224
}
225-
return new WindowFunction(visit(ctx.function), partitionByList, sortList);
225+
UnresolvedExpression function = visit(ctx.function);
226+
WindowFunction windowFunction = new WindowFunction(function, partitionByList, sortList);
227+
228+
// Aggregate window with ORDER BY defaults to a running RANGE frame (ranking ignores it).
229+
if (function instanceof AggregateFunction && !sortList.isEmpty()) {
230+
windowFunction.setWindowFrame(WindowFrame.rangeToCurrentRow());
231+
}
232+
return windowFunction;
226233
}
227234

228235
@Override

sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@
4444
import org.opensearch.sql.ast.expression.DataType;
4545
import org.opensearch.sql.ast.expression.Literal;
4646
import org.opensearch.sql.ast.expression.RelevanceFieldList;
47+
import org.opensearch.sql.ast.expression.WindowFrame;
48+
import org.opensearch.sql.ast.expression.WindowFunction;
4749
import org.opensearch.sql.ast.tree.Sort.SortOption;
4850
import org.opensearch.sql.common.antlr.CaseInsensitiveCharStream;
4951
import org.opensearch.sql.common.antlr.SyntaxAnalysisErrorListener;
@@ -308,12 +310,23 @@ public void canBuildWindowFunctionWithoutOrderBy() {
308310

309311
@Test
310312
public void canBuildAggregateWindowFunction() {
313+
WindowFunction expected =
314+
new WindowFunction(
315+
aggregate("AVG", qualifiedName("age")),
316+
ImmutableList.of(qualifiedName("state")),
317+
ImmutableList.of(ImmutablePair.of(new SortOption(null, null), qualifiedName("age"))));
318+
expected.setWindowFrame(WindowFrame.rangeToCurrentRow());
319+
assertEquals(expected, buildExprAst("AVG(age) OVER (PARTITION BY state ORDER BY age)"));
320+
}
321+
322+
@Test
323+
public void canBuildAggregateWindowFunctionWithoutOrderBy() {
311324
assertEquals(
312325
window(
313326
aggregate("AVG", qualifiedName("age")),
314327
ImmutableList.of(qualifiedName("state")),
315-
ImmutableList.of(ImmutablePair.of(new SortOption(null, null), qualifiedName("age")))),
316-
buildExprAst("AVG(age) OVER (PARTITION BY state ORDER BY age)"));
328+
ImmutableList.of()),
329+
buildExprAst("AVG(age) OVER (PARTITION BY state)"));
317330
}
318331

319332
@Test

0 commit comments

Comments
 (0)