diff --git a/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java b/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java index fb38f184925..2d214c8b5f4 100644 --- a/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java +++ b/api/src/test/java/org/opensearch/sql/api/UnifiedQueryPlannerSqlV2Test.java @@ -424,6 +424,33 @@ GROUP BY department HAVING MAX(age) > 30 AND MIN(age) < 50 """); } + @Test + public void testCountDistinctWindowWithOrderBy() { + // No frame printed: RANGE .. CURRENT ROW is Calcite's default for ORDER BY. + givenQuery( + """ + SELECT department, COUNT(DISTINCT name) OVER(ORDER BY department) FROM catalog.employees + """) + .assertPlan( + """ + LogicalProject(department=[$3], COUNT(DISTINCT name) OVER(ORDER BY department)=[COUNT(DISTINCT $1) OVER (ORDER BY $3 NULLS FIRST)]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + + @Test + public void testSumWindowWithPartitionAndOrderBy() { + givenQuery( + """ + SELECT name, SUM(age) OVER(PARTITION BY department ORDER BY age) FROM catalog.employees + """) + .assertPlan( + """ + LogicalProject(name=[$1], SUM(age) OVER(PARTITION BY department ORDER BY age)=[SUM($2) OVER (PARTITION BY $3 ORDER BY $2 NULLS FIRST)]) + LogicalTableScan(table=[[catalog, employees]]) + """); + } + @Test public void testWindowOrderByDefaultsNullsFirst() { // Window function ORDER BY without explicit NULLS FIRST/LAST defaults to NULLS FIRST, diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/WindowBound.java b/core/src/main/java/org/opensearch/sql/ast/expression/WindowBound.java index d4241165cbc..62c6559a225 100644 --- a/core/src/main/java/org/opensearch/sql/ast/expression/WindowBound.java +++ b/core/src/main/java/org/opensearch/sql/ast/expression/WindowBound.java @@ -5,6 +5,7 @@ package org.opensearch.sql.ast.expression; +import lombok.EqualsAndHashCode; import lombok.Getter; public abstract class WindowBound { @@ -25,6 +26,7 @@ public boolean isPreceding() { } } + @EqualsAndHashCode(callSuper = false) public static class CurrentRowWindowBound extends WindowBound { CurrentRowWindowBound() {} diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/WindowFrame.java b/core/src/main/java/org/opensearch/sql/ast/expression/WindowFrame.java index 7ea1a072f8a..9e2fbf246d2 100644 --- a/core/src/main/java/org/opensearch/sql/ast/expression/WindowFrame.java +++ b/core/src/main/java/org/opensearch/sql/ast/expression/WindowFrame.java @@ -40,6 +40,13 @@ public static WindowFrame toCurrentRow() { AstDSL.stringLiteral("CURRENT ROW")); } + public static WindowFrame rangeToCurrentRow() { + return WindowFrame.of( + FrameType.RANGE, + AstDSL.stringLiteral("UNBOUNDED PRECEDING"), + AstDSL.stringLiteral("CURRENT ROW")); + } + public static WindowFrame of(FrameType type, String lower, String upper) { return WindowFrame.of(type, AstDSL.stringLiteral(lower), AstDSL.stringLiteral(upper)); } diff --git a/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java b/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java index 273076af40f..33cfb1a56ca 100644 --- a/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java +++ b/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java @@ -222,7 +222,14 @@ public UnresolvedExpression visitWindowFunctionClause(WindowFunctionClauseContex .map(item -> ImmutablePair.of(createSortOption(item), visit(item.expression()))) .collect(Collectors.toList()); } - return new WindowFunction(visit(ctx.function), partitionByList, sortList); + UnresolvedExpression function = visit(ctx.function); + WindowFunction windowFunction = new WindowFunction(function, partitionByList, sortList); + + // Aggregate window with ORDER BY defaults to a running RANGE frame (ranking ignores it). + if (function instanceof AggregateFunction && !sortList.isEmpty()) { + windowFunction.setWindowFrame(WindowFrame.rangeToCurrentRow()); + } + return windowFunction; } @Override diff --git a/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java b/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java index e89f2af9b01..cb981f6f45f 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java @@ -44,6 +44,8 @@ import org.opensearch.sql.ast.expression.DataType; import org.opensearch.sql.ast.expression.Literal; import org.opensearch.sql.ast.expression.RelevanceFieldList; +import org.opensearch.sql.ast.expression.WindowFrame; +import org.opensearch.sql.ast.expression.WindowFunction; import org.opensearch.sql.ast.tree.Sort.SortOption; import org.opensearch.sql.common.antlr.CaseInsensitiveCharStream; import org.opensearch.sql.common.antlr.SyntaxAnalysisErrorListener; @@ -308,12 +310,23 @@ public void canBuildWindowFunctionWithoutOrderBy() { @Test public void canBuildAggregateWindowFunction() { + WindowFunction expected = + new WindowFunction( + aggregate("AVG", qualifiedName("age")), + ImmutableList.of(qualifiedName("state")), + ImmutableList.of(ImmutablePair.of(new SortOption(null, null), qualifiedName("age")))); + expected.setWindowFrame(WindowFrame.rangeToCurrentRow()); + assertEquals(expected, buildExprAst("AVG(age) OVER (PARTITION BY state ORDER BY age)")); + } + + @Test + public void canBuildAggregateWindowFunctionWithoutOrderBy() { assertEquals( window( aggregate("AVG", qualifiedName("age")), ImmutableList.of(qualifiedName("state")), - ImmutableList.of(ImmutablePair.of(new SortOption(null, null), qualifiedName("age")))), - buildExprAst("AVG(age) OVER (PARTITION BY state ORDER BY age)")); + ImmutableList.of()), + buildExprAst("AVG(age) OVER (PARTITION BY state)")); } @Test