diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/SumSqlAggregator.java b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/SumSqlAggregator.java index e56f1e8463a..29790ab7154 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/SumSqlAggregator.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/SumSqlAggregator.java @@ -19,9 +19,10 @@ package org.apache.druid.sql.calcite.aggregation.builtin; +import org.apache.calcite.linq4j.Nullness; import org.apache.calcite.rel.core.AggregateCall; import org.apache.calcite.sql.SqlAggFunction; -import org.apache.calcite.sql.fun.SqlStdOperatorTable; +import org.apache.calcite.sql.fun.SqlSumAggFunction; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.query.aggregation.AggregatorFactory; import org.apache.druid.query.aggregation.DoubleSumAggregatorFactory; @@ -35,11 +36,17 @@ import javax.annotation.Nullable; public class SumSqlAggregator extends SimpleSqlAggregator { + /** + * We use this custom aggregation function instead of builtin SqlStdOperatorTable.SUM + * to avoid transformation to COUNT+SUM0. See CALCITE-6020 for more details. + * It can be handled differently after CALCITE-6020 is addressed. + */ + private static final SqlAggFunction DRUID_SUM = new SqlSumAggFunction(Nullness.castNonNull(null)) {}; @Override public SqlAggFunction calciteFunction() { - return SqlStdOperatorTable.SUM; + return DRUID_SUM; } @Override diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java index aa8a5d74d86..7b42596d81a 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java @@ -28,11 +28,15 @@ import org.apache.druid.common.config.NullHandling; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.RE; +import org.apache.druid.query.Query; +import org.apache.druid.query.QueryContexts; import org.apache.druid.query.operator.OperatorFactory; import org.apache.druid.query.operator.WindowOperatorQuery; import org.apache.druid.segment.column.ColumnType; import org.apache.druid.segment.column.RowSignature; +import org.apache.druid.sql.calcite.CalciteWindowQueryTest.WindowQueryTestInputClass.TestType; import org.apache.druid.sql.calcite.planner.PlannerContext; +import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -47,6 +51,10 @@ import java.util.Locale; import java.util.Objects; import java.util.function.Function; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeThat; + /** * These tests are file-based, look in resources -> calcite/tests/window for the set of test specifications. */ @@ -116,15 +124,14 @@ public class CalciteWindowQueryTest extends BaseCalciteQueryTest } }; - if ("failingTest".equals(input.type)) { - return; - } + assumeThat(input.type, Matchers.not(TestType.failingTest)); - if ("operatorValidation".equals(input.type)) { + if (input.type == TestType.operatorValidation) { testBuilder() .skipVectorize(true) .sql(input.sql) - .queryContext(ImmutableMap.of(PlannerContext.CTX_ENABLE_WINDOW_FNS, true)) + .queryContext(ImmutableMap.of(PlannerContext.CTX_ENABLE_WINDOW_FNS, true, + QueryContexts.ENABLE_DEBUG, true)) .addCustomVerification(QueryVerification.ofResults(results -> { if (results.exception != null) { throw new RE(results.exception, "Failed to execute because of exception."); @@ -140,7 +147,7 @@ public class CalciteWindowQueryTest extends BaseCalciteQueryTest // and aggregations=[CountAggregatorFactory{name='w0'}, LongSumAggregatorFactory{fieldName='a0', expression='null', name='w1'}]}}]} // These 2 tests are marked as failingTests to unblock testing at this moment - final WindowOperatorQuery query = (WindowOperatorQuery) results.recordedQueries.get(0); + final WindowOperatorQuery query = getWindowOperatorQuery(results.recordedQueries); for (int i = 0; i < input.expectedOperators.size(); ++i) { final OperatorFactory expectedOperator = input.expectedOperators.get(i); final OperatorFactory actualOperator = query.getOperators().get(i); @@ -199,6 +206,14 @@ public class CalciteWindowQueryTest extends BaseCalciteQueryTest } } + private WindowOperatorQuery getWindowOperatorQuery(List> queries) + { + assertEquals(1, queries.size()); + Object query = queries.get(0); + assertTrue(query instanceof WindowOperatorQuery); + return (WindowOperatorQuery) query; + } + private void maybeDumpActualResults( Function toStrFn, List results ) @@ -212,8 +227,13 @@ public class CalciteWindowQueryTest extends BaseCalciteQueryTest public static class WindowQueryTestInputClass { + enum TestType + { + failingTest, + operatorValidation + } @JsonProperty - public String type; + public TestType type; @JsonProperty public String sql; diff --git a/sql/src/test/resources/calcite/tests/window/simpleSum.sqlTest b/sql/src/test/resources/calcite/tests/window/simpleSum.sqlTest index 09adf1b8ba6..d4affc6ec56 100644 --- a/sql/src/test/resources/calcite/tests/window/simpleSum.sqlTest +++ b/sql/src/test/resources/calcite/tests/window/simpleSum.sqlTest @@ -1,4 +1,4 @@ -type: "failingTest" +type: "operatorValidation" sql: | SELECT diff --git a/sql/src/test/resources/calcite/tests/window/wikipediaAggregationsMultipleOrdering.sqlTest b/sql/src/test/resources/calcite/tests/window/wikipediaAggregationsMultipleOrdering.sqlTest index b2d4b220326..b2b5aa49c61 100644 --- a/sql/src/test/resources/calcite/tests/window/wikipediaAggregationsMultipleOrdering.sqlTest +++ b/sql/src/test/resources/calcite/tests/window/wikipediaAggregationsMultipleOrdering.sqlTest @@ -1,4 +1,4 @@ -type: "failingTest" +type: "operatorValidation" sql: | SELECT diff --git a/sql/src/test/resources/calcite/tests/window/wikipediaCumulativeOrdered.sqlTest b/sql/src/test/resources/calcite/tests/window/wikipediaCumulativeOrdered.sqlTest index ab52096e8e1..7777120e2ba 100644 --- a/sql/src/test/resources/calcite/tests/window/wikipediaCumulativeOrdered.sqlTest +++ b/sql/src/test/resources/calcite/tests/window/wikipediaCumulativeOrdered.sqlTest @@ -1,4 +1,4 @@ -type: "failingTest" +type: "operatorValidation" sql: | SELECT diff --git a/sql/src/test/resources/calcite/tests/window/wikipediaFramedAggregations.sqlTest b/sql/src/test/resources/calcite/tests/window/wikipediaFramedAggregations.sqlTest index da5e1546bae..e1b5dcd1833 100644 --- a/sql/src/test/resources/calcite/tests/window/wikipediaFramedAggregations.sqlTest +++ b/sql/src/test/resources/calcite/tests/window/wikipediaFramedAggregations.sqlTest @@ -1,4 +1,4 @@ -type: "failingTest" +type: "operatorValidation" sql: | SELECT diff --git a/sql/src/test/resources/calcite/tests/window/wikipediaSimplePartition.sqlTest b/sql/src/test/resources/calcite/tests/window/wikipediaSimplePartition.sqlTest index 542fe203f8d..8284d3c2105 100644 --- a/sql/src/test/resources/calcite/tests/window/wikipediaSimplePartition.sqlTest +++ b/sql/src/test/resources/calcite/tests/window/wikipediaSimplePartition.sqlTest @@ -1,4 +1,4 @@ -type: "failingTest" +type: "operatorValidation" sql: | SELECT diff --git a/sql/src/test/resources/calcite/tests/window/wikipediaSimplePartitionInitialSort.sqlTest b/sql/src/test/resources/calcite/tests/window/wikipediaSimplePartitionInitialSort.sqlTest index 1b4a4de3eb2..282647c2f77 100644 --- a/sql/src/test/resources/calcite/tests/window/wikipediaSimplePartitionInitialSort.sqlTest +++ b/sql/src/test/resources/calcite/tests/window/wikipediaSimplePartitionInitialSort.sqlTest @@ -1,4 +1,4 @@ -type: "failingTest" +type: "operatorValidation" # Like wikipediaSimplePartition, but requires re-sorting the input data because the order of the GROUP BY # does not match the required order for window partitioning. ("t" and "countryIsoCode" are flipped.)