From af9e3ed43aa6e0a90bc32e5541433612eb627bfc Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Fri, 15 Sep 2023 12:54:37 +0000 Subject: [PATCH 01/13] 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/13] 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/13] 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/13] 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 2b6385df9f7142562ec7b0e84dcf389257af88fa Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 13:53:33 +0000 Subject: [PATCH 05/13] add projectmerge;aggrproject --- .../sql/calcite/planner/CalciteRulesManager.java | 2 ++ .../apache/druid/sql/calcite/CalciteQueryTest.java | 14 +++++++------- 2 files changed, 9 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 9a4abf5b226a..bc98624c2412 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 @@ -133,6 +133,8 @@ public class CalciteRulesManager CoreRules.PROJECT_FILTER_VALUES_MERGE, CoreRules.PROJECT_VALUES_MERGE, CoreRules.SORT_PROJECT_TRANSPOSE, + CoreRules.PROJECT_MERGE, + CoreRules.AGGREGATE_PROJECT_MERGE, CoreRules.AGGREGATE_VALUES ); 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 499dd8faeb8e..064e80780a5b 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 @@ -6706,8 +6706,8 @@ public void testExactCountDistinctWithGroupingAndOtherAggregators() .setInterval(querySegmentSpec(Filtration.eternity())) .setGranularity(Granularities.ALL) .setDimensions(dimensions( - new DefaultDimensionSpec("dim2", "d0"), - new DefaultDimensionSpec("dim1", "d1") + new DefaultDimensionSpec("dim1", "d0"), + new DefaultDimensionSpec("dim2", "d1") )) .setAggregatorSpecs(aggregators(new LongSumAggregatorFactory("a0", "cnt"))) .setContext(QUERY_CONTEXT_DEFAULT) @@ -6716,12 +6716,12 @@ public void testExactCountDistinctWithGroupingAndOtherAggregators() ) .setInterval(querySegmentSpec(Filtration.eternity())) .setGranularity(Granularities.ALL) - .setDimensions(dimensions(new DefaultDimensionSpec("d0", "_d0"))) + .setDimensions(dimensions(new DefaultDimensionSpec("d1", "_d0"))) .setAggregatorSpecs(aggregators( new LongSumAggregatorFactory("_a0", "a0"), new FilteredAggregatorFactory( new CountAggregatorFactory("_a1"), - notNull("d1") + notNull("d0") ) )) .setContext(QUERY_CONTEXT_DEFAULT) @@ -7877,8 +7877,8 @@ public void testGroupBySortPushDown() .setGranularity(Granularities.ALL) .setDimensions( dimensions( - new DefaultDimensionSpec("dim2", "d0"), - new DefaultDimensionSpec("dim1", "d1") + new DefaultDimensionSpec("dim1", "d0"), + new DefaultDimensionSpec("dim2", "d1") ) ) .setAggregatorSpecs( @@ -7889,7 +7889,7 @@ public void testGroupBySortPushDown() .setLimitSpec( new DefaultLimitSpec( ImmutableList.of( - new OrderByColumnSpec("d1", OrderByColumnSpec.Direction.ASCENDING) + new OrderByColumnSpec("d0", OrderByColumnSpec.Direction.ASCENDING) ), 4 ) From eaa5a209b28ce622727eee07fc529c2f16250be7 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 14:02:02 +0000 Subject: [PATCH 06/13] fix order --- .../sql/calcite/planner/CalciteRulesManager.java | 2 +- .../DecoupledPlanningCalciteQueryTest.java | 16 ---------------- 2 files changed, 1 insertion(+), 17 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 bc98624c2412..a835e7cfaf3e 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,9 +132,9 @@ public class CalciteRulesManager CoreRules.FILTER_VALUES_MERGE, CoreRules.PROJECT_FILTER_VALUES_MERGE, CoreRules.PROJECT_VALUES_MERGE, - CoreRules.SORT_PROJECT_TRANSPOSE, CoreRules.PROJECT_MERGE, CoreRules.AGGREGATE_PROJECT_MERGE, + CoreRules.SORT_PROJECT_TRANSPOSE, CoreRules.AGGREGATE_VALUES ); 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 2e6bccee73ae..baab3c2be8dc 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 @@ -265,22 +265,6 @@ public void testFilterOnCurrentTimestampWithIntervalArithmetic() public void testFilterOnCurrentTimestampOnView() { - } - // When run through decoupled, it expects - // dimensions=[DefaultDimensionSpec{dimension='dim2', outputName='d0', outputType='STRING'}, - // DefaultDimensionSpec{dimension='dim1', outputName='d1', outputType='STRING'}] - // - // but gets - // dimensions=[DefaultDimensionSpec{dimension='dim1', outputName='d0', outputType='STRING'}, - // DefaultDimensionSpec{dimension='dim2', outputName='d1', outputType='STRING'}] - // - // The change in the ordering fails the query plan exact match. This needs to be revisited - // when we make more advancements into the decoupled planner - @Override - @Ignore - public void testExactCountDistinctWithGroupingAndOtherAggregators() - { - } @Override From 8c1b2356f472b38e0065c6a1cb02029b363de3fb Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Wed, 17 Jan 2024 15:47:56 +0000 Subject: [PATCH 07/13] not needed --- .../DecoupledPlanningCalciteQueryTest.java | 231 ------------------ 1 file changed, 231 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 f631f138cf1a..c429ac24f657 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,11 +25,7 @@ import org.apache.druid.sql.calcite.NotYetSupported.NotYetSupportedProcessor; import org.apache.druid.sql.calcite.planner.PlannerConfig; import org.apache.druid.sql.calcite.util.SqlTestFramework; -<<<<<<< HEAD -import org.junit.Ignore; -======= import org.junit.Rule; ->>>>>>> apache/master public class DecoupledPlanningCalciteQueryTest extends CalciteQueryTest { @@ -57,231 +53,4 @@ public SqlTestFramework.PlannerFixture plannerFixture(PlannerConfig plannerConfi .cannotVectorize(cannotVectorize) .skipVectorize(skipVectorize); } -<<<<<<< HEAD - - - @Override - @Ignore - public void testGroupByWithSelectAndOrderByProjections() - { - - } - - @Override - @Ignore - public void testTopNWithSelectAndOrderByProjections() - { - - } - - @Override - @Ignore - public void testUnionAllQueries() - { - - } - - @Override - @Ignore - public void testUnionAllQueriesWithLimit() - { - - } - - @Override - @Ignore - public void testUnionAllDifferentTablesWithMapping() - { - - } - - @Override - @Ignore - public void testJoinUnionAllDifferentTablesWithMapping() - { - - } - - @Override - @Ignore - public void testUnionAllTablesColumnTypeMismatchFloatLong() - { - - } - - @Override - @Ignore - public void testUnionAllTablesColumnTypeMismatchStringLong() - { - - } - - @Override - @Ignore - public void testUnionAllTablesWhenMappingIsRequired() - { - - } - - @Override - @Ignore - public void testUnionIsUnplannable() - { - - } - - @Override - @Ignore - public void testUnionAllTablesWhenCastAndMappingIsRequired() - { - - } - - @Override - @Ignore - public void testUnionAllSameTableTwice() - { - - } - - @Override - @Ignore - public void testUnionAllSameTableTwiceWithSameMapping() - { - - } - - @Override - @Ignore - public void testUnionAllSameTableTwiceWithDifferentMapping() - { - - } - - @Override - @Ignore - public void testUnionAllSameTableThreeTimes() - { - - } - - @Override - @Ignore - public void testUnionAllSameTableThreeTimesWithSameMapping() - { - - } - - @Override - @Ignore - public void testGroupByWithSortOnPostAggregationDefault() - { - - } - - @Override - @Ignore - public void testGroupByWithSortOnPostAggregationNoTopNConfig() - { - - } - - @Override - @Ignore - public void testGroupByWithSortOnPostAggregationNoTopNContext() - { - - } - - @Override - @Ignore - public void testUnplannableQueries() - { - - } - - @Override - @Ignore - public void testUnplannableTwoExactCountDistincts() - { - - } - - @Override - @Ignore - public void testUnplannableExactCountDistinctOnSketch() - { - - } - - @Override - @Ignore - public void testExactCountDistinctUsingSubqueryOnUnionAllTables() - { - - } - - @Override - @Ignore - public void testExactCountDistinctUsingSubqueryWithWherePushDown() - { - - } - - @Override - @Ignore - public void testGroupByTimeFloorAndDimOnGroupByTimeFloorAndDim() - { - - } - - @Override - @Ignore - public void testRequireTimeConditionPositive() - { - - } - - @Override - public void testRequireTimeConditionSemiJoinNegative() - { - - } - - @Override - @Ignore - public void testOrderByAlongWithInternalScanQuery() - { - - } - - @Override - @Ignore - public void testOrderByAlongWithInternalScanQueryNoDistinct() - { - - } - - @Override - @Ignore - public void testFilterOnCurrentTimestampWithIntervalArithmetic() - { - - } - - @Override - @Ignore - public void testFilterOnCurrentTimestampOnView() - { - - } - - @Override - @Ignore - public void testPlanWithInFilterLessThanInSubQueryThreshold() - { - - } -======= ->>>>>>> apache/master } From 206e5563e822bc835376fd8a316cbd7bd1f4d018 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Wed, 17 Jan 2024 15:48:27 +0000 Subject: [PATCH 08/13] undo postAgg name change for now; fix 1 test --- .../java/org/apache/druid/sql/calcite/rel/DruidQuery.java | 2 +- .../java/org/apache/druid/sql/calcite/CalciteQueryTest.java | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) 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 495d4171f92b..54ac7c364e2f 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 @@ -664,7 +664,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/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 2e2130e5fdd3..81e062f75171 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 @@ -14732,11 +14732,9 @@ public void testLatestByOnStringColumnWithoutMaxBytesSpecified() .setDataSource(CalciteTests.DATASOURCE1) .setInterval(querySegmentSpec(Filtration.eternity())) .setGranularity(Granularities.ALL) - .setVirtualColumns( - expressionVirtualColumn("v0", "'abc'", ColumnType.STRING)) .setDimFilter(equality("dim2", "abc", ColumnType.STRING)) .setDimensions( - dimensions(new DefaultDimensionSpec("v0", "d0", ColumnType.STRING))) + dimensions(new DefaultDimensionSpec("dim2", "d0", ColumnType.STRING))) .setAggregatorSpecs( aggregators( new StringLastAggregatorFactory("a0", "dim3", "__time", 1024), From e8498f81363f97381c48e1d6ffd3724ee8166fde Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Thu, 18 Jan 2024 08:41:31 +0000 Subject: [PATCH 09/13] some changes --- .../calcite/planner/CalciteRulesManager.java | 26 ++++- .../druid/sql/calcite/CalciteQueryTest.java | 105 ++++++++++-------- 2 files changed, 84 insertions(+), 47 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 65c5ed0d7ae9..0853645df8da 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 @@ -142,8 +142,12 @@ public class CalciteRulesManager CoreRules.PROJECT_FILTER_VALUES_MERGE, CoreRules.PROJECT_VALUES_MERGE, CoreRules.PROJECT_MERGE, + CoreRules.FILTER_MERGE, CoreRules.AGGREGATE_PROJECT_MERGE, - CoreRules.SORT_PROJECT_TRANSPOSE, +// CoreRules.PROJECT_FILTER_TRANSPOSE, + CoreRules.FILTER_PROJECT_TRANSPOSE, + CoreRules.PROJECT_MERGE, +// CoreRules.SORT_PROJECT_TRANSPOSE, CoreRules.AGGREGATE_VALUES ); @@ -234,7 +238,7 @@ public CalciteRulesManager(final Set extensionCalc public List programs(final PlannerContext plannerContext) { - final boolean isDebug = plannerContext.queryContext().isDebug(); + final boolean isDebug = true || plannerContext.queryContext().isDebug(); final Program druidPreProgram = buildPreProgram(plannerContext, true); final Program bindablePreProgram = buildPreProgram(plannerContext, false); @@ -265,7 +269,7 @@ public List programs(final PlannerContext plannerContext) */ private Program buildPreProgram(final PlannerContext plannerContext, final boolean isDruid) { - final boolean isDebug = plannerContext.queryContext().isDebug(); + final boolean isDebug = true ||plannerContext.queryContext().isDebug(); // Program that pre-processes the tree before letting the full-on VolcanoPlanner loose. final List prePrograms = new ArrayList<>(); @@ -280,6 +284,8 @@ private Program buildPreProgram(final PlannerContext plannerContext, final boole if (isDruid) { prePrograms.add(buildPreVolcanoManipulationProgram(plannerContext)); prePrograms.add(new LoggingProgram("Finished pre-Volcano manipulation program", isDebug)); +// prePrograms.add(buildReductionProgram(plannerContext, true)); +// prePrograms.add(new LoggingProgram("Finished reduction II program", isDebug)); } return Programs.sequence(prePrograms.toArray(new Program[0])); @@ -300,9 +306,22 @@ private Program buildPreVolcanoManipulationProgram(final PlannerContext plannerC builder.addRuleInstance(CoreRules.FILTER_INTO_JOIN); } +// builder.addRuleInstance(CoreRules.FILTER_PROJECT_TRANSPOSE); +// builder.addRuleInstance(CoreRules.PROJECT_MERGE); +//// builder.addRuleInstance(CoreRules.PROJECT_AGGREGATE_MERGE); +// builder.addRuleInstance(CoreRules.AGGREGATE_PROJECT_MERGE); // Apply SORT_PROJECT_TRANSPOSE to match the expected order of "sort" and "sortProject" in PartialDruidQuery. builder.addRuleInstance(CoreRules.SORT_PROJECT_TRANSPOSE); +// builder.addRuleInstance(CoreRules.FILTER_PROJECT_TRANSPOSE); +// builder.addRuleInstance(CoreRules.PROJECT_MERGE); +// builder.addRuleInstance(CoreRules.PROJECT_AGGREGATE_MERGE); +// builder.addRuleInstance(CoreRules.AGGREGATE_PROJECT_MERGE); +//CoreRules.PROJECT_AGGREGATE_MERGE, +//CoreRules.AGGREGATE_PROJECT_MERGE, +//CoreRules.SORT_PROJECT_TRANSPOSE, + + return Programs.of(builder.build(), true, DefaultRelMetadataProvider.INSTANCE); } @@ -429,6 +448,7 @@ public List baseRuleSet(final PlannerContext plannerContext) final ImmutableList.Builder rules = ImmutableList.builder(); // Calcite rules. + rules.addAll(BASE_RULES); rules.addAll(ABSTRACT_RULES); rules.addAll(ABSTRACT_RELATIONAL_RULES); 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 81e062f75171..8b96ed030b52 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 @@ -6815,7 +6815,6 @@ public void testApproxCountDistinctBuiltin() ); } - @NotYetSupported(Modes.PLAN_MISMATCH) @Test public void testExactCountDistinctWithGroupingAndOtherAggregators() { @@ -14085,61 +14084,78 @@ public void testReturnEmptyRowWhenGroupByIsConvertedToTimeseriesWithSingleConsta ), ImmutableList.of() ); + } + @Test + public void testReturnEmptyRowWhenGroupByIsConvertedToTimeseriesWithSingleConstantDimension2() + { + skipVectorize(); - - // dim1 is not getting reduced to 'wat' in this case in Calcite (ProjectMergeRule is not getting applied), - // therefore the query is not optimized to a timeseries query testQuery( "SELECT 'A' from foo WHERE dim1 = 'wat' GROUP BY dim1", ImmutableList.of( - GroupByQuery.builder() - .setDataSource( - "foo" - ) - .setInterval(querySegmentSpec(Intervals.ETERNITY)) - .setGranularity(Granularities.ALL) - .addDimension(new DefaultDimensionSpec("dim1", "d0", ColumnType.STRING)) - .setDimFilter(equality("dim1", "wat", ColumnType.STRING)) - .setPostAggregatorSpecs( - ImmutableList.of( - expressionPostAgg("p0", "'A'", ColumnType.STRING) - ) - ) - - .build() + Druids.newTimeseriesQueryBuilder() + .dataSource(CalciteTests.DATASOURCE1) + .intervals(querySegmentSpec(Filtration.eternity())) + .filters( + equality("dim1", "wat", ColumnType.STRING) + ) + .granularity(Granularities.ALL) + .postAggregators( + expressionPostAgg("p0", "'A'", ColumnType.STRING) + ) + .context(QUERY_CONTEXT_DO_SKIP_EMPTY_BUCKETS) + .build() ), ImmutableList.of() ); } + public void testQuery1( + final String sql, + final List> expectedQueries, + final List expectedResults + ) + { + testBuilder() + .sql(sql) + .expectedQueries(expectedQueries) + .expectedResults(expectedResults) + .run(); + } + @Test public void testReturnEmptyRowWhenGroupByIsConvertedToTimeseriesWithMultipleConstantDimensions() { skipVectorize(); - testQuery( - "SELECT 'A', dim1 from foo WHERE m1 = 50 AND dim1 = 'wat' GROUP BY dim1", - ImmutableList.of( - Druids.newTimeseriesQueryBuilder() - .dataSource(CalciteTests.DATASOURCE1) - .intervals(querySegmentSpec(Filtration.eternity())) - .filters( - and( - NullHandling.replaceWithDefault() - ? selector("m1", "50") - : equality("m1", 50.0, ColumnType.FLOAT), - equality("dim1", "wat", ColumnType.STRING) + testBuilder() + .sql("SELECT 'A', dim1 from foo WHERE m1 = 50 AND dim1 = 'wat' GROUP BY dim1") + .queryContext( + ImmutableMap.of( + QueryContexts.ENABLE_DEBUG, true + ) + ) + .expectedQueries(ImmutableList.of( + Druids.newTimeseriesQueryBuilder() + .dataSource(CalciteTests.DATASOURCE1) + .intervals(querySegmentSpec(Filtration.eternity())) + .filters( + and( + NullHandling.replaceWithDefault() + ? selector("m1", "50") + : equality("m1", 50.0, ColumnType.FLOAT), + equality("dim1", "wat", ColumnType.STRING) + ) ) - ) - .granularity(Granularities.ALL) - .postAggregators( - expressionPostAgg("p0", "'A'", ColumnType.STRING), - expressionPostAgg("p1", "'wat'", ColumnType.STRING) - ) - .context(QUERY_CONTEXT_DO_SKIP_EMPTY_BUCKETS) - .build() - ), - ImmutableList.of() - ); + .granularity(Granularities.ALL) + .postAggregators( + expressionPostAgg("p0", "'A'", ColumnType.STRING), + expressionPostAgg("p1", "'wat'", ColumnType.STRING) + ) + .context(QUERY_CONTEXT_DO_SKIP_EMPTY_BUCKETS) + .build() + )) + .expectedResults(ImmutableList.of()) + .run(); // Sanity test, that even when dimensions are reduced, but should produce a valid result (i.e. when filters are // correct, then they should @@ -14167,7 +14183,6 @@ public void testReturnEmptyRowWhenGroupByIsConvertedToTimeseriesWithMultipleCons ); } - @NotYetSupported(Modes.PLAN_MISMATCH) @Test public void testPlanWithInFilterLessThanInSubQueryThreshold() { @@ -14732,9 +14747,11 @@ public void testLatestByOnStringColumnWithoutMaxBytesSpecified() .setDataSource(CalciteTests.DATASOURCE1) .setInterval(querySegmentSpec(Filtration.eternity())) .setGranularity(Granularities.ALL) + .setVirtualColumns( + expressionVirtualColumn("v0", "'abc'", ColumnType.STRING)) .setDimFilter(equality("dim2", "abc", ColumnType.STRING)) .setDimensions( - dimensions(new DefaultDimensionSpec("dim2", "d0", ColumnType.STRING))) + dimensions(new DefaultDimensionSpec("v0", "d0", ColumnType.STRING))) .setAggregatorSpecs( aggregators( new StringLastAggregatorFactory("a0", "dim3", "__time", 1024), From 8ceb566257c2db625689ec268bbde61a353abe08 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Thu, 18 Jan 2024 08:43:58 +0000 Subject: [PATCH 10/13] kinda back to orig diffs --- .../calcite/planner/CalciteRulesManager.java | 19 ++----------------- 1 file changed, 2 insertions(+), 17 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 0853645df8da..62b4f2975e0e 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 @@ -238,7 +238,7 @@ public CalciteRulesManager(final Set extensionCalc public List programs(final PlannerContext plannerContext) { - final boolean isDebug = true || plannerContext.queryContext().isDebug(); + final boolean isDebug = plannerContext.queryContext().isDebug(); final Program druidPreProgram = buildPreProgram(plannerContext, true); final Program bindablePreProgram = buildPreProgram(plannerContext, false); @@ -269,7 +269,7 @@ public List programs(final PlannerContext plannerContext) */ private Program buildPreProgram(final PlannerContext plannerContext, final boolean isDruid) { - final boolean isDebug = true ||plannerContext.queryContext().isDebug(); + final boolean isDebug = plannerContext.queryContext().isDebug(); // Program that pre-processes the tree before letting the full-on VolcanoPlanner loose. final List prePrograms = new ArrayList<>(); @@ -284,8 +284,6 @@ private Program buildPreProgram(final PlannerContext plannerContext, final boole if (isDruid) { prePrograms.add(buildPreVolcanoManipulationProgram(plannerContext)); prePrograms.add(new LoggingProgram("Finished pre-Volcano manipulation program", isDebug)); -// prePrograms.add(buildReductionProgram(plannerContext, true)); -// prePrograms.add(new LoggingProgram("Finished reduction II program", isDebug)); } return Programs.sequence(prePrograms.toArray(new Program[0])); @@ -306,22 +304,9 @@ private Program buildPreVolcanoManipulationProgram(final PlannerContext plannerC builder.addRuleInstance(CoreRules.FILTER_INTO_JOIN); } -// builder.addRuleInstance(CoreRules.FILTER_PROJECT_TRANSPOSE); -// builder.addRuleInstance(CoreRules.PROJECT_MERGE); -//// builder.addRuleInstance(CoreRules.PROJECT_AGGREGATE_MERGE); -// builder.addRuleInstance(CoreRules.AGGREGATE_PROJECT_MERGE); // Apply SORT_PROJECT_TRANSPOSE to match the expected order of "sort" and "sortProject" in PartialDruidQuery. builder.addRuleInstance(CoreRules.SORT_PROJECT_TRANSPOSE); -// builder.addRuleInstance(CoreRules.FILTER_PROJECT_TRANSPOSE); -// builder.addRuleInstance(CoreRules.PROJECT_MERGE); -// builder.addRuleInstance(CoreRules.PROJECT_AGGREGATE_MERGE); -// builder.addRuleInstance(CoreRules.AGGREGATE_PROJECT_MERGE); -//CoreRules.PROJECT_AGGREGATE_MERGE, -//CoreRules.AGGREGATE_PROJECT_MERGE, -//CoreRules.SORT_PROJECT_TRANSPOSE, - - return Programs.of(builder.build(), true, DefaultRelMetadataProvider.INSTANCE); } From 2e99c01f967bbd881582610157a8ae5e6c048096 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Thu, 18 Jan 2024 11:43:19 +0000 Subject: [PATCH 11/13] some changes --- .../org/apache/druid/query/QueryContexts.java | 2 +- .../calcite/planner/CalciteRulesManager.java | 2 +- .../sql/calcite/rel/PartialDruidQuery.java | 3 ++- .../druid/sql/calcite/CalciteQueryTest.java | 25 +++++++++++++++++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/QueryContexts.java b/processing/src/main/java/org/apache/druid/query/QueryContexts.java index 0948f9513380..4f8921823ec1 100644 --- a/processing/src/main/java/org/apache/druid/query/QueryContexts.java +++ b/processing/src/main/java/org/apache/druid/query/QueryContexts.java @@ -114,7 +114,7 @@ public class QueryContexts public static final boolean DEFAULT_ENABLE_SQL_JOIN_LEFT_SCAN_DIRECT = false; public static final boolean DEFAULT_USE_FILTER_CNF = false; public static final boolean DEFAULT_SECONDARY_PARTITION_PRUNING = true; - public static final boolean DEFAULT_ENABLE_DEBUG = false; + public static final boolean DEFAULT_ENABLE_DEBUG = true; public static final int DEFAULT_IN_SUB_QUERY_THRESHOLD = 20; public static final boolean DEFAULT_ENABLE_TIME_BOUNDARY_PLANNING = false; 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 62b4f2975e0e..8333c9b8559c 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 @@ -143,7 +143,7 @@ public class CalciteRulesManager CoreRules.PROJECT_VALUES_MERGE, CoreRules.PROJECT_MERGE, CoreRules.FILTER_MERGE, - CoreRules.AGGREGATE_PROJECT_MERGE, +// CoreRules.AGGREGATE_PROJECT_MERGE, // CoreRules.PROJECT_FILTER_TRANSPOSE, CoreRules.FILTER_PROJECT_TRANSPOSE, CoreRules.PROJECT_MERGE, diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java index 1775cce3ae62..bd21f1ba66d5 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java @@ -50,6 +50,7 @@ import javax.annotation.Nullable; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.function.Supplier; @@ -501,7 +502,7 @@ public RelTraitSet getTraitSet(final Convention convention) } } } - + Collections.reverse(sortFields); return leafRelTraits.plus(convention).plus(RelCollations.of(sortFields)); } // Fall through. 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 8b96ed030b52..27b341a31a5d 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 @@ -15054,6 +15054,31 @@ public void testWindowingWithScanAndSort() .run(); } + + @NotYetSupported(Modes.CANNOT_TRANSLATE) + @Test + public void testWindowingWithScanAndSortX() + { + skipVectorize(); + cannotVectorize(); + msqIncompatible(); + String sql = "\n" + + "SELECT dim1,dim2,count(1) FROM foo GROUP BY dim2,dim1 order by dim2"; + ImmutableList expectedResults = ImmutableList.of( + new Object[]{"", "a", 1L}, + new Object[]{"1", "a", 1L}, + new Object[]{"10.1", null, 1L}, + new Object[]{"2", "", 1L}, + new Object[]{"abc", null, 1L}, + new Object[]{"def", "abc", 1L} + ); + + testBuilder() + .sql(sql) + .expectedResults(expectedResults) + .run(); + } + @NotYetSupported(Modes.CANNOT_TRANSLATE) @Test public void testWindowingWithOrderBy() From 0fd791c6646c5a434f6f3315f5af97c8e75df6c9 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Thu, 18 Jan 2024 12:36:23 +0000 Subject: [PATCH 12/13] x --- .../org/apache/druid/sql/calcite/rel/PartialDruidQuery.java | 3 +-- .../java/org/apache/druid/sql/calcite/CalciteQueryTest.java | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java index bd21f1ba66d5..1775cce3ae62 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/PartialDruidQuery.java @@ -50,7 +50,6 @@ import javax.annotation.Nullable; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.function.Supplier; @@ -502,7 +501,7 @@ public RelTraitSet getTraitSet(final Convention convention) } } } - Collections.reverse(sortFields); + return leafRelTraits.plus(convention).plus(RelCollations.of(sortFields)); } // Fall through. 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 27b341a31a5d..237464fc70ed 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 @@ -15063,7 +15063,7 @@ public void testWindowingWithScanAndSortX() cannotVectorize(); msqIncompatible(); String sql = "\n" - + "SELECT dim1,dim2,count(1) FROM foo GROUP BY dim2,dim1 order by dim2"; + + "SELECT dim1,dim2,count(1) FROM foo GROUP BY dim2,dim1 order by dim1"; ImmutableList expectedResults = ImmutableList.of( new Object[]{"", "a", 1L}, new Object[]{"1", "a", 1L}, From 454afd3c3a883f3ebf9efbc78a3c3b58a1f7cc61 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Thu, 18 Jan 2024 13:24:19 +0000 Subject: [PATCH 13/13] more trials --- .../druid/sql/calcite/planner/CalciteRulesManager.java | 2 +- .../org/apache/druid/sql/calcite/CalciteQueryTest.java | 10 +++++++++- 2 files changed, 10 insertions(+), 2 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 8333c9b8559c..8e68a5036ddd 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 @@ -147,7 +147,7 @@ public class CalciteRulesManager // CoreRules.PROJECT_FILTER_TRANSPOSE, CoreRules.FILTER_PROJECT_TRANSPOSE, CoreRules.PROJECT_MERGE, -// CoreRules.SORT_PROJECT_TRANSPOSE, + CoreRules.SORT_PROJECT_TRANSPOSE, CoreRules.AGGREGATE_VALUES ); 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 237464fc70ed..6f845e602402 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 @@ -15063,7 +15063,9 @@ public void testWindowingWithScanAndSortX() cannotVectorize(); msqIncompatible(); String sql = "\n" - + "SELECT dim1,dim2,count(1) FROM foo GROUP BY dim2,dim1 order by dim1"; + + "SELECT dim1,dim2,count(1),count(1) OVER (partition by dim2) FROM " + + " (select dim1,dim2 from foo GROUP BY dim1,dim2 order by dim2) t group by dim1,dim2"; +// + "SELECT dim1,dim2,count(1) FROM foo GROUP BY dim1,dim2 order by dim2"; ImmutableList expectedResults = ImmutableList.of( new Object[]{"", "a", 1L}, new Object[]{"1", "a", 1L}, @@ -15074,6 +15076,12 @@ public void testWindowingWithScanAndSortX() ); testBuilder() + .queryContext( + ImmutableMap.of( + PlannerContext.CTX_ENABLE_WINDOW_FNS, true, + QueryContexts.ENABLE_DEBUG, true + ) + ) .sql(sql) .expectedResults(expectedResults) .run();