diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/NestedFieldVirtualColumn.java b/processing/src/main/java/org/apache/druid/segment/virtual/NestedFieldVirtualColumn.java index c7ef30e883b8..486448c2e91d 100644 --- a/processing/src/main/java/org/apache/druid/segment/virtual/NestedFieldVirtualColumn.java +++ b/processing/src/main/java/org/apache/druid/segment/virtual/NestedFieldVirtualColumn.java @@ -1120,7 +1120,7 @@ public ColumnIndexSupplier getIndexSupplier( final Set types = nestedColumn.getColumnTypes(parts); // if the expected output type is numeric but not all of the input types are numeric, we might have additional // null values than what the null value bitmap is tracking, wrap it - if (expectedType.isNumeric() && types.stream().anyMatch(t -> !t.isNumeric())) { + if (expectedType.isNumeric() && (types == null || types.stream().anyMatch(t -> !t.isNumeric()))) { return NoIndexesColumnIndexSupplier.getInstance(); } } diff --git a/processing/src/test/java/org/apache/druid/query/NestedDataTestUtils.java b/processing/src/test/java/org/apache/druid/query/NestedDataTestUtils.java index 1d8b9a0e6554..07ccfbd853e1 100644 --- a/processing/src/test/java/org/apache/druid/query/NestedDataTestUtils.java +++ b/processing/src/test/java/org/apache/druid/query/NestedDataTestUtils.java @@ -76,6 +76,11 @@ public class NestedDataTestUtils public static final String TYPES_DATA_FILE = "nested-types-test-data.json"; public static final String ARRAY_TYPES_DATA_FILE = "nested-array-test-data.json"; + public static final String INCREMENTAL_SEGMENTS_NAME = "incremental"; + public static final String DEFAULT_SEGMENTS_NAME = "segments"; + public static final String FRONT_CODED_SEGMENTS_NAME = "segments-frontcoded"; + public static final String MIX_SEGMENTS_NAME = "mixed"; + public static final ObjectMapper JSON_MAPPER; public static final TimestampSpec TIMESTAMP_SPEC = new TimestampSpec("timestamp", null, null); @@ -519,7 +524,7 @@ public List apply(TemporaryFolder tempFolder, Closer closer) @Override public String toString() { - return "mixed"; + return MIX_SEGMENTS_NAME; } }); segmentsGenerators.add(new BiFunction>() @@ -541,7 +546,7 @@ public List apply(TemporaryFolder tempFolder, Closer closer) @Override public String toString() { - return "incremental"; + return INCREMENTAL_SEGMENTS_NAME; } }); segmentsGenerators.add(new BiFunction>() @@ -577,7 +582,7 @@ public List apply(TemporaryFolder tempFolder, Closer closer) @Override public String toString() { - return "segments"; + return DEFAULT_SEGMENTS_NAME; } }); segmentsGenerators.add(new BiFunction>() @@ -621,9 +626,14 @@ public List apply(TemporaryFolder tempFolder, Closer closer) @Override public String toString() { - return "segments-frontcoded"; + return FRONT_CODED_SEGMENTS_NAME; } }); return segmentsGenerators; } + + public static boolean expectSegmentGeneratorCanVectorize(String name) + { + return DEFAULT_SEGMENTS_NAME.equals(name) || FRONT_CODED_SEGMENTS_NAME.equals(name); + } } diff --git a/processing/src/test/java/org/apache/druid/query/groupby/NestedDataGroupByQueryTest.java b/processing/src/test/java/org/apache/druid/query/groupby/NestedDataGroupByQueryTest.java index c3a155277045..dae2363fdd96 100644 --- a/processing/src/test/java/org/apache/druid/query/groupby/NestedDataGroupByQueryTest.java +++ b/processing/src/test/java/org/apache/druid/query/groupby/NestedDataGroupByQueryTest.java @@ -34,6 +34,7 @@ import org.apache.druid.query.aggregation.LongSumAggregatorFactory; import org.apache.druid.query.dimension.DefaultDimensionSpec; import org.apache.druid.query.expression.TestExprMacroTable; +import org.apache.druid.query.filter.Filter; import org.apache.druid.query.filter.InDimFilter; import org.apache.druid.query.filter.SelectorDimFilter; import org.apache.druid.query.groupby.strategy.GroupByStrategySelector; @@ -56,6 +57,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.function.BiFunction; @@ -70,7 +72,6 @@ public class NestedDataGroupByQueryTest extends InitializedNullHandlingTest public final TemporaryFolder tempFolder = new TemporaryFolder(); private final Closer closer; - private final GroupByQueryConfig config; private final QueryContexts.Vectorize vectorize; private final AggregationTestHelper helper; private final BiFunction> segmentsGenerator; @@ -83,7 +84,6 @@ public NestedDataGroupByQueryTest( ) { NestedDataModule.registerHandlersAndSerde(); - this.config = config; this.vectorize = QueryContexts.Vectorize.fromString(vectorize); this.helper = AggregationTestHelper.createGroupByQueryAggregationTestHelper( NestedDataModule.getJacksonModulesList(), @@ -111,9 +111,11 @@ public static Collection constructorFeeder() NestedDataTestUtils.getSegmentGenerators(NestedDataTestUtils.SIMPLE_DATA_FILE); for (GroupByQueryConfig config : GroupByQueryRunnerTest.testConfigs()) { - for (BiFunction> generatorFn : segmentsGenerators) { - for (String vectorize : new String[]{"false", "true", "force"}) { - constructors.add(new Object[]{config, generatorFn, vectorize}); + if (!GroupByStrategySelector.STRATEGY_V1.equals(config.getDefaultStrategy())) { + for (BiFunction> generatorFn : segmentsGenerators) { + for (String vectorize : new String[]{"false", "true", "force"}) { + constructors.add(new Object[]{config, generatorFn, vectorize}); + } } } } @@ -195,9 +197,7 @@ public void testGroupByRegularColumns() new Object[]{"100", null, 100L, 1L, "1", null, 2L}, new Object[]{"hello", null, null, 1L, "1", null, 12L}, new Object[]{"world", null, null, 1L, "1", null, 2L} - ), - "incremental".equals(segmentsName), - true + ) ); } @@ -307,12 +307,48 @@ public void testGroupByNonExistentVirtualColumn() groupQuery, NullHandling.sqlCompatible() ? ImmutableList.of(new Object[]{null, 16L}) - : ImmutableList.of(new Object[]{"foo", 16L}), - true, - false + : ImmutableList.of(new Object[]{"foo", 16L}) ); } + @Test + public void testGroupByNonExistentFilterAsString() + { + GroupByQuery groupQuery = GroupByQuery.builder() + .setDataSource("test_datasource") + .setGranularity(Granularities.ALL) + .setInterval(Intervals.ETERNITY) + .setDimensions(DefaultDimensionSpec.of("v0")) + .setVirtualColumns( + new NestedFieldVirtualColumn("nest", "$.fake", "v0", ColumnType.STRING) + ) + .setDimFilter(new SelectorDimFilter("v0", "1", null)) + .setAggregatorSpecs(new CountAggregatorFactory("count")) + .setContext(getContext()) + .build(); + + runResults(groupQuery, Collections.emptyList()); + } + + @Test + public void testGroupByNonExistentFilterAsNumeric() + { + GroupByQuery groupQuery = GroupByQuery.builder() + .setDataSource("test_datasource") + .setGranularity(Granularities.ALL) + .setInterval(Intervals.ETERNITY) + .setDimensions(DefaultDimensionSpec.of("v0")) + .setVirtualColumns( + new NestedFieldVirtualColumn("nest", "$.fake", "v0", ColumnType.LONG) + ) + .setDimFilter(new SelectorDimFilter("v0", "1", null)) + .setAggregatorSpecs(new CountAggregatorFactory("count")) + .setContext(getContext()) + .build(); + + runResults(groupQuery, Collections.emptyList()); + } + @Test public void testGroupBySomeFieldOnStringColumn() { @@ -390,12 +426,28 @@ public void testGroupBySomeFieldOnStringColumnWithFilterExpectedTypeLong() groupQuery, ImmutableList.of( new Object[]{100L, 2L} - ), - false, - true + ) ); } + @Test + public void testGroupBySomeFieldOnNestedStringColumnWithFilterExpectedTypeLong() + { + GroupByQuery groupQuery = GroupByQuery.builder() + .setDataSource("test_datasource") + .setGranularity(Granularities.ALL) + .setInterval(Intervals.ETERNITY) + .setDimensions(DefaultDimensionSpec.of("v0", ColumnType.LONG)) + .setVirtualColumns(new NestedFieldVirtualColumn("nester", "$.y.a", "v0", ColumnType.LONG)) + .setAggregatorSpecs(new CountAggregatorFactory("count")) + .setContext(getContext()) + .setDimFilter(new SelectorDimFilter("v0", "100", null)) + .build(); + + + runResults(groupQuery, Collections.emptyList()); + } + @Test public void testGroupBySomeFieldOnStringColumnWithFilterExpectedTypeDouble() { @@ -419,9 +471,7 @@ public void testGroupBySomeFieldOnStringColumnWithFilterExpectedTypeDouble() groupQuery, ImmutableList.of( new Object[]{100.0, 2L} - ), - false, - true + ) ); } @@ -448,9 +498,7 @@ public void testGroupBySomeFieldOnStringColumnWithFilterExpectedTypeFloat() groupQuery, ImmutableList.of( new Object[]{100f, 2L} - ), - false, - true + ) ); } @@ -504,9 +552,7 @@ public void testGroupBySomeFieldOnLongColumn() ImmutableList.of( new Object[]{1672531200000L, NullHandling.defaultLongValue(), 8L}, new Object[]{1672617600000L, NullHandling.defaultLongValue(), 8L} - ), - false, - true + ) ); } @@ -529,14 +575,7 @@ public void testGroupBySomeFieldOnLongColumnFilter() .build(); - runResults( - groupQuery, - ImmutableList.of( - new Object[]{1672531200000L, 8L} - ), - false, - true - ); + runResults(groupQuery, ImmutableList.of(new Object[]{1672531200000L, 8L})); } @Test @@ -558,14 +597,7 @@ public void testGroupBySomeFieldOnLongColumnFilterExpectedType() .build(); - runResults( - groupQuery, - ImmutableList.of( - new Object[]{"1672531200000", 8L} - ), - true, - false - ); + runResults(groupQuery, ImmutableList.of(new Object[]{"1672531200000", 8L})); } @Test @@ -589,53 +621,32 @@ public void testGroupBySomeFieldOnLongColumnFilterNil() runResults( groupQuery, - ImmutableList.of(), - false, - true + ImmutableList.of() ); } - private void runResults(GroupByQuery groupQuery, List expectedResults) - { - runResults(groupQuery, expectedResults, false, false); - } - private void runResults( GroupByQuery groupQuery, - List expectedResults, - boolean hasUnknownCardinality, - boolean hasNonStringOutput + List expectedResults ) { + List segments = segmentsGenerator.apply(tempFolder, closer); Supplier> runner = - () -> helper.runQueryOnSegmentsObjs(segmentsGenerator.apply(tempFolder, closer), groupQuery).toList(); - if (GroupByStrategySelector.STRATEGY_V1.equals(config.getDefaultStrategy())) { - if (hasUnknownCardinality) { - Throwable t = Assert.assertThrows(RuntimeException.class, runner::get); - Assert.assertEquals( - "java.lang.UnsupportedOperationException: GroupBy v1 does not support dimension selectors with unknown cardinality.", - t.getMessage() - ); - return; - } - if (hasNonStringOutput) { - Throwable t = Assert.assertThrows(RuntimeException.class, runner::get); - Assert.assertEquals( - "java.lang.UnsupportedOperationException: GroupBy v1 only supports dimensions with an outputType of STRING.", - t.getMessage() - ); - return; - } - } - if (!"segments".equals(segmentsName) && !"segments-frontcoded".equals(segmentsName)) { - if (GroupByStrategySelector.STRATEGY_V1.equals(config.getDefaultStrategy())) { - Throwable t = Assert.assertThrows(RuntimeException.class, runner::get); - Assert.assertEquals( - "java.lang.UnsupportedOperationException: GroupBy v1 does not support dimension selectors with unknown cardinality.", - t.getMessage() - ); - return; - } else if (vectorize == QueryContexts.Vectorize.FORCE) { + () -> helper.runQueryOnSegmentsObjs(segments, groupQuery).toList(); + Filter filter = groupQuery.getFilter() == null ? null : groupQuery.getFilter().toFilter(); + boolean allCanVectorize = segments.stream() + .allMatch( + s -> s.asStorageAdapter() + .canVectorize( + filter, + groupQuery.getVirtualColumns(), + groupQuery.isDescending() + ) + ); + + Assert.assertEquals(NestedDataTestUtils.expectSegmentGeneratorCanVectorize(segmentsName), allCanVectorize); + if (!allCanVectorize) { + if (vectorize == QueryContexts.Vectorize.FORCE) { Throwable t = Assert.assertThrows(RuntimeException.class, runner::get); Assert.assertEquals( "java.util.concurrent.ExecutionException: java.lang.RuntimeException: org.apache.druid.java.util.common.ISE: Cannot vectorize!",