From 9b76d13ff8f481b815b195aacb1612b7a789d868 Mon Sep 17 00:00:00 2001 From: Sree Charan Manamala Date: Fri, 26 Jul 2024 14:48:35 +0530 Subject: [PATCH] Check for Aggregation inside a window clause when syntax used as - WINDOW W AS DEF (#16801) --- .../calcite/planner/DruidSqlValidator.java | 16 ++++++++++++++++ .../druid/sql/calcite/CalciteQueryTest.java | 19 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java index e00a2915a2e8..16d3541e96c6 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java @@ -796,6 +796,22 @@ public void validateCall(SqlCall call, SqlValidatorScope scope) super.validateCall(call, scope); } + @Override + protected void validateWindowClause(SqlSelect select) + { + SqlNodeList windows = select.getWindowList(); + for (SqlNode sqlNode : windows) { + if (SqlUtil.containsAgg(sqlNode)) { + throw buildCalciteContextException( + "Aggregation inside window is currently not supported with syntax WINDOW W AS . " + + "Try providing window definition directly without alias", + sqlNode + ); + } + } + super.validateWindowClause(select); + } + @Override protected SqlNode performUnconditionalRewrites(SqlNode node, final boolean underFrom) { diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 5ceba91eb37c..84b437c20e08 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; +import org.apache.calcite.rel.RelNode; import org.apache.calcite.runtime.CalciteContextException; import org.apache.druid.common.config.NullHandling; import org.apache.druid.error.DruidException; @@ -149,6 +150,7 @@ import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeFalse; @@ -15592,6 +15594,23 @@ public void testDistinctNotSupportedWithWindow() assertThat(e, invalidSqlContains("DISTINCT is not supported for window functions")); } + @Test + public void testUnSupportedAggInSelectWindow() + { + assertEquals( + "1.37.0", + RelNode.class.getPackage().getImplementationVersion(), + "Calcite version changed; check if CALCITE-6500 is fixed and update:\n * method DruidSqlValidator#validateWindowClause" + ); + + DruidException e = assertThrows(DruidException.class, () -> testBuilder() + .queryContext(ImmutableMap.of(PlannerContext.CTX_ENABLE_WINDOW_FNS, true)) + .sql("SELECT dim1, ROW_NUMBER() OVER W from druid.foo WINDOW W as (ORDER BY max(length(dim1)))") + .run()); + + assertThat(e, invalidSqlContains("not supported with syntax WINDOW W AS ")); + } + @Test public void testInGroupByLimitOutGroupByOrderBy() {