diff --git a/core/src/main/java/org/apache/druid/math/expr/ApplyFunction.java b/core/src/main/java/org/apache/druid/math/expr/ApplyFunction.java index f50fe8eb4b42..7aea6f4fb0fd 100644 --- a/core/src/main/java/org/apache/druid/math/expr/ApplyFunction.java +++ b/core/src/main/java/org/apache/druid/math/expr/ApplyFunction.java @@ -105,10 +105,10 @@ ExprEval applyMap(LambdaExpr expr, IndexableMapLambdaObjectBinding bindings) stringsOut[i] = evaluated.asString(); break; case LONG: - longsOut[i] = evaluated.asLong(); + longsOut[i] = evaluated.isNumericNull() ? null : evaluated.asLong(); break; case DOUBLE: - doublesOut[i] = evaluated.asDouble(); + doublesOut[i] = evaluated.isNumericNull() ? null : evaluated.asDouble(); break; } } diff --git a/core/src/main/java/org/apache/druid/math/expr/ExprEval.java b/core/src/main/java/org/apache/druid/math/expr/ExprEval.java index 1cafb17a750c..317afdb8ddc8 100644 --- a/core/src/main/java/org/apache/druid/math/expr/ExprEval.java +++ b/core/src/main/java/org/apache/druid/math/expr/ExprEval.java @@ -26,7 +26,6 @@ import javax.annotation.Nullable; import java.util.Arrays; -import java.util.stream.Collectors; /** * Generic result holder for evaluated {@link Expr} containing the value and {@link ExprType} of the value to allow @@ -629,7 +628,7 @@ public ExprType type() @Override public String[] asStringArray() { - return value == null ? null : Arrays.stream(value).map(String::valueOf).toArray(String[]::new); + return value == null ? null : Arrays.stream(value).map(x -> x != null ? x.toString() : null).toArray(String[]::new); } @Nullable @@ -653,8 +652,6 @@ public ExprEval castTo(ExprType castTo) return StringExprEval.OF_NULL; } switch (castTo) { - case STRING: - return ExprEval.of(Arrays.stream(value).map(String::valueOf).collect(Collectors.joining(", "))); case LONG_ARRAY: return this; case DOUBLE_ARRAY: @@ -690,7 +687,7 @@ public ExprType type() @Override public String[] asStringArray() { - return value == null ? null : Arrays.stream(value).map(String::valueOf).toArray(String[]::new); + return value == null ? null : Arrays.stream(value).map(x -> x != null ? x.toString() : null).toArray(String[]::new); } @Nullable @@ -714,8 +711,6 @@ public ExprEval castTo(ExprType castTo) return StringExprEval.OF_NULL; } switch (castTo) { - case STRING: - return ExprEval.of(Arrays.stream(value).map(String::valueOf).collect(Collectors.joining(", "))); case LONG_ARRAY: return ExprEval.ofLongArray(asLongArray()); case DOUBLE_ARRAY: @@ -788,8 +783,6 @@ public ExprEval castTo(ExprType castTo) return StringExprEval.OF_NULL; } switch (castTo) { - case STRING: - return ExprEval.of(Arrays.stream(value).map(String::valueOf).collect(Collectors.joining(", "))); case STRING_ARRAY: return this; case LONG_ARRAY: diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java index c948bd0d183b..0ef0b48ee79f 100644 --- a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java +++ b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java @@ -107,8 +107,13 @@ public boolean isNull() public Object getObject() { // No need for null check on getObject() since baseSelector impls will never return null. - //noinspection ConstantConditions - return baseSelector.getObject().value(); + ExprEval eval = baseSelector.getObject(); + if (eval.isArray()) { + return Arrays.stream(eval.asStringArray()) + .map(NullHandling::emptyToNullIfNeeded) + .collect(Collectors.toList()); + } + return eval.value(); } @Override @@ -498,7 +503,7 @@ static Supplier supplierFromObjectSelector(final BaseObjectColumnValueSe */ private static Object coerceListDimToStringArray(List val) { - Object[] arrayVal = val.stream().map(Object::toString).toArray(String[]::new); + Object[] arrayVal = val.stream().map(x -> x != null ? x.toString() : x).toArray(String[]::new); if (arrayVal.length > 0) { return arrayVal; } diff --git a/processing/src/test/java/org/apache/druid/query/MultiValuedDimensionTest.java b/processing/src/test/java/org/apache/druid/query/MultiValuedDimensionTest.java index f0282c6ed7b0..199019b776ae 100644 --- a/processing/src/test/java/org/apache/druid/query/MultiValuedDimensionTest.java +++ b/processing/src/test/java/org/apache/druid/query/MultiValuedDimensionTest.java @@ -571,6 +571,52 @@ public void testGroupByExpressionMultiMultiAutoAuto() TestHelper.assertExpectedObjects(expectedResults, result.toList(), "expr-multi-multi-auto-auto"); } + @Test + public void testGroupByExpressionMultiMultiAutoAutoWithFilter() + { + if (config.getDefaultStrategy().equals(GroupByStrategySelector.STRATEGY_V1)) { + expectedException.expect(RuntimeException.class); + expectedException.expectMessage("GroupBy v1 does not support dimension selectors with unknown cardinality."); + } + GroupByQuery query = GroupByQuery + .builder() + .setDataSource("xx") + .setQuerySegmentSpec(new LegacySegmentSpec("1970/3000")) + .setGranularity(Granularities.ALL) + .setDimensions(new DefaultDimensionSpec("texpr", "texpr")) + .setVirtualColumns( + new ExpressionVirtualColumn( + "texpr", + "concat(tags, othertags)", + ValueType.STRING, + TestExprMacroTable.INSTANCE + ) + ) + .setDimFilter(new SelectorDimFilter("texpr", "t1u1", null)) + .setLimit(5) + .setAggregatorSpecs(new CountAggregatorFactory("count")) + .setContext(context) + .build(); + + Sequence result = helper.runQueryOnSegmentsObjs( + ImmutableList.of( + new QueryableIndexSegment(queryableIndex, SegmentId.dummy("sid1")), + new IncrementalIndexSegment(incrementalIndex, SegmentId.dummy("sid2")) + ), + query + ); + + List expectedResults = Arrays.asList( + GroupByQueryRunnerTestHelper.createExpectedRow("1970-01-01T00:00:00.000Z", "texpr", "t1u1", "count", 2L), + GroupByQueryRunnerTestHelper.createExpectedRow("1970-01-01T00:00:00.000Z", "texpr", "t1u2", "count", 2L), + GroupByQueryRunnerTestHelper.createExpectedRow("1970-01-01T00:00:00.000Z", "texpr", "t2u1", "count", 2L), + GroupByQueryRunnerTestHelper.createExpectedRow("1970-01-01T00:00:00.000Z", "texpr", "t2u2", "count", 2L), + GroupByQueryRunnerTestHelper.createExpectedRow("1970-01-01T00:00:00.000Z", "texpr", "t3u1", "count", 2L) + ); + + TestHelper.assertExpectedObjects(expectedResults, result.toList(), "expr-multi-multi-auto-auto"); + } + @Test public void testGroupByExpressionAuto() { diff --git a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVirtualColumnTest.java b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVirtualColumnTest.java index 66223bed66e8..a29cc6ea1549 100644 --- a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVirtualColumnTest.java +++ b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVirtualColumnTest.java @@ -46,6 +46,8 @@ import org.junit.Assert; import org.junit.Test; +import java.util.Arrays; + public class ExpressionVirtualColumnTest { private static final InputRow ROW0 = new MapBasedInputRow( @@ -93,6 +95,15 @@ public class ExpressionVirtualColumnTest "c", ImmutableList.of("7", "8", "9") ) ); + private static final InputRow ROWMULTI3 = new MapBasedInputRow( + DateTimes.of("2000-01-02T01:00:00").getMillis(), + ImmutableList.of(), + ImmutableMap.of( + "x", 3L, + "y", 4L, + "b", Arrays.asList(new String[]{"3", null, "5"}) + ) + ); private static final ExpressionVirtualColumn X_PLUS_Y = new ExpressionVirtualColumn( "expr", @@ -202,12 +213,16 @@ public void testMultiObjectSelector() Assert.assertEquals(ImmutableList.of("2.0", "4.0", "6.0"), selectorImplicit.getObject()); CURRENT_ROW.set(ROWMULTI2); Assert.assertEquals(ImmutableList.of("6.0", "8.0", "10.0"), selectorImplicit.getObject()); + CURRENT_ROW.set(ROWMULTI3); + Assert.assertEquals(Arrays.asList("6.0", NullHandling.replaceWithDefault() ? "0.0" : null, "10.0"), selectorImplicit.getObject()); final BaseObjectColumnValueSelector selectorExplicit = SCALE_LIST_EXPLICIT.makeDimensionSelector(spec, COLUMN_SELECTOR_FACTORY); CURRENT_ROW.set(ROWMULTI); Assert.assertEquals(ImmutableList.of("2.0", "4.0", "6.0"), selectorExplicit.getObject()); CURRENT_ROW.set(ROWMULTI2); Assert.assertEquals(ImmutableList.of("6.0", "8.0", "10.0"), selectorExplicit.getObject()); + CURRENT_ROW.set(ROWMULTI3); + Assert.assertEquals(Arrays.asList("6.0", NullHandling.replaceWithDefault() ? "0.0" : null, "10.0"), selectorExplicit.getObject()); } @Test 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 1f1e0b4dd318..cc7a924342c0 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 @@ -7945,4 +7945,144 @@ public void testTimestampCeil() throws Exception ); } + + @Test + public void testMultiValueStringWorksLikeStringGroupBy() throws Exception + { + List expected; + if (NullHandling.replaceWithDefault()) { + expected = ImmutableList.of( + new Object[]{"foo", 3L}, + new Object[]{"bfoo", 2L}, + new Object[]{"afoo", 1L}, + new Object[]{"cfoo", 1L}, + new Object[]{"dfoo", 1L} + ); + } else { + expected = ImmutableList.of( + new Object[]{null, 2L}, + new Object[]{"bfoo", 2L}, + new Object[]{"afoo", 1L}, + new Object[]{"cfoo", 1L}, + new Object[]{"dfoo", 1L}, + new Object[]{"foo", 1L} + ); + } + testQuery( + "SELECT concat(dim3, 'foo'), SUM(cnt) FROM druid.numfoo GROUP BY 1 ORDER BY 2 DESC", + ImmutableList.of( + GroupByQuery.builder() + .setDataSource(CalciteTests.DATASOURCE3) + .setInterval(querySegmentSpec(Filtration.eternity())) + .setGranularity(Granularities.ALL) + .setVirtualColumns(expressionVirtualColumn("v0", "concat(\"dim3\",'foo')", ValueType.STRING)) + .setDimensions( + dimensions( + new DefaultDimensionSpec("v0", "v0", ValueType.STRING) + ) + ) + .setAggregatorSpecs(aggregators(new LongSumAggregatorFactory("a0", "cnt"))) + .setLimitSpec(new DefaultLimitSpec( + ImmutableList.of(new OrderByColumnSpec( + "a0", + Direction.DESCENDING, + StringComparators.NUMERIC + )), + Integer.MAX_VALUE + )) + .setContext(QUERY_CONTEXT_DEFAULT) + .build() + ), + expected + ); + } + + @Test + public void testMultiValueStringWorksLikeStringGroupByWithFilter() throws Exception + { + testQuery( + "SELECT concat(dim3, 'foo'), SUM(cnt) FROM druid.numfoo where concat(dim3, 'foo') = 'bfoo' GROUP BY 1 ORDER BY 2 DESC", + ImmutableList.of( + GroupByQuery.builder() + .setDataSource(CalciteTests.DATASOURCE3) + .setInterval(querySegmentSpec(Filtration.eternity())) + .setGranularity(Granularities.ALL) + .setVirtualColumns(expressionVirtualColumn("v0", "concat(\"dim3\",'foo')", ValueType.STRING)) + .setDimensions( + dimensions( + new DefaultDimensionSpec("v0", "v0", ValueType.STRING) + ) + ) + .setDimFilter(selector("v0", "bfoo", null)) + .setAggregatorSpecs(aggregators(new LongSumAggregatorFactory("a0", "cnt"))) + .setLimitSpec(new DefaultLimitSpec( + ImmutableList.of(new OrderByColumnSpec( + "a0", + Direction.DESCENDING, + StringComparators.NUMERIC + )), + Integer.MAX_VALUE + )) + .setContext(QUERY_CONTEXT_DEFAULT) + .build() + ), + ImmutableList.of( + new Object[]{"bfoo", 2L}, + new Object[]{"afoo", 1L}, + new Object[]{"cfoo", 1L} + ) + ); + } + + @Test + public void testMultiValueStringWorksLikeStringScan() throws Exception + { + final String nullVal = NullHandling.replaceWithDefault() ? "[\"foo\"]" : "[null]"; + testQuery( + "SELECT concat(dim3, 'foo') FROM druid.numfoo", + ImmutableList.of( + new Druids.ScanQueryBuilder() + .dataSource(CalciteTests.DATASOURCE3) + .intervals(querySegmentSpec(Filtration.eternity())) + .virtualColumns(expressionVirtualColumn("v0", "concat(\"dim3\",'foo')", ValueType.STRING)) + .columns(ImmutableList.of("v0")) + .context(QUERY_CONTEXT_DEFAULT) + .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .legacy(false) + .build() + ), + ImmutableList.of( + new Object[]{"[\"afoo\",\"bfoo\"]"}, + new Object[]{"[\"bfoo\",\"cfoo\"]"}, + new Object[]{"[\"dfoo\"]"}, + new Object[]{"[\"foo\"]"}, + new Object[]{nullVal}, + new Object[]{nullVal} + ) + ); + } + + @Test + public void testMultiValueStringWorksLikeStringScanWithFilter() throws Exception + { + testQuery( + "SELECT concat(dim3, 'foo') FROM druid.numfoo where concat(dim3, 'foo') = 'bfoo'", + ImmutableList.of( + new Druids.ScanQueryBuilder() + .dataSource(CalciteTests.DATASOURCE3) + .intervals(querySegmentSpec(Filtration.eternity())) + .virtualColumns(expressionVirtualColumn("v0", "concat(\"dim3\",'foo')", ValueType.STRING)) + .filters(selector("v0", "bfoo", null)) + .columns(ImmutableList.of("v0")) + .context(QUERY_CONTEXT_DEFAULT) + .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST) + .legacy(false) + .build() + ), + ImmutableList.of( + new Object[]{"[\"afoo\",\"bfoo\"]"}, + new Object[]{"[\"bfoo\",\"cfoo\"]"} + ) + ); + } } diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java b/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java index cc4e935c311a..265c6b26d4df 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/util/CalciteTests.java @@ -20,8 +20,6 @@ package org.apache.druid.sql.calcite.util; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -87,7 +85,6 @@ import org.apache.druid.query.scan.ScanQueryQueryToolChest; import org.apache.druid.query.scan.ScanQueryRunnerFactory; import org.apache.druid.query.select.SelectQuery; -import org.apache.druid.query.select.SelectQueryConfig; import org.apache.druid.query.select.SelectQueryEngine; import org.apache.druid.query.select.SelectQueryQueryToolChest; import org.apache.druid.query.select.SelectQueryRunnerFactory; @@ -218,9 +215,6 @@ public AuthenticationResult createEscalatedAuthenticationResult() ); private static final String TIMESTAMP_COLUMN = "t"; - private static final Supplier SELECT_CONFIG_SUPPLIER = Suppliers.ofInstance( - new SelectQueryConfig(true) - ); private static final Injector INJECTOR = Guice.createInjector( (Module) binder -> {