From af9e3ed43aa6e0a90bc32e5541433612eb627bfc Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Fri, 15 Sep 2023 12:54:37 +0000 Subject: [PATCH 01/10] Enable already passing tests in DecoupledPlanningCalciteQueryTest --- .../DecoupledPlanningCalciteQueryTest.java | 58 ------------------- 1 file changed, 58 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java index 7d7559f85279..29619b8f2cbb 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java @@ -25,7 +25,6 @@ import org.apache.druid.sql.calcite.planner.PlannerConfig; import org.apache.druid.sql.calcite.util.SqlTestFramework; import org.junit.Ignore; -import org.junit.Test; public class DecoupledPlanningCalciteQueryTest extends CalciteQueryTest { @@ -254,42 +253,6 @@ public void testRequireTimeConditionSemiJoinNegative() } - @Override - @Ignore - public void testSubqueryTypeMismatchWithLiterals() - { - - } - - @Override - @Ignore - public void testTimeseriesQueryWithEmptyInlineDatasourceAndGranularity() - { - - } - - @Override - @Ignore - public void testGroupBySortPushDown() - { - - } - - @Override - @Ignore - public void testGroupingWithNullInFilter() - { - - } - - @Override - @Ignore - @Test - public void testStringAggExpressionNonConstantSeparator() - { - - } - @Override @Ignore public void testOrderByAlongWithInternalScanQuery() @@ -297,13 +260,6 @@ public void testOrderByAlongWithInternalScanQuery() } - @Override - @Ignore - public void testSortProjectAfterNestedGroupBy() - { - - } - @Override @Ignore public void testOrderByAlongWithInternalScanQueryNoDistinct() @@ -311,20 +267,6 @@ public void testOrderByAlongWithInternalScanQueryNoDistinct() } - @Override - @Ignore - public void testNestedGroupBy() - { - - } - - @Override - @Ignore - public void testQueryWithSelectProjectAndIdentityProjectDoesNotRename() - { - - } - @Override @Ignore public void testFilterOnCurrentTimestampWithIntervalArithmetic() From b22bd262c506a85821f0acd6382763e02b1235d7 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Fri, 15 Sep 2023 18:17:25 +0000 Subject: [PATCH 02/10] empty From 1644321e0ab1381ddf0f67e0c79601cba40c27cb Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 12:50:48 +0000 Subject: [PATCH 03/10] add transposerule; fix prefix --- .../druid/sql/calcite/planner/CalciteRulesManager.java | 1 + .../java/org/apache/druid/sql/calcite/rel/DruidQuery.java | 2 +- .../sql/calcite/DecoupledPlanningCalciteQueryTest.java | 6 ------ 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java index 8d2f1103922b..9a4abf5b226a 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/CalciteRulesManager.java @@ -132,6 +132,7 @@ public class CalciteRulesManager CoreRules.FILTER_VALUES_MERGE, CoreRules.PROJECT_FILTER_VALUES_MERGE, CoreRules.PROJECT_VALUES_MERGE, + CoreRules.SORT_PROJECT_TRANSPOSE, CoreRules.AGGREGATE_VALUES ); diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java index ef19c559a8af..02708e93d343 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java @@ -650,7 +650,7 @@ private static Sorting computeSorting( projection = Projection.preAggregation(sortProject, plannerContext, rowSignature, virtualColumnRegistry); } else { - projection = Projection.postAggregation(sortProject, plannerContext, rowSignature, "s"); + projection = Projection.postAggregation(sortProject, plannerContext, rowSignature, "p"); } return Sorting.create(orderBys, offsetLimit, projection); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java index 29619b8f2cbb..ef7857cb1bcf 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java @@ -226,12 +226,6 @@ public void testGroupByTimeFloorAndDimOnGroupByTimeFloorAndDim() } - @Override - @Ignore - public void testPostAggWithTimeseries() - { - - } @Override @Ignore From 38bfec39e54ce38c33e274fff81f98cb7c11731c Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 13:00:54 +0000 Subject: [PATCH 04/10] accept a few more --- .../DecoupledPlanningCalciteQueryTest.java | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java index ef7857cb1bcf..2e6bccee73ae 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledPlanningCalciteQueryTest.java @@ -226,14 +226,6 @@ public void testGroupByTimeFloorAndDimOnGroupByTimeFloorAndDim() } - - @Override - @Ignore - public void testPostAggWithTopN() - { - - } - @Override @Ignore public void testRequireTimeConditionPositive() @@ -291,13 +283,6 @@ public void testExactCountDistinctWithGroupingAndOtherAggregators() } - @Override - @Ignore - public void testTopNWithSelectProjections() - { - - } - @Override @Ignore public void testPlanWithInFilterLessThanInSubQueryThreshold() From 6bb32b3c9d6c43ea4331cb1511935ac718add744 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 14:51:10 +0000 Subject: [PATCH 05/10] fix postagg --- .../org/apache/druid/sql/calcite/CalciteArraysQueryTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java index df4e9b62cc9e..386d139c25a5 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java @@ -2206,7 +2206,7 @@ public void testArrayAggGroupByArrayAggFromSubquery() .intervals(querySegmentSpec(Filtration.eternity())) .granularity(Granularities.ALL) .context(QUERY_CONTEXT_NO_STRINGIFY_ARRAY) - .postAggregators(new ExpressionPostAggregator("s0", "1", null, ExprMacroTable.nil())) + .postAggregators(new ExpressionPostAggregator("p0", "1", null, ExprMacroTable.nil())) .build() ), useDefault ? From 061029921433ceafc6b16f6df1776e338abe7539 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Thu, 21 Sep 2023 08:12:58 +0000 Subject: [PATCH 06/10] new-ignores --- .../java/org/apache/druid/sql/calcite/CalciteQueryTest.java | 3 --- 1 file changed, 3 deletions(-) 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 99b974aa7110..e82f12617efd 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 @@ -2733,7 +2733,6 @@ public void testGroupByWithSelectAndOrderByProjections() ); } - @DecoupledIgnore(mode = Modes.PLAN_MISMATCH) @Test public void testTopNWithSelectProjections() { @@ -11620,7 +11619,6 @@ public void testSortProjectAfterNestedGroupBy() ); } - @DecoupledIgnore(mode = Modes.CANNOT_CONVERT) @Test public void testPostAggWithTimeseries() { @@ -11664,7 +11662,6 @@ public void testPostAggWithTimeseries() ); } - @DecoupledIgnore(mode = Modes.PLAN_MISMATCH) @Test public void testPostAggWithTopN() { From 10743ec4eacb613b3155b2efff9d6e50eb29f948 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Tue, 26 Sep 2023 09:42:21 +0000 Subject: [PATCH 07/10] col renames --- .../datasketches/hll/sql/HllSketchSqlAggregatorTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregatorTest.java index 349f1a57d1c0..27543e585c85 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregatorTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregatorTest.java @@ -824,8 +824,8 @@ public void testtHllSketchPostAggsPostSort() .postAggregators( ImmutableList.of( new HllSketchToEstimatePostAggregator("p1", new FieldAccessPostAggregator("p0", "a0"), false), - new HllSketchToEstimatePostAggregator("s1", new FieldAccessPostAggregator("s0", "a0"), false), - new HllSketchToStringPostAggregator("s3", new FieldAccessPostAggregator("s2", "a0")) + new HllSketchToEstimatePostAggregator("_p1", new FieldAccessPostAggregator("_p0", "a0"), false), + new HllSketchToStringPostAggregator("_p3", new FieldAccessPostAggregator("_p2", "a0")) ) ) .context(QUERY_CONTEXT_DEFAULT) From 4604ec95d40b4fba1b294398cea41e876aa45cbb Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Tue, 26 Sep 2023 09:45:04 +0000 Subject: [PATCH 08/10] fix ref --- .../theta/sql/ThetaSketchSqlAggregatorTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSqlAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSqlAggregatorTest.java index 3946ce558b19..2bff1b5f3296 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSqlAggregatorTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSqlAggregatorTest.java @@ -747,8 +747,8 @@ public void testThetaSketchPostAggsPostSort() null ), new SketchEstimatePostAggregator( - "s1", - new FieldAccessPostAggregator("s0", "a0"), + "_p1", + new FieldAccessPostAggregator("_p0", "a0"), null ) ) @@ -803,8 +803,8 @@ public void testThetaSketchPostAggsPostSortFinalizeOuterSketches() null ), new SketchEstimatePostAggregator( - "s1", - new FieldAccessPostAggregator("s0", "a0"), + "_p1", + new FieldAccessPostAggregator("_p0", "a0"), null ) ) From cc63c99318eecd80b500fd6ebba156d4b7fc893e Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Tue, 26 Sep 2023 09:46:10 +0000 Subject: [PATCH 09/10] fix ref --- .../quantiles/sql/DoublesSketchSqlAggregatorTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java index c2d81cede8ce..e294a099af5e 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java @@ -692,13 +692,13 @@ public void testDoublesSketchPostAggsPostSort() 0.5 ), new DoublesSketchToQuantilePostAggregator( - "s1", - new FieldAccessPostAggregator("s0", "a0:agg"), + "_p1", + new FieldAccessPostAggregator("_p0", "a0:agg"), 0.5 ), new DoublesSketchToQuantilePostAggregator( - "s3", - new FieldAccessPostAggregator("s2", "a0:agg"), + "_p3", + new FieldAccessPostAggregator("_p2", "a0:agg"), 0.9800000190734863 ) ) From ba65bd9dffb40e029c6ae2167ad4dd3bf83b5145 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Wed, 27 Sep 2023 10:51:21 +0000 Subject: [PATCH 10/10] remove change of alias prefix for alternate postagg call --- .../datasketches/hll/sql/HllSketchSqlAggregatorTest.java | 4 ++-- .../quantiles/sql/DoublesSketchSqlAggregatorTest.java | 8 ++++---- .../theta/sql/ThetaSketchSqlAggregatorTest.java | 8 ++++---- .../java/org/apache/druid/sql/calcite/rel/DruidQuery.java | 2 +- .../apache/druid/sql/calcite/CalciteArraysQueryTest.java | 2 +- .../org/apache/druid/sql/calcite/CalciteQueryTest.java | 6 +++--- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregatorTest.java index 27543e585c85..349f1a57d1c0 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregatorTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSqlAggregatorTest.java @@ -824,8 +824,8 @@ public void testtHllSketchPostAggsPostSort() .postAggregators( ImmutableList.of( new HllSketchToEstimatePostAggregator("p1", new FieldAccessPostAggregator("p0", "a0"), false), - new HllSketchToEstimatePostAggregator("_p1", new FieldAccessPostAggregator("_p0", "a0"), false), - new HllSketchToStringPostAggregator("_p3", new FieldAccessPostAggregator("_p2", "a0")) + new HllSketchToEstimatePostAggregator("s1", new FieldAccessPostAggregator("s0", "a0"), false), + new HllSketchToStringPostAggregator("s3", new FieldAccessPostAggregator("s2", "a0")) ) ) .context(QUERY_CONTEXT_DEFAULT) diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java index e294a099af5e..c2d81cede8ce 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/quantiles/sql/DoublesSketchSqlAggregatorTest.java @@ -692,13 +692,13 @@ public void testDoublesSketchPostAggsPostSort() 0.5 ), new DoublesSketchToQuantilePostAggregator( - "_p1", - new FieldAccessPostAggregator("_p0", "a0:agg"), + "s1", + new FieldAccessPostAggregator("s0", "a0:agg"), 0.5 ), new DoublesSketchToQuantilePostAggregator( - "_p3", - new FieldAccessPostAggregator("_p2", "a0:agg"), + "s3", + new FieldAccessPostAggregator("s2", "a0:agg"), 0.9800000190734863 ) ) diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSqlAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSqlAggregatorTest.java index 2bff1b5f3296..3946ce558b19 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSqlAggregatorTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSqlAggregatorTest.java @@ -747,8 +747,8 @@ public void testThetaSketchPostAggsPostSort() null ), new SketchEstimatePostAggregator( - "_p1", - new FieldAccessPostAggregator("_p0", "a0"), + "s1", + new FieldAccessPostAggregator("s0", "a0"), null ) ) @@ -803,8 +803,8 @@ public void testThetaSketchPostAggsPostSortFinalizeOuterSketches() null ), new SketchEstimatePostAggregator( - "_p1", - new FieldAccessPostAggregator("_p0", "a0"), + "s1", + new FieldAccessPostAggregator("s0", "a0"), null ) ) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java index 02708e93d343..ef19c559a8af 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java @@ -650,7 +650,7 @@ private static Sorting computeSorting( projection = Projection.preAggregation(sortProject, plannerContext, rowSignature, virtualColumnRegistry); } else { - projection = Projection.postAggregation(sortProject, plannerContext, rowSignature, "p"); + projection = Projection.postAggregation(sortProject, plannerContext, rowSignature, "s"); } return Sorting.create(orderBys, offsetLimit, projection); diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java index 386d139c25a5..df4e9b62cc9e 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java @@ -2206,7 +2206,7 @@ public void testArrayAggGroupByArrayAggFromSubquery() .intervals(querySegmentSpec(Filtration.eternity())) .granularity(Granularities.ALL) .context(QUERY_CONTEXT_NO_STRINGIFY_ARRAY) - .postAggregators(new ExpressionPostAggregator("p0", "1", null, ExprMacroTable.nil())) + .postAggregators(new ExpressionPostAggregator("s0", "1", null, ExprMacroTable.nil())) .build() ), useDefault ? 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 e82f12617efd..d8505b521029 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 @@ -2749,7 +2749,7 @@ public void testTopNWithSelectProjections() .intervals(querySegmentSpec(Filtration.eternity())) .granularity(Granularities.ALL) .dimension(new DefaultDimensionSpec("dim1", "d0")) - .postAggregators(expressionPostAgg("p0", "substring(\"d0\", 1, -1)")) + .postAggregators(expressionPostAgg("s0", "substring(\"d0\", 1, -1)")) .metric(new DimensionTopNMetricSpec(null, StringComparators.LEXICOGRAPHIC)) .threshold(10) .context(QUERY_CONTEXT_DEFAULT) @@ -11649,7 +11649,7 @@ public void testPostAggWithTimeseries() ) ) .postAggregators( - expressionPostAgg("p0", "(\"a0\" + \"a1\")") + expressionPostAgg("s0", "(\"a0\" + \"a1\")") ) .descending(true) .context(getTimeseriesContextWithFloorTime(TIMESERIES_CONTEXT_BY_GRAN, "d0")) @@ -11710,7 +11710,7 @@ public void testPostAggWithTopN() new FieldAccessPostAggregator(null, "a0:count") ) ), - expressionPostAgg("p0", "(\"a1\" + \"a2\")") + expressionPostAgg("s0", "(\"a1\" + \"a2\")") ) .metric(new DimensionTopNMetricSpec(null, StringComparators.NUMERIC)) .threshold(5)