From 3865fed007678783c716e835e4cd91165d748b0b Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Tue, 9 Apr 2024 10:57:53 -0700 Subject: [PATCH 01/11] SQL: Support GROUP BY and ORDER BY for NULL types. Treats SQL NULL types as strings at the native layer. --- .../sql/calcite/expression/Expressions.java | 5 + .../druid/sql/calcite/planner/Calcites.java | 18 ++- .../druid/sql/calcite/CalciteQueryTest.java | 94 ++++++++++++ .../sql/calcite/planner/CalcitesTest.java | 143 ++++++++++++++---- 4 files changed, 229 insertions(+), 31 deletions(-) diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java index 0d3e2505853d..74bff7826872 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java @@ -729,6 +729,11 @@ private static DimFilter toSimpleLeafFilter( // Numeric lhs needs a numeric comparison. final StringComparator comparator = Calcites.getStringComparatorForRelDataType(lhs.getType()); + if (comparator == null) { + // Type is not comparable. + return null; + } + final BoundRefKey boundRefKey = new BoundRefKey(column, extractionFn, comparator); final DimFilter filter; diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java index 64929c6445e1..8b988fcfbd8b 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java @@ -206,7 +206,7 @@ public static ColumnType getValueTypeForRelDataTypeFull(final RelDataType type) return ColumnType.DOUBLE; } else if (isLongType(sqlTypeName)) { return ColumnType.LONG; - } else if (isStringType(sqlTypeName)) { + } else if (isStringType(sqlTypeName) || sqlTypeName == SqlTypeName.NULL) { return ColumnType.STRING; } else if (SqlTypeName.OTHER == sqlTypeName) { if (type instanceof RowSignatures.ComplexSqlType) { @@ -244,12 +244,22 @@ public static boolean isLongType(SqlTypeName sqlTypeName) } /** - * Returns the natural StringComparator associated with the RelDataType + * Returns the natural StringComparator associated with the RelDataType, or null if the type is not convertible to + * {@link ColumnType} by {@link #getColumnTypeForRelDataType(RelDataType)}. */ + @Nullable public static StringComparator getStringComparatorForRelDataType(RelDataType dataType) { - final ColumnType valueType = getColumnTypeForRelDataType(dataType); - return getStringComparatorForValueType(valueType); + if (dataType.getSqlTypeName() == SqlTypeName.NULL) { + return StringComparators.NATURAL; + } else { + final ColumnType valueType = getColumnTypeForRelDataType(dataType); + if (valueType == null) { + return null; + } + + return getStringComparatorForValueType(valueType); + } } /** 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 478648088c32..c770e549dcaf 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 @@ -6108,6 +6108,100 @@ public void testCountStarWithTimeInIntervalFilterLosAngeles() ); } + @Test + public void testGroupByNullType() + { + cannotVectorize(); + testQuery( + "SELECT NULL as nullcol, COUNT(*) FROM druid.foo GROUP BY 1", + ImmutableList.of( + new GroupByQuery.Builder() + .setDataSource(CalciteTests.DATASOURCE1) + .setInterval(querySegmentSpec(Filtration.eternity())) + .setGranularity(Granularities.ALL) + .setVirtualColumns(expressionVirtualColumn("v0", "null", ColumnType.STRING)) + .setDimensions(dimensions(new DefaultDimensionSpec("v0", "d0", ColumnType.STRING))) + .setAggregatorSpecs(aggregators(new CountAggregatorFactory("a0"))) + .setContext(QUERY_CONTEXT_DEFAULT) + .build() + ), + ImmutableList.of( + new Object[]{NullHandling.defaultStringValue(), 6L} + ) + ); + } + + @Test + public void testOrderByNullType() + { + cannotVectorize(); + testQuery( + // Order on subquery, since the native engine doesn't currently support ordering when selecting directly + // from a table. + "SELECT dim1, NULL as nullcol FROM (SELECT DISTINCT dim1 FROM druid.foo LIMIT 1) ORDER BY 2", + ImmutableList.of( + WindowOperatorQueryBuilder + .builder() + .setDataSource( + new TopNQueryBuilder() + .dataSource(CalciteTests.DATASOURCE1) + .intervals(querySegmentSpec(Filtration.eternity())) + .dimension(new DefaultDimensionSpec("dim1", "d0", ColumnType.STRING)) + .threshold(1) + .metric(new DimensionTopNMetricSpec(null, StringComparators.LEXICOGRAPHIC)) + .postAggregators(expressionPostAgg("s0", "null", ColumnType.STRING)) + .context(QUERY_CONTEXT_DEFAULT) + .build() + ) + .setSignature( + RowSignature.builder() + .add("d0", ColumnType.STRING) + .add("s0", ColumnType.STRING) + .build() + ) + .setOperators( + OperatorFactoryBuilders.naiveSortOperator("s0", ColumnWithDirection.Direction.ASC) + ) + .setLeafOperators( + OperatorFactoryBuilders + .scanOperatorFactoryBuilder() + .setOffsetLimit(0, Long.MAX_VALUE) + .setProjectedColumns("d0", "s0") + .build() + ) + .build() + ), + ImmutableList.of( + new Object[]{"", NullHandling.defaultStringValue()} + ) + ); + } + + @Test + public void testGroupByOrderByNullType() + { + // Cannot vectorize due to + cannotVectorize(); + + testQuery( + "SELECT NULL as nullcol, COUNT(*) FROM druid.foo GROUP BY 1 ORDER BY 1", + ImmutableList.of( + new GroupByQuery.Builder() + .setDataSource(CalciteTests.DATASOURCE1) + .setInterval(querySegmentSpec(Filtration.eternity())) + .setGranularity(Granularities.ALL) + .setVirtualColumns(expressionVirtualColumn("v0", "null", ColumnType.STRING)) + .setDimensions(dimensions(new DefaultDimensionSpec("v0", "d0", ColumnType.STRING))) + .setAggregatorSpecs(aggregators(new CountAggregatorFactory("a0"))) + .setContext(QUERY_CONTEXT_DEFAULT) + .build() + ), + ImmutableList.of( + new Object[]{NullHandling.defaultStringValue(), 6L} + ) + ); + } + @Test public void testCountStarWithTimeInIntervalFilterInvalidInterval() { diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/planner/CalcitesTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/planner/CalcitesTest.java index cc033162d08d..0b82ee4f7868 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/planner/CalcitesTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/planner/CalcitesTest.java @@ -20,52 +20,141 @@ package org.apache.druid.sql.calcite.planner; import com.google.common.collect.ImmutableSortedSet; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.sql.type.SqlTypeFamily; +import org.apache.calcite.sql.type.SqlTypeName; import org.apache.druid.query.ordering.StringComparators; import org.apache.druid.segment.column.ColumnType; +import org.apache.druid.sql.calcite.table.RowSignatures; import org.apache.druid.sql.calcite.util.CalciteTestBase; -import org.junit.Assert; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; + public class CalcitesTest extends CalciteTestBase { @Test public void testEscapeStringLiteral() { - Assert.assertEquals("''", Calcites.escapeStringLiteral("")); - Assert.assertEquals("'foo'", Calcites.escapeStringLiteral("foo")); - Assert.assertEquals("'foo bar'", Calcites.escapeStringLiteral("foo bar")); - Assert.assertEquals("U&'foö bar'", Calcites.escapeStringLiteral("foö bar")); - Assert.assertEquals("U&'foo \\0026\\0026 bar'", Calcites.escapeStringLiteral("foo && bar")); - Assert.assertEquals("U&'foo \\005C bar'", Calcites.escapeStringLiteral("foo \\ bar")); - Assert.assertEquals("U&'foo\\0027s bar'", Calcites.escapeStringLiteral("foo's bar")); - Assert.assertEquals("U&'друид'", Calcites.escapeStringLiteral("друид")); + assertEquals("''", Calcites.escapeStringLiteral("")); + assertEquals("'foo'", Calcites.escapeStringLiteral("foo")); + assertEquals("'foo bar'", Calcites.escapeStringLiteral("foo bar")); + assertEquals("U&'foö bar'", Calcites.escapeStringLiteral("foö bar")); + assertEquals("U&'foo \\0026\\0026 bar'", Calcites.escapeStringLiteral("foo && bar")); + assertEquals("U&'foo \\005C bar'", Calcites.escapeStringLiteral("foo \\ bar")); + assertEquals("U&'foo\\0027s bar'", Calcites.escapeStringLiteral("foo's bar")); + assertEquals("U&'друид'", Calcites.escapeStringLiteral("друид")); } @Test public void testFindUnusedPrefix() { - Assert.assertEquals("x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "bar"))); - Assert.assertEquals("x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "bar", "x"))); - Assert.assertEquals("_x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "bar", "x0"))); - Assert.assertEquals("_x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "bar", "x4"))); - Assert.assertEquals("__x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "xa", "_x2xx", "x0"))); - Assert.assertEquals("x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "xa", "_x2xx", " x"))); - Assert.assertEquals("x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "_xbxx"))); - Assert.assertEquals("x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "xa", "_x"))); - Assert.assertEquals("__x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "x1a", "_x90"))); + assertEquals("x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "bar"))); + assertEquals("x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "bar", "x"))); + assertEquals("_x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "bar", "x0"))); + assertEquals("_x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "bar", "x4"))); + assertEquals("__x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "xa", "_x2xx", "x0"))); + assertEquals("x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "xa", "_x2xx", " x"))); + assertEquals("x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "_xbxx"))); + assertEquals("x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "xa", "_x"))); + assertEquals("__x", Calcites.findUnusedPrefixForDigits("x", ImmutableSortedSet.of("foo", "x1a", "_x90"))); + } + + @Test + public void testGetStringComparatorForRelDataType() + { + for (final SqlTypeName typeName : SqlTypeFamily.CHARACTER.getTypeNames()) { + final RelDataType type = DruidTypeSystem.TYPE_FACTORY.createSqlType(typeName); + assertEquals( + StringComparators.LEXICOGRAPHIC, + Calcites.getStringComparatorForRelDataType(type), + type.getFullTypeString() + ); + } + + for (final SqlTypeName typeName : SqlTypeFamily.NUMERIC.getTypeNames()) { + final RelDataType type = DruidTypeSystem.TYPE_FACTORY.createSqlType(typeName); + assertEquals( + StringComparators.NUMERIC, + Calcites.getStringComparatorForRelDataType(type), + type.getFullTypeString() + ); + } + + assertEquals( + StringComparators.NATURAL, + Calcites.getStringComparatorForRelDataType( + RowSignatures.makeComplexType(DruidTypeSystem.TYPE_FACTORY, ColumnType.UNKNOWN_COMPLEX, false) + ), + ColumnType.UNKNOWN_COMPLEX.toString() + ); + + assertEquals( + StringComparators.NATURAL, + Calcites.getStringComparatorForRelDataType( + RowSignatures.makeComplexType(DruidTypeSystem.TYPE_FACTORY, ColumnType.NESTED_DATA, false) + ), + ColumnType.NESTED_DATA.toString() + ); + + final RelDataType timestampType = DruidTypeSystem.TYPE_FACTORY.createSqlType(SqlTypeName.TIMESTAMP); + assertEquals( + StringComparators.NUMERIC, + Calcites.getStringComparatorForRelDataType(timestampType), + timestampType.getFullTypeString() + ); + + final RelDataType dateType = DruidTypeSystem.TYPE_FACTORY.createSqlType(SqlTypeName.DATE); + assertEquals( + StringComparators.NUMERIC, + Calcites.getStringComparatorForRelDataType(dateType), + dateType.getFullTypeString() + ); + + final RelDataType bigintArrayType = + DruidTypeSystem.TYPE_FACTORY.createArrayType( + DruidTypeSystem.TYPE_FACTORY.createSqlType(SqlTypeName.BIGINT), + -1 + ); + assertEquals( + StringComparators.NATURAL, + Calcites.getStringComparatorForRelDataType(bigintArrayType), + bigintArrayType.getFullTypeString() + ); + + final RelDataType booleanType = DruidTypeSystem.TYPE_FACTORY.createSqlType(SqlTypeName.BOOLEAN); + assertEquals( + StringComparators.NUMERIC, + Calcites.getStringComparatorForRelDataType(booleanType), + booleanType.getFullTypeString() + ); + + final RelDataType otherType = DruidTypeSystem.TYPE_FACTORY.createSqlType(SqlTypeName.OTHER); + assertEquals( + StringComparators.NATURAL, + Calcites.getStringComparatorForRelDataType(otherType), + otherType.getFullTypeString() + ); + + final RelDataType nullType = DruidTypeSystem.TYPE_FACTORY.createSqlType(SqlTypeName.NULL); + assertEquals( + StringComparators.NATURAL, + Calcites.getStringComparatorForRelDataType(nullType), + nullType.getFullTypeString() + ); } @Test public void testGetStringComparatorForColumnType() { - Assert.assertEquals(StringComparators.LEXICOGRAPHIC, Calcites.getStringComparatorForValueType(ColumnType.STRING)); - Assert.assertEquals(StringComparators.NUMERIC, Calcites.getStringComparatorForValueType(ColumnType.LONG)); - Assert.assertEquals(StringComparators.NUMERIC, Calcites.getStringComparatorForValueType(ColumnType.FLOAT)); - Assert.assertEquals(StringComparators.NUMERIC, Calcites.getStringComparatorForValueType(ColumnType.DOUBLE)); - Assert.assertEquals(StringComparators.NATURAL, Calcites.getStringComparatorForValueType(ColumnType.STRING_ARRAY)); - Assert.assertEquals(StringComparators.NATURAL, Calcites.getStringComparatorForValueType(ColumnType.LONG_ARRAY)); - Assert.assertEquals(StringComparators.NATURAL, Calcites.getStringComparatorForValueType(ColumnType.DOUBLE_ARRAY)); - Assert.assertEquals(StringComparators.NATURAL, Calcites.getStringComparatorForValueType(ColumnType.NESTED_DATA)); - Assert.assertEquals(StringComparators.NATURAL, Calcites.getStringComparatorForValueType(ColumnType.UNKNOWN_COMPLEX)); + assertEquals(StringComparators.LEXICOGRAPHIC, Calcites.getStringComparatorForValueType(ColumnType.STRING)); + assertEquals(StringComparators.NUMERIC, Calcites.getStringComparatorForValueType(ColumnType.LONG)); + assertEquals(StringComparators.NUMERIC, Calcites.getStringComparatorForValueType(ColumnType.FLOAT)); + assertEquals(StringComparators.NUMERIC, Calcites.getStringComparatorForValueType(ColumnType.DOUBLE)); + assertEquals(StringComparators.NATURAL, Calcites.getStringComparatorForValueType(ColumnType.STRING_ARRAY)); + assertEquals(StringComparators.NATURAL, Calcites.getStringComparatorForValueType(ColumnType.LONG_ARRAY)); + assertEquals(StringComparators.NATURAL, Calcites.getStringComparatorForValueType(ColumnType.DOUBLE_ARRAY)); + assertEquals(StringComparators.NATURAL, Calcites.getStringComparatorForValueType(ColumnType.NESTED_DATA)); + assertEquals(StringComparators.NATURAL, Calcites.getStringComparatorForValueType(ColumnType.UNKNOWN_COMPLEX)); } } From 903079763b61681ba55c4f8b0eae6a90418d5f79 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Tue, 9 Apr 2024 11:44:59 -0700 Subject: [PATCH 02/11] Update tests. --- .../druid/sql/calcite/CalciteSelectQueryTest.java | 2 +- .../druid/sql/calcite/CalciteTableAppendTest.java | 4 +--- .../sql/calcite/DecoupledPlanningCalciteQueryTest.java | 10 ++++++++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java index 48d9518a9687..08d817881dbc 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java @@ -141,7 +141,7 @@ public void testValuesContainingNull() ImmutableList.of(new Object[]{null, "United States"}), RowSignature .builder() - .add("EXPR$0", null) + .add("EXPR$0", ColumnType.STRING) .add("EXPR$1", ColumnType.STRING) .build() ) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteTableAppendTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteTableAppendTest.java index 8b6170cbfaac..4421ceebe8ba 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteTableAppendTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteTableAppendTest.java @@ -46,9 +46,7 @@ public void testUnion() .columns("dim1", "v0") .context(QUERY_CONTEXT_DEFAULT) .resultFormat(ResultFormat.RESULT_FORMAT_COMPACTED_LIST) - .virtualColumns( - expressionVirtualColumn("v0", "null", null) - ) + .virtualColumns(expressionVirtualColumn("v0", "null", ColumnType.STRING)) .legacy(false) .build(), Druids.newScanQueryBuilder() 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 1a0ae448319f..092c5a212f02 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 @@ -26,6 +26,8 @@ import org.apache.druid.sql.calcite.planner.PlannerConfig; import org.apache.druid.sql.calcite.util.SqlTestFramework; import org.apache.druid.sql.calcite.util.SqlTestFramework.PlannerComponentSupplier; +import org.junit.Test; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.extension.ExtendWith; @ExtendWith(NotYetSupportedProcessor.class) @@ -64,4 +66,12 @@ public SqlTestFramework.PlannerFixture plannerFixture(PlannerConfig plannerConfi return builder; } + + @Override + @Test + @Disabled("Unhandled Query Planning Failure") + public void testOrderByNullType() + { + super.testOrderByNullType(); + } } From 231a3dc3fc4f8d72eef2891157454199981616c1 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Tue, 9 Apr 2024 13:41:25 -0700 Subject: [PATCH 03/11] Fixes. --- .../org/apache/druid/sql/calcite/CalciteQueryTest.java | 10 ++++++---- .../sql/calcite/DecoupledPlanningCalciteQueryTest.java | 2 +- 2 files changed, 7 insertions(+), 5 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 c770e549dcaf..94742d477290 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 @@ -6111,6 +6111,7 @@ public void testCountStarWithTimeInIntervalFilterLosAngeles() @Test public void testGroupByNullType() { + // Cannot vectorize due to null constant expression. cannotVectorize(); testQuery( "SELECT NULL as nullcol, COUNT(*) FROM druid.foo GROUP BY 1", @@ -6126,7 +6127,7 @@ public void testGroupByNullType() .build() ), ImmutableList.of( - new Object[]{NullHandling.defaultStringValue(), 6L} + new Object[]{null, 6L} ) ); } @@ -6134,6 +6135,7 @@ public void testGroupByNullType() @Test public void testOrderByNullType() { + // Cannot vectorize due to null constant expression. cannotVectorize(); testQuery( // Order on subquery, since the native engine doesn't currently support ordering when selecting directly @@ -6172,7 +6174,7 @@ public void testOrderByNullType() .build() ), ImmutableList.of( - new Object[]{"", NullHandling.defaultStringValue()} + new Object[]{"", null} ) ); } @@ -6180,7 +6182,7 @@ public void testOrderByNullType() @Test public void testGroupByOrderByNullType() { - // Cannot vectorize due to + // Cannot vectorize due to null constant expression. cannotVectorize(); testQuery( @@ -6197,7 +6199,7 @@ public void testGroupByOrderByNullType() .build() ), ImmutableList.of( - new Object[]{NullHandling.defaultStringValue(), 6L} + new Object[]{null, 6L} ) ); } 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 092c5a212f02..ca6b0b0bb444 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 @@ -26,8 +26,8 @@ import org.apache.druid.sql.calcite.planner.PlannerConfig; import org.apache.druid.sql.calcite.util.SqlTestFramework; import org.apache.druid.sql.calcite.util.SqlTestFramework.PlannerComponentSupplier; -import org.junit.Test; import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @ExtendWith(NotYetSupportedProcessor.class) From 3ae380b11117cfac524afac99c56741abcf56252 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Tue, 9 Apr 2024 14:39:24 -0700 Subject: [PATCH 04/11] Fix tests. --- .../sql/ArrayOfDoublesSketchSqlAggregatorTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/sql/ArrayOfDoublesSketchSqlAggregatorTest.java b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/sql/ArrayOfDoublesSketchSqlAggregatorTest.java index 0ed01f3f4117..6ac61fe11847 100644 --- a/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/sql/ArrayOfDoublesSketchSqlAggregatorTest.java +++ b/extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/tuple/sql/ArrayOfDoublesSketchSqlAggregatorTest.java @@ -386,7 +386,7 @@ public void testNullInputs() ImmutableList.of( new ArrayOfDoublesSketchToMetricsSumEstimatePostAggregator( "p1", - expressionPostAgg("p0", "null", null) + expressionPostAgg("p0", "null", ColumnType.STRING) ), new ArrayOfDoublesSketchSetOpPostAggregator( "p4", @@ -394,8 +394,8 @@ public void testNullInputs() null, null, ImmutableList.of( - expressionPostAgg("p2", "null", null), - expressionPostAgg("p3", "null", null) + expressionPostAgg("p2", "null", ColumnType.STRING), + expressionPostAgg("p3", "null", ColumnType.STRING) ) ), new ArrayOfDoublesSketchSetOpPostAggregator( @@ -404,7 +404,7 @@ public void testNullInputs() null, null, ImmutableList.of( - expressionPostAgg("p5", "null", null), + expressionPostAgg("p5", "null", ColumnType.STRING), new FieldAccessPostAggregator("p6", "a1") ) ), @@ -415,7 +415,7 @@ public void testNullInputs() null, ImmutableList.of( new FieldAccessPostAggregator("p8", "a1"), - expressionPostAgg("p9", "null", null) + expressionPostAgg("p9", "null", ColumnType.STRING) ) ) ) From 9e368f9c2429f68361da91f374b8fc09d7a0130b Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Thu, 11 Apr 2024 15:19:31 -0700 Subject: [PATCH 05/11] Fix tests. --- .../apache/druid/sql/calcite/CalciteQueryTest.java | 14 ++++++++++++++ .../calcite/DecoupledPlanningCalciteQueryTest.java | 3 +-- 2 files changed, 15 insertions(+), 2 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 94742d477290..3da9ecb722ab 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 @@ -6195,6 +6195,20 @@ public void testGroupByOrderByNullType() .setVirtualColumns(expressionVirtualColumn("v0", "null", ColumnType.STRING)) .setDimensions(dimensions(new DefaultDimensionSpec("v0", "d0", ColumnType.STRING))) .setAggregatorSpecs(aggregators(new CountAggregatorFactory("a0"))) + .setLimitSpec( + queryFramework().engine().featureAvailable(EngineFeature.GROUPBY_IMPLICITLY_SORTS) + ? NoopLimitSpec.instance() + : new DefaultLimitSpec( + ImmutableList.of( + new OrderByColumnSpec( + "d0", + Direction.ASCENDING, + StringComparators.NATURAL + ) + ), + Integer.MAX_VALUE + ) + ) .setContext(QUERY_CONTEXT_DEFAULT) .build() ), 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 ca6b0b0bb444..1af3eaeb1437 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 @@ -26,7 +26,6 @@ import org.apache.druid.sql.calcite.planner.PlannerConfig; import org.apache.druid.sql.calcite.util.SqlTestFramework; import org.apache.druid.sql.calcite.util.SqlTestFramework.PlannerComponentSupplier; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -69,7 +68,7 @@ public SqlTestFramework.PlannerFixture plannerFixture(PlannerConfig plannerConfi @Override @Test - @Disabled("Unhandled Query Planning Failure") + @NotYetSupported(NotYetSupported.Modes.NOT_ENOUGH_RULES) public void testOrderByNullType() { super.testOrderByNullType(); From 10b50ddbfb50ac54e5f840061a4645594024534e Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Tue, 14 May 2024 09:55:09 -0700 Subject: [PATCH 06/11] Fix test. --- .../apache/druid/sql/calcite/expression/ExpressionsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java index 86b5d38639be..bc76aeb15125 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/expression/ExpressionsTest.java @@ -2859,7 +2859,7 @@ public void testCalciteLiteralToDruidLiteral() ); assertDruidLiteral( - new DruidLiteral(null, null), + new DruidLiteral(ExpressionType.STRING, null), Expressions.calciteLiteralToDruidLiteral( plannerContext, rexBuilder.makeNullLiteral(rexBuilder.getTypeFactory().createSqlType(SqlTypeName.NULL)) From 725d2e95fd65ac75bc0fceeb6d5887c95c311627 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Wed, 24 Jul 2024 12:15:21 -0700 Subject: [PATCH 07/11] Add back NOT_ENOUGH_RULES. --- .../test/java/org/apache/druid/sql/calcite/NotYetSupported.java | 1 + 1 file changed, 1 insertion(+) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/NotYetSupported.java b/sql/src/test/java/org/apache/druid/sql/calcite/NotYetSupported.java index e5442a2bda24..99809d644d4a 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/NotYetSupported.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/NotYetSupported.java @@ -77,6 +77,7 @@ enum Modes { // @formatter:off + NOT_ENOUGH_RULES(DruidException.class, "There are not enough rules to produce a node"), DISTINCT_AGGREGATE_NOT_SUPPORTED(DruidException.class, "DISTINCT is not supported"), ERROR_HANDLING(AssertionError.class, "targetPersona: is <[A-Z]+> and category: is <[A-Z_]+> and errorCode: is"), EXPRESSION_NOT_GROUPED(DruidException.class, "Expression '[a-z]+' is not being grouped"), From 7fc2b774ef06309108d99a51e6de1c5a99d74903 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Wed, 2 Oct 2024 13:11:59 -0700 Subject: [PATCH 08/11] Fix test. --- .../java/org/apache/druid/sql/calcite/CalciteQueryTest.java | 2 -- 1 file changed, 2 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 6925b73212a8..1b0e5ce8e517 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 @@ -6782,8 +6782,6 @@ public void testGroupByNullType() @Test public void testOrderByNullType() { - // Cannot vectorize due to null constant expression. - cannotVectorize(); testQuery( // Order on subquery, since the native engine doesn't currently support ordering when selecting directly // from a table. From 694b08986df330389e66ae1d4655e018855ba6db Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Tue, 8 Oct 2024 01:13:29 -0700 Subject: [PATCH 09/11] Fix test. --- .../apache/druid/sql/calcite/CalciteArraysQueryTest.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java index f48166211344..e30db8c3712a 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java @@ -7469,7 +7469,13 @@ public void testNullArray() newScanQueryBuilder() .dataSource(CalciteTests.ARRAYS_DATASOURCE) .intervals(querySegmentSpec(Filtration.eternity())) - .virtualColumns(expressionVirtualColumn("v0", "(\"arrayLongNulls\" == array(null,null))", ColumnType.LONG)) + .virtualColumns( + expressionVirtualColumn( + "v0", + "(\"arrayLongNulls\" == CAST(array(null,null), 'ARRAY'))", + ColumnType.LONG + ) + ) .columns("v0") .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) .limit(1) From 46b2c3e8b9da7e21476e22cfb7ba1e07b6fe0337 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Thu, 10 Oct 2024 12:20:00 -0700 Subject: [PATCH 10/11] Update test case. --- .../union_removed_branch_union_nulls.iq | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/union_removed_branch_union_nulls.iq b/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/union_removed_branch_union_nulls.iq index 0711686e9bac..e2c5f9ffbe93 100644 --- a/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/union_removed_branch_union_nulls.iq +++ b/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/union_removed_branch_union_nulls.iq @@ -37,6 +37,7 @@ DruidProject(EXPR$0=[CAST($0):BIGINT], channel=[CAST($1):VARCHAR], druid=[logica "dataSource" : { "type" : "inline", "columnNames" : [ "EXPR$0", "EXPR$1" ], + "columnTypes" : [ "STRING", "STRING" ], "rows" : [ [ null, null ] ] }, "intervals" : { @@ -46,17 +47,12 @@ DruidProject(EXPR$0=[CAST($0):BIGINT], channel=[CAST($1):VARCHAR], druid=[logica "virtualColumns" : [ { "type" : "expression", "name" : "v0", - "expression" : "null", + "expression" : "CAST(\"EXPR$0\", 'LONG')", "outputType" : "LONG" - }, { - "type" : "expression", - "name" : "v1", - "expression" : "null", - "outputType" : "STRING" } ], "resultFormat" : "compactedList", - "columns" : [ "v0", "v1" ], - "columnTypes" : [ "LONG", "STRING" ], + "columns" : [ "EXPR$1", "v0" ], + "columnTypes" : [ "STRING", "LONG" ], "granularity" : { "type" : "all" }, From 9a922cf49476d381c591b1de86ede466a0d4c0dd Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Mon, 3 Feb 2025 21:06:03 -0800 Subject: [PATCH 11/11] Update quidem test. --- .../union_removed_branch_union_nulls.iq | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/union_removed_branch_union_nulls.iq b/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/union_removed_branch_union_nulls.iq index e2c5f9ffbe93..dc714560fed6 100644 --- a/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/union_removed_branch_union_nulls.iq +++ b/sql/src/test/quidem/org.apache.druid.quidem.SqlQuidemTest/union_removed_branch_union_nulls.iq @@ -51,8 +51,8 @@ DruidProject(EXPR$0=[CAST($0):BIGINT], channel=[CAST($1):VARCHAR], druid=[logica "outputType" : "LONG" } ], "resultFormat" : "compactedList", - "columns" : [ "EXPR$1", "v0" ], - "columnTypes" : [ "STRING", "LONG" ], + "columns" : [ "v0", "EXPR$1" ], + "columnTypes" : [ "LONG", "STRING" ], "granularity" : { "type" : "all" },