From 5403406a71e9c52f351cee5bc34efcee878a2bcb Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 12:30:17 +0000 Subject: [PATCH 01/16] decoupledIgnore (cherry picked from commit e922c820a7d563ca49c9c686644bed967c42cb4b) --- .../druid/sql/calcite/CalciteQueryTest.java | 21 +- .../druid/sql/calcite/DecoupledIgnore.java | 31 ++ .../DecoupledPlanningCalciteQueryTest.java | 363 ++---------------- 3 files changed, 84 insertions(+), 331 deletions(-) create mode 100644 sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java 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..7ee5664879d3 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 @@ -2651,6 +2651,7 @@ public void testGroupByWithSelectProjections() ); } + @DecoupledIgnore @Test public void testGroupByWithSelectAndOrderByProjections() { @@ -2702,6 +2703,7 @@ public void testGroupByWithSelectAndOrderByProjections() ); } + @DecoupledIgnore @Test public void testTopNWithSelectProjections() { @@ -2735,7 +2737,6 @@ public void testTopNWithSelectProjections() ); } - @Test public void testTopNWithSelectAndOrderByProjections() { @@ -2834,6 +2835,7 @@ public void testUnionAllQueriesWithLimit() ); } + @DecoupledIgnore @Test public void testUnionAllDifferentTablesWithMapping() { @@ -2877,6 +2879,7 @@ public void testUnionAllDifferentTablesWithMapping() ); } + @DecoupledIgnore @Test public void testJoinUnionAllDifferentTablesWithMapping() { @@ -2987,6 +2990,7 @@ public void testUnionAllTablesColumnTypeMismatchFloatLong() ); } + @DecoupledIgnore @Test public void testUnionAllTablesColumnTypeMismatchStringLong() { @@ -3030,6 +3034,7 @@ public void testUnionIsUnplannable() ); } + @DecoupledIgnore @Test public void testUnionAllTablesWhenCastAndMappingIsRequired() { @@ -3045,6 +3050,7 @@ public void testUnionAllTablesWhenCastAndMappingIsRequired() ); } + @DecoupledIgnore @Test public void testUnionAllSameTableTwice() { @@ -3144,7 +3150,7 @@ public void testUnionAllSameTableTwiceWithDifferentMapping() "SQL requires union between two tables and column names queried for each table are different Left: [dim1, dim2, m1], Right: [dim2, dim1, m1]." ); } - + @DecoupledIgnore @Test public void testUnionAllSameTableThreeTimes() { @@ -4969,7 +4975,7 @@ public void testSimpleAggregations() new Object[]{6L, 6L, 6L, 1L, 6L, 8L, 4L, 3L, ((1 + 1.7) / 3)} ) ); - } + }@DecoupledIgnore @Test public void testGroupByWithSortOnPostAggregationDefault() @@ -5044,7 +5050,7 @@ public void testGroupByWithSortOnPostAggregationNoTopNConfig() ) ); } - + @DecoupledIgnore @Test public void testGroupByWithSortOnPostAggregationNoTopNContext() { @@ -5702,7 +5708,7 @@ public void testCountStarWithBoundFilterSimplifyOr() ) ); } - + @DecoupledIgnore @Test public void testUnplannableTwoExactCountDistincts() { @@ -11744,6 +11750,7 @@ public void testTextcat() ); } + @DecoupledIgnore @Test public void testRequireTimeConditionPositive() { @@ -11946,7 +11953,7 @@ public void testRequireTimeConditionSubQueryNegative() ImmutableList.of() ); } - + @DecoupledIgnore @Test public void testRequireTimeConditionSemiJoinNegative() { @@ -14209,7 +14216,7 @@ public void testComplexDecodeAgg() ) ); } - + @DecoupledIgnore @Test public void testOrderByAlongWithInternalScanQuery() { diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java new file mode 100644 index 000000000000..703c1182de66 --- /dev/null +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.druid.sql.calcite; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target({ElementType.METHOD}) +public @interface DecoupledIgnore +{ + +} 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..98f1f57ad1ad 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 @@ -24,15 +24,48 @@ import org.apache.druid.server.security.AuthConfig; import org.apache.druid.sql.calcite.planner.PlannerConfig; import org.apache.druid.sql.calcite.util.SqlTestFramework; -import org.junit.Ignore; -import org.junit.Test; +import org.junit.AssumptionViolatedException; +import org.junit.Rule; +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +import static org.junit.Assert.assertThrows; public class DecoupledPlanningCalciteQueryTest extends CalciteQueryTest { + + @Rule + public DecoupledIgnoreProcessor decoupledIgnoreProcessor = new DecoupledIgnoreProcessor(); + + public static class DecoupledIgnoreProcessor implements TestRule + { + + public Statement apply(Statement base, Description description) + { + DecoupledIgnore annotation = description.getAnnotation(DecoupledIgnore.class); + return new Statement() + { + @Override + public void evaluate() throws Throwable + { + if (annotation != null) { + Exception e = assertThrows("Expected that this testcase will fail - it might got fixed?", Exception.class, + () -> { + base.evaluate(); + }); + throw new AssumptionViolatedException("Test is not-yet supported in Decoupled mode"); + } else { + base.evaluate(); + } + } + }; + } + }; + private static final ImmutableMap CONTEXT_OVERRIDES = ImmutableMap.of( PlannerConfig.CTX_NATIVE_QUERY_SQL_PLANNING_MODE, PlannerConfig.NATIVE_QUERY_SQL_PLANNING_MODE_DECOUPLED, - QueryContexts.ENABLE_DEBUG, true - ); + QueryContexts.ENABLE_DEBUG, true); @Override protected QueryTestBuilder testBuilder() @@ -47,325 +80,7 @@ public SqlTestFramework.PlannerFixture plannerFixture(PlannerConfig plannerConfi return queryFramework().plannerFixture(DecoupledPlanningCalciteQueryTest.this, plannerConfig, authConfig); } }) - .cannotVectorize(cannotVectorize) - .skipVectorize(skipVectorize); - } - - - @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 testPostAggWithTimeseries() - { - - } - - @Override - @Ignore - public void testPostAggWithTopN() - { - - } - - @Override - @Ignore - public void testRequireTimeConditionPositive() - { - - } - - @Override - 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() - { - - } - - @Override - @Ignore - public void testSortProjectAfterNestedGroupBy() - { - - } - - @Override - @Ignore - public void testOrderByAlongWithInternalScanQueryNoDistinct() - { - - } - - @Override - @Ignore - public void testNestedGroupBy() - { - - } - - @Override - @Ignore - public void testQueryWithSelectProjectAndIdentityProjectDoesNotRename() - { - - } - - @Override - @Ignore - public void testFilterOnCurrentTimestampWithIntervalArithmetic() - { - - } - - @Override - @Ignore - 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 - @Ignore - public void testTopNWithSelectProjections() - { - - } - - @Override - @Ignore - public void testPlanWithInFilterLessThanInSubQueryThreshold() - { - + .cannotVectorize(cannotVectorize) + .skipVectorize(skipVectorize); } } From c55737279d1e07f1ceee1a30ebeee2be4835aeea Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 15:31:20 +0000 Subject: [PATCH 02/16] some stuff --- .../sql/calcite/BaseCalciteQueryTest.java | 4 ++-- .../druid/sql/calcite/CalciteQueryTest.java | 23 ++++++++++++++----- .../druid/sql/calcite/DecoupledIgnore.java | 17 ++++++++++++++ .../DecoupledPlanningCalciteQueryTest.java | 15 +++++++++--- 4 files changed, 48 insertions(+), 11 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java index 3c73319898e4..e72537c7da5f 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/BaseCalciteQueryTest.java @@ -284,10 +284,10 @@ public static Map getTimeseriesContextWithFloorTime( private static SqlTestFramework queryFramework; final boolean useDefault = NullHandling.replaceWithDefault(); - @Rule + @Rule(order = 1) public ExpectedException expectedException = ExpectedException.none(); - @Rule + @Rule(order = 2) public TemporaryFolder temporaryFolder = new TemporaryFolder(); public boolean cannotVectorize = false; 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 7ee5664879d3..24a1d5f10ad8 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 @@ -2703,7 +2703,7 @@ public void testGroupByWithSelectAndOrderByProjections() ); } - @DecoupledIgnore + @DecoupledIgnore(expected = AssertionError.class) @Test public void testTopNWithSelectProjections() { @@ -2807,6 +2807,7 @@ public void testUnionAllQueries() ); } + @DecoupledIgnore(expected = DruidException.class, regex="not enough rules") @Test public void testUnionAllQueriesWithLimit() { @@ -2943,6 +2944,7 @@ public void testUnionAllTablesColumnCountMismatch() } } + @DecoupledIgnore(expected = DruidException.class, regex="not enough rules") @Test public void testUnionAllTablesColumnTypeMismatchFloatLong() { @@ -2990,7 +2992,7 @@ public void testUnionAllTablesColumnTypeMismatchFloatLong() ); } - @DecoupledIgnore + @DecoupledIgnore(expected = AssertionError.class) @Test public void testUnionAllTablesColumnTypeMismatchStringLong() { @@ -3024,6 +3026,7 @@ public void testUnionAllTablesWhenMappingIsRequired() ); } + @DecoupledIgnore(expected = AssertionError.class) @Test public void testUnionIsUnplannable() { @@ -3034,7 +3037,7 @@ public void testUnionIsUnplannable() ); } - @DecoupledIgnore + @DecoupledIgnore(expected = AssertionError.class) @Test public void testUnionAllTablesWhenCastAndMappingIsRequired() { @@ -3094,6 +3097,7 @@ public void testUnionAllSameTableTwice() ); } + @DecoupledIgnore(expected = DruidException.class, regex="not enough rules") @Test public void testUnionAllSameTableTwiceWithSameMapping() { @@ -5008,6 +5012,7 @@ public void testGroupByWithSortOnPostAggregationDefault() ); } + @DecoupledIgnore(expected = DruidException.class, regex="Cannot convert query") @Test public void testGroupByWithSortOnPostAggregationNoTopNConfig() { @@ -5638,6 +5643,7 @@ public void testCountStarWithNotOfDegenerateFilter() ); } + @DecoupledIgnore(expected = AssertionError.class) @Test public void testUnplannableQueries() { @@ -5708,7 +5714,7 @@ public void testCountStarWithBoundFilterSimplifyOr() ) ); } - @DecoupledIgnore + @DecoupledIgnore(expected = AssertionError.class) @Test public void testUnplannableTwoExactCountDistincts() { @@ -5721,6 +5727,7 @@ public void testUnplannableTwoExactCountDistincts() ); } + @DecoupledIgnore(expected = AssertionError.class) @Test public void testUnplannableExactCountDistinctOnSketch() { @@ -8063,6 +8070,7 @@ public void testGroupAndFilterOnTimeFloorWithTimeZone() ); } + @DecoupledIgnore(expected = AssertionError.class, regex="^query #1") @Test public void testFilterOnCurrentTimestampWithIntervalArithmetic() { @@ -10301,6 +10309,8 @@ public void testGroupByTimeAndOtherDimension() ); } + @DecoupledIgnore(expected = AssertionError.class, regex="AssertionError: query #1", + mode = DecoupledIgnore.Modes.PLAN_MISMATCH) @Test public void testGroupByTimeFloorAndDimOnGroupByTimeFloorAndDim() { @@ -11467,7 +11477,7 @@ public void testSortProjectAfterNestedGroupBy() ); } - + @DecoupledIgnore(expected = DruidException.class, regex="not enough rules") @Test public void testPostAggWithTimeseries() { @@ -11953,7 +11963,8 @@ public void testRequireTimeConditionSubQueryNegative() ImmutableList.of() ); } - @DecoupledIgnore + + @DecoupledIgnore(expected = AssertionError.class) @Test public void testRequireTimeConditionSemiJoinNegative() { diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java index 703c1182de66..31b17b7bef6c 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java @@ -27,5 +27,22 @@ @Target({ElementType.METHOD}) public @interface DecoupledIgnore { + static enum Modes + { + PLAN_MISMATCH(AssertionError.class); + + private Class throwableClass; + + Modes(Class cl) + { + this.throwableClass = cl; + } + }; + + Class expected() default Exception.class; + + String regex() default ""; + + Modes mode() default Modes.PLAN_MISMATCH; } 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 98f1f57ad1ad..b58b37fb4bf1 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 @@ -19,6 +19,7 @@ package org.apache.druid.sql.calcite; +import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import org.apache.druid.query.QueryContexts; import org.apache.druid.server.security.AuthConfig; @@ -31,16 +32,16 @@ import org.junit.runners.model.Statement; import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; public class DecoupledPlanningCalciteQueryTest extends CalciteQueryTest { - @Rule + @Rule(order = 0) public DecoupledIgnoreProcessor decoupledIgnoreProcessor = new DecoupledIgnoreProcessor(); public static class DecoupledIgnoreProcessor implements TestRule { - public Statement apply(Statement base, Description description) { DecoupledIgnore annotation = description.getAnnotation(DecoupledIgnore.class); @@ -50,10 +51,18 @@ public Statement apply(Statement base, Description description) public void evaluate() throws Throwable { if (annotation != null) { - Exception e = assertThrows("Expected that this testcase will fail - it might got fixed?", Exception.class, + Throwable e = assertThrows( + "Expected that this testcase will fail - it might got fixed?", + annotation.expected(), () -> { base.evaluate(); }); + + if (!annotation.regex().isBlank()) { + assertTrue( + "Exception stactrace doesn't match regex: " + annotation.regex(), + Throwables.getStackTraceAsString(e).contains(annotation.regex())); + } throw new AssumptionViolatedException("Test is not-yet supported in Decoupled mode"); } else { base.evaluate(); From f2a4fce7e2067c8deb268ab12e4f53ef75e23455 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 16:10:07 +0000 Subject: [PATCH 03/16] fixes/etc/etc --- .../druid/sql/calcite/CalciteQueryTest.java | 53 ++++++++++++------- .../druid/sql/calcite/DecoupledIgnore.java | 20 ++++--- .../DecoupledPlanningCalciteQueryTest.java | 19 ++++--- 3 files changed, 58 insertions(+), 34 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 24a1d5f10ad8..b9e8412a5c1a 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 @@ -107,6 +107,7 @@ import org.apache.druid.segment.column.ColumnType; import org.apache.druid.segment.column.RowSignature; import org.apache.druid.segment.join.JoinType; +import org.apache.druid.sql.calcite.DecoupledIgnore.Modes; import org.apache.druid.sql.calcite.expression.DruidExpression; import org.apache.druid.sql.calcite.filtration.Filtration; import org.apache.druid.sql.calcite.planner.Calcites; @@ -2651,7 +2652,7 @@ public void testGroupByWithSelectProjections() ); } - @DecoupledIgnore + @DecoupledIgnore(mode=Modes.CANNOT_CONVERT) @Test public void testGroupByWithSelectAndOrderByProjections() { @@ -2703,7 +2704,7 @@ public void testGroupByWithSelectAndOrderByProjections() ); } - @DecoupledIgnore(expected = AssertionError.class) + @DecoupledIgnore(mode = Modes.PLAN_MISMATCH) @Test public void testTopNWithSelectProjections() { @@ -2737,6 +2738,7 @@ public void testTopNWithSelectProjections() ); } + @DecoupledIgnore(mode=Modes.CANNOT_CONVERT) @Test public void testTopNWithSelectAndOrderByProjections() { @@ -2807,7 +2809,7 @@ public void testUnionAllQueries() ); } - @DecoupledIgnore(expected = DruidException.class, regex="not enough rules") + @DecoupledIgnore(mode=Modes.NOT_ENOUGH_RULES) @Test public void testUnionAllQueriesWithLimit() { @@ -2880,7 +2882,7 @@ public void testUnionAllDifferentTablesWithMapping() ); } - @DecoupledIgnore + @DecoupledIgnore(mode=Modes.NOT_ENOUGH_RULES) @Test public void testJoinUnionAllDifferentTablesWithMapping() { @@ -2944,7 +2946,7 @@ public void testUnionAllTablesColumnCountMismatch() } } - @DecoupledIgnore(expected = DruidException.class, regex="not enough rules") + @DecoupledIgnore(mode=Modes.NOT_ENOUGH_RULES) @Test public void testUnionAllTablesColumnTypeMismatchFloatLong() { @@ -2992,7 +2994,7 @@ public void testUnionAllTablesColumnTypeMismatchFloatLong() ); } - @DecoupledIgnore(expected = AssertionError.class) + @DecoupledIgnore(mode=Modes.TARGET_PERSONA) @Test public void testUnionAllTablesColumnTypeMismatchStringLong() { @@ -3010,6 +3012,7 @@ public void testUnionAllTablesColumnTypeMismatchStringLong() ); } + @DecoupledIgnore(mode = Modes.TARGET_PERSONA) @Test public void testUnionAllTablesWhenMappingIsRequired() { @@ -3026,7 +3029,7 @@ public void testUnionAllTablesWhenMappingIsRequired() ); } - @DecoupledIgnore(expected = AssertionError.class) + @DecoupledIgnore(mode=Modes.TARGET_PERSONA) @Test public void testUnionIsUnplannable() { @@ -3037,7 +3040,7 @@ public void testUnionIsUnplannable() ); } - @DecoupledIgnore(expected = AssertionError.class) + @DecoupledIgnore(mode=Modes.TARGET_PERSONA) @Test public void testUnionAllTablesWhenCastAndMappingIsRequired() { @@ -3097,7 +3100,7 @@ public void testUnionAllSameTableTwice() ); } - @DecoupledIgnore(expected = DruidException.class, regex="not enough rules") + @DecoupledIgnore(mode=Modes.NOT_ENOUGH_RULES) @Test public void testUnionAllSameTableTwiceWithSameMapping() { @@ -3141,6 +3144,7 @@ public void testUnionAllSameTableTwiceWithSameMapping() ); } + @DecoupledIgnore(mode=Modes.TARGET_PERSONA) @Test public void testUnionAllSameTableTwiceWithDifferentMapping() { @@ -4979,8 +4983,9 @@ public void testSimpleAggregations() new Object[]{6L, 6L, 6L, 1L, 6L, 8L, 4L, 3L, ((1 + 1.7) / 3)} ) ); - }@DecoupledIgnore + } + @DecoupledIgnore @Test public void testGroupByWithSortOnPostAggregationDefault() { @@ -5012,7 +5017,7 @@ public void testGroupByWithSortOnPostAggregationDefault() ); } - @DecoupledIgnore(expected = DruidException.class, regex="Cannot convert query") + @DecoupledIgnore(mode=Modes.NOT_ENOUGH_RULES) @Test public void testGroupByWithSortOnPostAggregationNoTopNConfig() { @@ -5055,7 +5060,8 @@ public void testGroupByWithSortOnPostAggregationNoTopNConfig() ) ); } - @DecoupledIgnore + + @DecoupledIgnore(mode = Modes.CANNOT_CONVERT) @Test public void testGroupByWithSortOnPostAggregationNoTopNContext() { @@ -5643,7 +5649,7 @@ public void testCountStarWithNotOfDegenerateFilter() ); } - @DecoupledIgnore(expected = AssertionError.class) + @DecoupledIgnore(mode=Modes.CANNOT_CONVERT) @Test public void testUnplannableQueries() { @@ -5714,7 +5720,8 @@ public void testCountStarWithBoundFilterSimplifyOr() ) ); } - @DecoupledIgnore(expected = AssertionError.class) + + @DecoupledIgnore(mode=Modes.TARGET_PERSONA) @Test public void testUnplannableTwoExactCountDistincts() { @@ -5727,7 +5734,7 @@ public void testUnplannableTwoExactCountDistincts() ); } - @DecoupledIgnore(expected = AssertionError.class) + @DecoupledIgnore(mode=Modes.TARGET_PERSONA) @Test public void testUnplannableExactCountDistinctOnSketch() { @@ -6701,6 +6708,7 @@ public void testApproxCountDistinctBuiltin() ); } + @DecoupledIgnore(mode = Modes.PLAN_MISMATCH) @Test public void testExactCountDistinctWithGroupingAndOtherAggregators() { @@ -7166,6 +7174,7 @@ public void testExactCountDistinctUsingSubquery() ); } + @DecoupledIgnore(mode = Modes.NOT_ENOUGH_RULES) @Test public void testExactCountDistinctUsingSubqueryOnUnionAllTables() { @@ -7340,6 +7349,7 @@ public void testQueryWithMoreThanMaxNumericInFilter() ); } + @DecoupledIgnore(mode = Modes.PLAN_MISMATCH) @Test public void testExactCountDistinctUsingSubqueryWithWherePushDown() { @@ -8070,7 +8080,7 @@ public void testGroupAndFilterOnTimeFloorWithTimeZone() ); } - @DecoupledIgnore(expected = AssertionError.class, regex="^query #1") + @DecoupledIgnore(mode=Modes.PLAN_MISMATCH) @Test public void testFilterOnCurrentTimestampWithIntervalArithmetic() { @@ -8118,6 +8128,7 @@ public void testFilterOnCurrentTimestampLosAngeles() ); } + @DecoupledIgnore(mode = Modes.PLAN_MISMATCH) @Test public void testFilterOnCurrentTimestampOnView() { @@ -10309,8 +10320,7 @@ public void testGroupByTimeAndOtherDimension() ); } - @DecoupledIgnore(expected = AssertionError.class, regex="AssertionError: query #1", - mode = DecoupledIgnore.Modes.PLAN_MISMATCH) + @DecoupledIgnore(mode=Modes.PLAN_MISMATCH) @Test public void testGroupByTimeFloorAndDimOnGroupByTimeFloorAndDim() { @@ -11477,7 +11487,7 @@ public void testSortProjectAfterNestedGroupBy() ); } - @DecoupledIgnore(expected = DruidException.class, regex="not enough rules") + @DecoupledIgnore(mode=Modes.CANNOT_CONVERT) @Test public void testPostAggWithTimeseries() { @@ -11521,6 +11531,7 @@ public void testPostAggWithTimeseries() ); } + @DecoupledIgnore(mode = Modes.PLAN_MISMATCH) @Test public void testPostAggWithTopN() { @@ -11964,7 +11975,7 @@ public void testRequireTimeConditionSubQueryNegative() ); } - @DecoupledIgnore(expected = AssertionError.class) + @DecoupledIgnore(mode=Modes.TARGET_PERSONA) @Test public void testRequireTimeConditionSemiJoinNegative() { @@ -13907,6 +13918,7 @@ public void testReturnEmptyRowWhenGroupByIsConvertedToTimeseriesWithMultipleCons ); } + @DecoupledIgnore(mode = Modes.PLAN_MISMATCH) @Test public void testPlanWithInFilterLessThanInSubQueryThreshold() { @@ -14270,6 +14282,7 @@ public void testOrderByAlongWithInternalScanQuery() ); } + @DecoupledIgnore(mode=Modes.NOT_ENOUGH_RULES) @Test public void testOrderByAlongWithInternalScanQueryNoDistinct() { diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java index 31b17b7bef6c..1be4940e8745 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java @@ -18,6 +18,8 @@ package org.apache.druid.sql.calcite; +import org.apache.druid.error.DruidException; + import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -29,20 +31,22 @@ { static enum Modes { - PLAN_MISMATCH(AssertionError.class); + PLAN_MISMATCH(AssertionError.class, "AssertionError: query #"), + NOT_ENOUGH_RULES(DruidException.class, "not enough rules"), + CANNOT_CONVERT(DruidException.class, "Cannot convert query parts"), + TARGET_PERSONA(AssertionError.class, "(is was |with message a string containing)"), + ; - private Class throwableClass; + public Class throwableClass; + public String regex; - Modes(Class cl) + Modes(Class cl,String regex) { this.throwableClass = cl; + this.regex = regex; } }; - Class expected() default Exception.class; - - String regex() default ""; - - Modes mode() default Modes.PLAN_MISMATCH; + Modes mode() default Modes.NOT_ENOUGH_RULES; } 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 b58b37fb4bf1..12bf8086d1ef 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 @@ -31,8 +31,10 @@ import org.junit.runner.Description; import org.junit.runners.model.Statement; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; public class DecoupledPlanningCalciteQueryTest extends CalciteQueryTest { @@ -53,15 +55,20 @@ public void evaluate() throws Throwable if (annotation != null) { Throwable e = assertThrows( "Expected that this testcase will fail - it might got fixed?", - annotation.expected(), + annotation.mode().throwableClass, () -> { base.evaluate(); }); - if (!annotation.regex().isBlank()) { - assertTrue( - "Exception stactrace doesn't match regex: " + annotation.regex(), - Throwables.getStackTraceAsString(e).contains(annotation.regex())); + String regex = annotation.mode().regex; + String trace = Throwables.getStackTraceAsString(e); + Pattern pat = Pattern.compile(regex); + Matcher m=pat.matcher(trace); + + if (!m.find()) { +// if(!Throwables.getStackTraceAsString(e).matches(regex)) { + throw new AssertionError("Exception stactrace doesn't match regex: " + regex,e); +// } } throw new AssumptionViolatedException("Test is not-yet supported in Decoupled mode"); } else { From 868b3e30f52ad492effccd82928b083d693f28c3 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 16:12:16 +0000 Subject: [PATCH 04/16] more wood --- .../java/org/apache/druid/sql/calcite/CalciteQueryTest.java | 6 ++++-- .../java/org/apache/druid/sql/calcite/DecoupledIgnore.java | 2 +- 2 files changed, 5 insertions(+), 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 b9e8412a5c1a..05b05a12ff53 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 @@ -2776,6 +2776,7 @@ public void testTopNWithSelectAndOrderByProjections() ); } + @DecoupledIgnore @Test public void testUnionAllQueries() { @@ -3263,6 +3264,7 @@ public void testUnionAllThreeTablesColumnCountMismatch3() } } + @DecoupledIgnore @Test public void testUnionAllSameTableThreeTimesWithSameMapping() { @@ -4985,7 +4987,7 @@ public void testSimpleAggregations() ); } - @DecoupledIgnore + @DecoupledIgnore(mode=Modes.CANNOT_CONVERT) @Test public void testGroupByWithSortOnPostAggregationDefault() { @@ -5017,7 +5019,7 @@ public void testGroupByWithSortOnPostAggregationDefault() ); } - @DecoupledIgnore(mode=Modes.NOT_ENOUGH_RULES) + @DecoupledIgnore(mode=Modes.CANNOT_CONVERT) @Test public void testGroupByWithSortOnPostAggregationNoTopNConfig() { diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java index 1be4940e8745..ea35ef39e71b 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java @@ -34,7 +34,7 @@ static enum Modes PLAN_MISMATCH(AssertionError.class, "AssertionError: query #"), NOT_ENOUGH_RULES(DruidException.class, "not enough rules"), CANNOT_CONVERT(DruidException.class, "Cannot convert query parts"), - TARGET_PERSONA(AssertionError.class, "(is was |with message a string containing)"), + TARGET_PERSONA(AssertionError.class, "(is was |is was |with message a string containing)"), ; public Class throwableClass; From 10e2f78129a5e9cff8589caf7c0696a4400e5677 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 16:13:38 +0000 Subject: [PATCH 05/16] fix/rename --- .../druid/sql/calcite/CalciteQueryTest.java | 18 +++++++++--------- .../druid/sql/calcite/DecoupledIgnore.java | 2 +- 2 files changed, 10 insertions(+), 10 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 05b05a12ff53..1889c4d0754b 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 @@ -2995,7 +2995,7 @@ public void testUnionAllTablesColumnTypeMismatchFloatLong() ); } - @DecoupledIgnore(mode=Modes.TARGET_PERSONA) + @DecoupledIgnore(mode=Modes.ERROR_HANDLING) @Test public void testUnionAllTablesColumnTypeMismatchStringLong() { @@ -3013,7 +3013,7 @@ public void testUnionAllTablesColumnTypeMismatchStringLong() ); } - @DecoupledIgnore(mode = Modes.TARGET_PERSONA) + @DecoupledIgnore(mode = Modes.ERROR_HANDLING) @Test public void testUnionAllTablesWhenMappingIsRequired() { @@ -3030,7 +3030,7 @@ public void testUnionAllTablesWhenMappingIsRequired() ); } - @DecoupledIgnore(mode=Modes.TARGET_PERSONA) + @DecoupledIgnore(mode=Modes.ERROR_HANDLING) @Test public void testUnionIsUnplannable() { @@ -3041,7 +3041,7 @@ public void testUnionIsUnplannable() ); } - @DecoupledIgnore(mode=Modes.TARGET_PERSONA) + @DecoupledIgnore(mode=Modes.ERROR_HANDLING) @Test public void testUnionAllTablesWhenCastAndMappingIsRequired() { @@ -3145,7 +3145,7 @@ public void testUnionAllSameTableTwiceWithSameMapping() ); } - @DecoupledIgnore(mode=Modes.TARGET_PERSONA) + @DecoupledIgnore(mode=Modes.ERROR_HANDLING) @Test public void testUnionAllSameTableTwiceWithDifferentMapping() { @@ -5651,7 +5651,7 @@ public void testCountStarWithNotOfDegenerateFilter() ); } - @DecoupledIgnore(mode=Modes.CANNOT_CONVERT) + @DecoupledIgnore(mode=Modes.ERROR_HANDLING) @Test public void testUnplannableQueries() { @@ -5723,7 +5723,7 @@ public void testCountStarWithBoundFilterSimplifyOr() ); } - @DecoupledIgnore(mode=Modes.TARGET_PERSONA) + @DecoupledIgnore(mode=Modes.ERROR_HANDLING) @Test public void testUnplannableTwoExactCountDistincts() { @@ -5736,7 +5736,7 @@ public void testUnplannableTwoExactCountDistincts() ); } - @DecoupledIgnore(mode=Modes.TARGET_PERSONA) + @DecoupledIgnore(mode=Modes.ERROR_HANDLING) @Test public void testUnplannableExactCountDistinctOnSketch() { @@ -11977,7 +11977,7 @@ public void testRequireTimeConditionSubQueryNegative() ); } - @DecoupledIgnore(mode=Modes.TARGET_PERSONA) + @DecoupledIgnore(mode=Modes.ERROR_HANDLING) @Test public void testRequireTimeConditionSemiJoinNegative() { diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java index ea35ef39e71b..754de1233f9c 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java @@ -34,7 +34,7 @@ static enum Modes PLAN_MISMATCH(AssertionError.class, "AssertionError: query #"), NOT_ENOUGH_RULES(DruidException.class, "not enough rules"), CANNOT_CONVERT(DruidException.class, "Cannot convert query parts"), - TARGET_PERSONA(AssertionError.class, "(is was |is was |with message a string containing)"), + ERROR_HANDLING(AssertionError.class, "(is was |is was |with message a string containing)"), ; public Class throwableClass; From fec75ecb90e05c3c24a15153aca854163dc9a28d Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 16:14:34 +0000 Subject: [PATCH 06/16] format --- .../java/org/apache/druid/sql/calcite/CalciteQueryTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1889c4d0754b..bf991d270503 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 @@ -14284,7 +14284,7 @@ public void testOrderByAlongWithInternalScanQuery() ); } - @DecoupledIgnore(mode=Modes.NOT_ENOUGH_RULES) + @DecoupledIgnore(mode = Modes.NOT_ENOUGH_RULES) @Test public void testOrderByAlongWithInternalScanQueryNoDistinct() { From 36a2b2ad6d546ab81e5184add21f843aabdb1fe6 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 16:15:10 +0000 Subject: [PATCH 07/16] checkstyle --- .../druid/sql/calcite/CalciteQueryTest.java | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 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 bf991d270503..ae89abc72ed8 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 @@ -2652,7 +2652,7 @@ public void testGroupByWithSelectProjections() ); } - @DecoupledIgnore(mode=Modes.CANNOT_CONVERT) + @DecoupledIgnore(mode = Modes.CANNOT_CONVERT) @Test public void testGroupByWithSelectAndOrderByProjections() { @@ -2738,7 +2738,7 @@ public void testTopNWithSelectProjections() ); } - @DecoupledIgnore(mode=Modes.CANNOT_CONVERT) + @DecoupledIgnore(mode = Modes.CANNOT_CONVERT) @Test public void testTopNWithSelectAndOrderByProjections() { @@ -2810,7 +2810,7 @@ public void testUnionAllQueries() ); } - @DecoupledIgnore(mode=Modes.NOT_ENOUGH_RULES) + @DecoupledIgnore(mode = Modes.NOT_ENOUGH_RULES) @Test public void testUnionAllQueriesWithLimit() { @@ -2883,7 +2883,7 @@ public void testUnionAllDifferentTablesWithMapping() ); } - @DecoupledIgnore(mode=Modes.NOT_ENOUGH_RULES) + @DecoupledIgnore(mode = Modes.NOT_ENOUGH_RULES) @Test public void testJoinUnionAllDifferentTablesWithMapping() { @@ -2947,7 +2947,7 @@ public void testUnionAllTablesColumnCountMismatch() } } - @DecoupledIgnore(mode=Modes.NOT_ENOUGH_RULES) + @DecoupledIgnore(mode = Modes.NOT_ENOUGH_RULES) @Test public void testUnionAllTablesColumnTypeMismatchFloatLong() { @@ -2995,7 +2995,7 @@ public void testUnionAllTablesColumnTypeMismatchFloatLong() ); } - @DecoupledIgnore(mode=Modes.ERROR_HANDLING) + @DecoupledIgnore(mode = Modes.ERROR_HANDLING) @Test public void testUnionAllTablesColumnTypeMismatchStringLong() { @@ -3030,7 +3030,7 @@ public void testUnionAllTablesWhenMappingIsRequired() ); } - @DecoupledIgnore(mode=Modes.ERROR_HANDLING) + @DecoupledIgnore(mode = Modes.ERROR_HANDLING) @Test public void testUnionIsUnplannable() { @@ -3041,7 +3041,7 @@ public void testUnionIsUnplannable() ); } - @DecoupledIgnore(mode=Modes.ERROR_HANDLING) + @DecoupledIgnore(mode = Modes.ERROR_HANDLING) @Test public void testUnionAllTablesWhenCastAndMappingIsRequired() { @@ -3101,7 +3101,7 @@ public void testUnionAllSameTableTwice() ); } - @DecoupledIgnore(mode=Modes.NOT_ENOUGH_RULES) + @DecoupledIgnore(mode = Modes.NOT_ENOUGH_RULES) @Test public void testUnionAllSameTableTwiceWithSameMapping() { @@ -3145,7 +3145,7 @@ public void testUnionAllSameTableTwiceWithSameMapping() ); } - @DecoupledIgnore(mode=Modes.ERROR_HANDLING) + @DecoupledIgnore(mode = Modes.ERROR_HANDLING) @Test public void testUnionAllSameTableTwiceWithDifferentMapping() { @@ -4987,7 +4987,7 @@ public void testSimpleAggregations() ); } - @DecoupledIgnore(mode=Modes.CANNOT_CONVERT) + @DecoupledIgnore(mode = Modes.CANNOT_CONVERT) @Test public void testGroupByWithSortOnPostAggregationDefault() { @@ -5019,7 +5019,7 @@ public void testGroupByWithSortOnPostAggregationDefault() ); } - @DecoupledIgnore(mode=Modes.CANNOT_CONVERT) + @DecoupledIgnore(mode = Modes.CANNOT_CONVERT) @Test public void testGroupByWithSortOnPostAggregationNoTopNConfig() { @@ -5651,7 +5651,7 @@ public void testCountStarWithNotOfDegenerateFilter() ); } - @DecoupledIgnore(mode=Modes.ERROR_HANDLING) + @DecoupledIgnore(mode = Modes.ERROR_HANDLING) @Test public void testUnplannableQueries() { @@ -5723,7 +5723,7 @@ public void testCountStarWithBoundFilterSimplifyOr() ); } - @DecoupledIgnore(mode=Modes.ERROR_HANDLING) + @DecoupledIgnore(mode = Modes.ERROR_HANDLING) @Test public void testUnplannableTwoExactCountDistincts() { @@ -5736,7 +5736,7 @@ public void testUnplannableTwoExactCountDistincts() ); } - @DecoupledIgnore(mode=Modes.ERROR_HANDLING) + @DecoupledIgnore(mode = Modes.ERROR_HANDLING) @Test public void testUnplannableExactCountDistinctOnSketch() { @@ -8082,7 +8082,7 @@ public void testGroupAndFilterOnTimeFloorWithTimeZone() ); } - @DecoupledIgnore(mode=Modes.PLAN_MISMATCH) + @DecoupledIgnore(mode = Modes.PLAN_MISMATCH) @Test public void testFilterOnCurrentTimestampWithIntervalArithmetic() { @@ -10322,7 +10322,7 @@ public void testGroupByTimeAndOtherDimension() ); } - @DecoupledIgnore(mode=Modes.PLAN_MISMATCH) + @DecoupledIgnore(mode = Modes.PLAN_MISMATCH) @Test public void testGroupByTimeFloorAndDimOnGroupByTimeFloorAndDim() { @@ -11489,7 +11489,7 @@ public void testSortProjectAfterNestedGroupBy() ); } - @DecoupledIgnore(mode=Modes.CANNOT_CONVERT) + @DecoupledIgnore(mode = Modes.CANNOT_CONVERT) @Test public void testPostAggWithTimeseries() { @@ -11977,7 +11977,7 @@ public void testRequireTimeConditionSubQueryNegative() ); } - @DecoupledIgnore(mode=Modes.ERROR_HANDLING) + @DecoupledIgnore(mode = Modes.ERROR_HANDLING) @Test public void testRequireTimeConditionSemiJoinNegative() { From ccdaca5a5edbba22528f213ecace04b71750e4c7 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 16:24:24 +0000 Subject: [PATCH 08/16] fixes --- .../druid/sql/calcite/DecoupledIgnore.java | 29 +++++++++------ .../DecoupledPlanningCalciteQueryTest.java | 36 ++++++++----------- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java index 754de1233f9c..f36be0eb84e3 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java @@ -7,13 +7,14 @@ * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.org/licenses/LICENSE-2.0 * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. */ package org.apache.druid.sql.calcite; @@ -24,27 +25,35 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.util.regex.Pattern; +/** + * Can be used to mark tests which are not-yet supported in decoupled mode. + */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.METHOD}) public @interface DecoupledIgnore { - static enum Modes + enum Modes { PLAN_MISMATCH(AssertionError.class, "AssertionError: query #"), NOT_ENOUGH_RULES(DruidException.class, "not enough rules"), CANNOT_CONVERT(DruidException.class, "Cannot convert query parts"), - ERROR_HANDLING(AssertionError.class, "(is was |is was |with message a string containing)"), - ; + ERROR_HANDLING(AssertionError.class, "(is was |is was |with message a string containing)"); public Class throwableClass; public String regex; - Modes(Class cl,String regex) + Modes(Class cl, String regex) { this.throwableClass = cl; this.regex = regex; } + + Pattern getPattern() + { + return Pattern.compile(regex); + } }; Modes mode() default Modes.NOT_ENOUGH_RULES; 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 12bf8086d1ef..cbe662e42d39 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 @@ -32,7 +32,6 @@ import org.junit.runners.model.Statement; import java.util.regex.Matcher; -import java.util.regex.Pattern; import static org.junit.Assert.assertThrows; @@ -47,37 +46,32 @@ public static class DecoupledIgnoreProcessor implements TestRule public Statement apply(Statement base, Description description) { DecoupledIgnore annotation = description.getAnnotation(DecoupledIgnore.class); + if (annotation == null) { + return base; + } return new Statement() { @Override public void evaluate() throws Throwable { - if (annotation != null) { - Throwable e = assertThrows( - "Expected that this testcase will fail - it might got fixed?", - annotation.mode().throwableClass, - () -> { - base.evaluate(); - }); + Throwable e = assertThrows( + "Expected that this testcase will fail - it might got fixed?", + annotation.mode().throwableClass, + () -> { + base.evaluate(); + }); - String regex = annotation.mode().regex; - String trace = Throwables.getStackTraceAsString(e); - Pattern pat = Pattern.compile(regex); - Matcher m=pat.matcher(trace); + String trace = Throwables.getStackTraceAsString(e); + Matcher m = annotation.mode().getPattern().matcher(trace); - if (!m.find()) { -// if(!Throwables.getStackTraceAsString(e).matches(regex)) { - throw new AssertionError("Exception stactrace doesn't match regex: " + regex,e); -// } - } - throw new AssumptionViolatedException("Test is not-yet supported in Decoupled mode"); - } else { - base.evaluate(); + if (!m.find()) { + throw new AssertionError("Exception stactrace doesn't match regex: " + annotation.mode().regex, e); } + throw new AssumptionViolatedException("Test is not-yet supported in Decoupled mode"); } }; } - }; + } private static final ImmutableMap CONTEXT_OVERRIDES = ImmutableMap.of( PlannerConfig.CTX_NATIVE_QUERY_SQL_PLANNING_MODE, PlannerConfig.NATIVE_QUERY_SQL_PLANNING_MODE_DECOUPLED, From 8e0989ecbddce820c50529d7f275677a28b9097b Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Mon, 18 Sep 2023 16:24:51 +0000 Subject: [PATCH 09/16] small updates --- .../sql/calcite/DecoupledPlanningCalciteQueryTest.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 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 cbe662e42d39..fb44c3a141af 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 @@ -57,9 +57,8 @@ public void evaluate() throws Throwable Throwable e = assertThrows( "Expected that this testcase will fail - it might got fixed?", annotation.mode().throwableClass, - () -> { - base.evaluate(); - }); + base::evaluate + ); String trace = Throwables.getStackTraceAsString(e); Matcher m = annotation.mode().getPattern().matcher(trace); @@ -90,7 +89,7 @@ public SqlTestFramework.PlannerFixture plannerFixture(PlannerConfig plannerConfi return queryFramework().plannerFixture(DecoupledPlanningCalciteQueryTest.this, plannerConfig, authConfig); } }) - .cannotVectorize(cannotVectorize) - .skipVectorize(skipVectorize); + .cannotVectorize(cannotVectorize) + .skipVectorize(skipVectorize); } } From 0be39d9e88b0ef63c9456a0e39e73a2006a7007b Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Tue, 19 Sep 2023 11:13:12 +0000 Subject: [PATCH 10/16] updates/etc --- .../druid/sql/calcite/DecoupledIgnore.java | 46 ++++++++++++++++++- .../DecoupledPlanningCalciteQueryTest.java | 41 +---------------- 2 files changed, 46 insertions(+), 41 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java index f36be0eb84e3..d552c170b230 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java @@ -19,21 +19,34 @@ package org.apache.druid.sql.calcite; +import com.google.common.base.Throwables; import org.apache.druid.error.DruidException; +import org.junit.AssumptionViolatedException; +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.util.regex.Matcher; import java.util.regex.Pattern; +import static org.junit.Assert.assertThrows; + /** * Can be used to mark tests which are not-yet supported in decoupled mode. + * + * In case a testcase marked with this annotation fails - it may mean that the + * testcase no longer needs that annotation. */ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.METHOD}) public @interface DecoupledIgnore { + Modes mode() default Modes.NOT_ENOUGH_RULES; + enum Modes { PLAN_MISMATCH(AssertionError.class, "AssertionError: query #"), @@ -56,6 +69,37 @@ Pattern getPattern() } }; - Modes mode() default Modes.NOT_ENOUGH_RULES; + + public static class DecoupledIgnoreProcessor implements TestRule + { + public Statement apply(Statement base, Description description) + { + DecoupledIgnore annotation = description.getAnnotation(DecoupledIgnore.class); + if (annotation == null) { + return base; + } + return new Statement() + { + @Override + public void evaluate() throws Throwable + { + Throwable e = assertThrows( + "Expected that this testcase will fail - it might got fixed?", + annotation.mode().throwableClass, + base::evaluate + ); + + String trace = Throwables.getStackTraceAsString(e); + Matcher m = annotation.mode().getPattern().matcher(trace); + + if (!m.find()) { + throw new AssertionError("Exception stactrace doesn't match regex: " + annotation.mode().regex, e); + } + throw new AssumptionViolatedException("Test is not-yet supported in Decoupled mode"); + } + }; + } + } + } 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 fb44c3a141af..c07a9b4e0d48 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 @@ -19,21 +19,13 @@ package org.apache.druid.sql.calcite; -import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import org.apache.druid.query.QueryContexts; import org.apache.druid.server.security.AuthConfig; +import org.apache.druid.sql.calcite.DecoupledIgnore.DecoupledIgnoreProcessor; import org.apache.druid.sql.calcite.planner.PlannerConfig; import org.apache.druid.sql.calcite.util.SqlTestFramework; -import org.junit.AssumptionViolatedException; import org.junit.Rule; -import org.junit.rules.TestRule; -import org.junit.runner.Description; -import org.junit.runners.model.Statement; - -import java.util.regex.Matcher; - -import static org.junit.Assert.assertThrows; public class DecoupledPlanningCalciteQueryTest extends CalciteQueryTest { @@ -41,37 +33,6 @@ public class DecoupledPlanningCalciteQueryTest extends CalciteQueryTest @Rule(order = 0) public DecoupledIgnoreProcessor decoupledIgnoreProcessor = new DecoupledIgnoreProcessor(); - public static class DecoupledIgnoreProcessor implements TestRule - { - public Statement apply(Statement base, Description description) - { - DecoupledIgnore annotation = description.getAnnotation(DecoupledIgnore.class); - if (annotation == null) { - return base; - } - return new Statement() - { - @Override - public void evaluate() throws Throwable - { - Throwable e = assertThrows( - "Expected that this testcase will fail - it might got fixed?", - annotation.mode().throwableClass, - base::evaluate - ); - - String trace = Throwables.getStackTraceAsString(e); - Matcher m = annotation.mode().getPattern().matcher(trace); - - if (!m.find()) { - throw new AssertionError("Exception stactrace doesn't match regex: " + annotation.mode().regex, e); - } - throw new AssumptionViolatedException("Test is not-yet supported in Decoupled mode"); - } - }; - } - } - private static final ImmutableMap CONTEXT_OVERRIDES = ImmutableMap.of( PlannerConfig.CTX_NATIVE_QUERY_SQL_PLANNING_MODE, PlannerConfig.NATIVE_QUERY_SQL_PLANNING_MODE_DECOUPLED, QueryContexts.ENABLE_DEBUG, true); From 567f61d4a5b3afebbb8ed0e224c9fb12e8c3579c Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Tue, 19 Sep 2023 11:16:12 +0000 Subject: [PATCH 11/16] add apidoc --- .../java/org/apache/druid/sql/calcite/DecoupledIgnore.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java index d552c170b230..0838b596608a 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java @@ -69,7 +69,12 @@ Pattern getPattern() } }; - + /** + * Processes {@link DecoupledIgnore} annotations. + * + * Ensures that test cases disabled with that annotation can still not pass. + * If the error is as expected; the testcase is marked as "ignored". + */ public static class DecoupledIgnoreProcessor implements TestRule { public Statement apply(Statement base, Description description) From ed31174afa0377cc945690f2b5f129ac7284e98b Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Tue, 19 Sep 2023 12:30:05 +0000 Subject: [PATCH 12/16] add missing override --- .../test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java | 1 + 1 file changed, 1 insertion(+) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java index 0838b596608a..e2d43f0a6dbd 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java @@ -77,6 +77,7 @@ Pattern getPattern() */ public static class DecoupledIgnoreProcessor implements TestRule { + @Override public Statement apply(Statement base, Description description) { DecoupledIgnore annotation = description.getAnnotation(DecoupledIgnore.class); From 21e6140caff573bf5f4bafe1d9417b2dc0770f0e Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Tue, 19 Sep 2023 12:42:33 +0000 Subject: [PATCH 13/16] empty From e1a294f8fbe88f6a1ad45878691f6851db609a8b Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Wed, 20 Sep 2023 06:35:18 +0000 Subject: [PATCH 14/16] fix intellijwarning --- .../java/org/apache/druid/sql/calcite/DecoupledIgnore.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java index e2d43f0a6dbd..0c30432d3d67 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/DecoupledIgnore.java @@ -75,7 +75,7 @@ Pattern getPattern() * Ensures that test cases disabled with that annotation can still not pass. * If the error is as expected; the testcase is marked as "ignored". */ - public static class DecoupledIgnoreProcessor implements TestRule + class DecoupledIgnoreProcessor implements TestRule { @Override public Statement apply(Statement base, Description description) @@ -87,7 +87,7 @@ public Statement apply(Statement base, Description description) return new Statement() { @Override - public void evaluate() throws Throwable + public void evaluate() { Throwable e = assertThrows( "Expected that this testcase will fail - it might got fixed?", From ee2c6c8c1419a242653a22462334e67a922d4efd Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Wed, 20 Sep 2023 19:21:59 +0000 Subject: [PATCH 15/16] remove annot --- .../test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java | 1 - 1 file changed, 1 deletion(-) 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 00c022ffbdb7..718406fa9ea3 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 @@ -5719,7 +5719,6 @@ public void testUnplannableExactCountDistinctOnSketch() ); } - @DecoupledIgnore(mode = Modes.ERROR_HANDLING) @Test public void testIsNotDistinctFromLiteral() { From ecd4979f985ec6940b82fcc562437dc6fb3db456 Mon Sep 17 00:00:00 2001 From: Zoltan Haindrich Date: Wed, 20 Sep 2023 19:31:39 +0000 Subject: [PATCH 16/16] ignore --- .../test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java | 1 + 1 file changed, 1 insertion(+) 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 718406fa9ea3..b49dea3e3554 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 @@ -6757,6 +6757,7 @@ public void testExactCountDistinctWithGroupingAndOtherAggregators() ); } + @DecoupledIgnore(mode = Modes.NOT_ENOUGH_RULES) @Test public void testMultipleExactCountDistinctWithGroupingAndOtherAggregatorsUsingJoin() {