From 8bbc0e7b3fee1747d1c9dbf9e730b597fc1c3cbf Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 25 Oct 2019 01:04:55 -0700 Subject: [PATCH 01/12] transformSpec + array expressions changes: * added array expression support to transformSpec * removed ParseSpec.verify since its only use afaict was preventing transform expr that did not replace their input from functioning * hijacked index task test to test changes --- .../druid/data/input/impl/CSVParseSpec.java | 9 --- .../data/input/impl/DelimitedParseSpec.java | 9 --- .../input/impl/JSONLowercaseParseSpec.java | 5 -- .../druid/data/input/impl/JSONParseSpec.java | 5 -- .../data/input/impl/JavaScriptParseSpec.java | 5 -- .../druid/data/input/impl/ParseSpec.java | 6 -- .../druid/data/input/impl/RegexParseSpec.java | 12 ---- .../indexing/common/task/IndexTaskTest.java | 64 +++++++++++++++++-- .../transform/ExpressionTransform.java | 14 +++- .../segment/virtual/ExpressionSelectors.java | 23 +++++-- 10 files changed, 87 insertions(+), 65 deletions(-) diff --git a/core/src/main/java/org/apache/druid/data/input/impl/CSVParseSpec.java b/core/src/main/java/org/apache/druid/data/input/impl/CSVParseSpec.java index 61eca2e091e0..5bd9a2b1c320 100644 --- a/core/src/main/java/org/apache/druid/data/input/impl/CSVParseSpec.java +++ b/core/src/main/java/org/apache/druid/data/input/impl/CSVParseSpec.java @@ -57,7 +57,6 @@ public CSVParseSpec( for (String column : columns) { Preconditions.checkArgument(!column.contains(","), "Column[%s] has a comma, it cannot", column); } - verify(dimensionsSpec.getDimensionNames()); } else { Preconditions.checkArgument( hasHeaderRow, @@ -102,14 +101,6 @@ public int getSkipHeaderRows() return skipHeaderRows; } - @Override - public void verify(List usedCols) - { - for (String columnName : usedCols) { - Preconditions.checkArgument(columns.contains(columnName), "column[%s] not in columns.", columnName); - } - } - @Override public Parser makeParser() { diff --git a/core/src/main/java/org/apache/druid/data/input/impl/DelimitedParseSpec.java b/core/src/main/java/org/apache/druid/data/input/impl/DelimitedParseSpec.java index f88b13260029..5940e70e11fd 100644 --- a/core/src/main/java/org/apache/druid/data/input/impl/DelimitedParseSpec.java +++ b/core/src/main/java/org/apache/druid/data/input/impl/DelimitedParseSpec.java @@ -60,7 +60,6 @@ public DelimitedParseSpec( for (String column : this.columns) { Preconditions.checkArgument(!column.contains(","), "Column[%s] has a comma, it cannot", column); } - verify(dimensionsSpec.getDimensionNames()); } else { Preconditions.checkArgument( hasHeaderRow, @@ -112,14 +111,6 @@ public int getSkipHeaderRows() return skipHeaderRows; } - @Override - public void verify(List usedCols) - { - for (String columnName : usedCols) { - Preconditions.checkArgument(columns.contains(columnName), "column[%s] not in columns.", columnName); - } - } - @Override public Parser makeParser() { diff --git a/core/src/main/java/org/apache/druid/data/input/impl/JSONLowercaseParseSpec.java b/core/src/main/java/org/apache/druid/data/input/impl/JSONLowercaseParseSpec.java index c555f8feec83..c833f2611f38 100644 --- a/core/src/main/java/org/apache/druid/data/input/impl/JSONLowercaseParseSpec.java +++ b/core/src/main/java/org/apache/druid/data/input/impl/JSONLowercaseParseSpec.java @@ -45,11 +45,6 @@ public JSONLowercaseParseSpec( this.objectMapper = new ObjectMapper(); } - @Override - public void verify(List usedCols) - { - } - @Override public Parser makeParser() { diff --git a/core/src/main/java/org/apache/druid/data/input/impl/JSONParseSpec.java b/core/src/main/java/org/apache/druid/data/input/impl/JSONParseSpec.java index bf33d5136ec2..7685c2f27af0 100644 --- a/core/src/main/java/org/apache/druid/data/input/impl/JSONParseSpec.java +++ b/core/src/main/java/org/apache/druid/data/input/impl/JSONParseSpec.java @@ -62,11 +62,6 @@ public JSONParseSpec(TimestampSpec ts, DimensionsSpec dims) this(ts, dims, null, null); } - @Override - public void verify(List usedCols) - { - } - @Override public Parser makeParser() { diff --git a/core/src/main/java/org/apache/druid/data/input/impl/JavaScriptParseSpec.java b/core/src/main/java/org/apache/druid/data/input/impl/JavaScriptParseSpec.java index d12b978991d7..7f48477565b8 100644 --- a/core/src/main/java/org/apache/druid/data/input/impl/JavaScriptParseSpec.java +++ b/core/src/main/java/org/apache/druid/data/input/impl/JavaScriptParseSpec.java @@ -59,11 +59,6 @@ public String getFunction() return function; } - @Override - public void verify(List usedCols) - { - } - @Override public Parser makeParser() { diff --git a/core/src/main/java/org/apache/druid/data/input/impl/ParseSpec.java b/core/src/main/java/org/apache/druid/data/input/impl/ParseSpec.java index b872219e7c8f..cbffbd868ac4 100644 --- a/core/src/main/java/org/apache/druid/data/input/impl/ParseSpec.java +++ b/core/src/main/java/org/apache/druid/data/input/impl/ParseSpec.java @@ -64,12 +64,6 @@ public DimensionsSpec getDimensionsSpec() return dimensionsSpec; } - @PublicApi - public void verify(List usedCols) - { - // do nothing - } - public Parser makeParser() { return null; diff --git a/core/src/main/java/org/apache/druid/data/input/impl/RegexParseSpec.java b/core/src/main/java/org/apache/druid/data/input/impl/RegexParseSpec.java index 5f8c1ea3ef10..e1c2255e5dfb 100644 --- a/core/src/main/java/org/apache/druid/data/input/impl/RegexParseSpec.java +++ b/core/src/main/java/org/apache/druid/data/input/impl/RegexParseSpec.java @@ -50,8 +50,6 @@ public RegexParseSpec( this.listDelimiter = listDelimiter; this.columns = columns; this.pattern = pattern; - - verify(dimensionsSpec.getDimensionNames()); } @JsonProperty @@ -72,16 +70,6 @@ public List getColumns() return columns; } - @Override - public void verify(List usedCols) - { - if (columns != null) { - for (String columnName : usedCols) { - Preconditions.checkArgument(columns.contains(columnName), "column[%s] not in columns.", columnName); - } - } - } - @Override public Parser makeParser() { diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskTest.java index 8cb99452a80b..184479c83bf8 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskTest.java @@ -101,6 +101,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -237,9 +238,9 @@ public void testTransformSpec() throws Exception File tmpFile = File.createTempFile("druid", "index", tmpDir); try (BufferedWriter writer = Files.newWriter(tmpFile, StandardCharsets.UTF_8)) { - writer.write("2014-01-01T00:00:10Z,a,1\n"); - writer.write("2014-01-01T01:00:20Z,b,1\n"); - writer.write("2014-01-01T02:00:30Z,c,1\n"); + writer.write("2014-01-01T00:00:10Z,a,an|array,1\n"); + writer.write("2014-01-01T01:00:20Z,b,another|array,1\n"); + writer.write("2014-01-01T02:00:30Z,c,and|another,1\n"); } IndexTask indexTask = new IndexTask( @@ -248,11 +249,28 @@ public void testTransformSpec() throws Exception createIngestionSpec( jsonMapper, tmpDir, - null, + new CSVParseSpec( + new TimestampSpec( + "ts", + "auto", + null + ), + new DimensionsSpec( + DimensionsSpec.getDefaultSchemas(Arrays.asList("ts", "dim", "dim_array", "dimt", "dimtarray1", "dimtarray2")), + new ArrayList<>(), + new ArrayList<>() + ), + "|", + Arrays.asList("ts", "dim", "dim_array", "val"), + false, + 0 + ), new TransformSpec( new SelectorDimFilter("dim", "b", null), ImmutableList.of( - new ExpressionTransform("dimt", "concat(dim,dim)", ExprMacroTable.nil()) + new ExpressionTransform("dimt", "concat(dim,dim)", ExprMacroTable.nil()), + new ExpressionTransform("dimtarray1", "array(dim, dim)", ExprMacroTable.nil()), + new ExpressionTransform("dimtarray2", "map(d -> concat(d, 'foo'), dim_array)", ExprMacroTable.nil()) ) ), null, @@ -270,6 +288,42 @@ public void testTransformSpec() throws Exception final List segments = runTask(indexTask).rhs; + DataSegment segment = segments.get(0); + final File segmentFile = segmentLoader.getSegmentFiles(segment); + + final WindowedStorageAdapter adapter = new WindowedStorageAdapter( + new QueryableIndexStorageAdapter(indexIO.loadIndex(segmentFile)), + segment.getInterval() + ); + final Sequence cursorSequence = adapter.getAdapter().makeCursors( + null, + segment.getInterval(), + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ); + final List> transforms = cursorSequence + .map(cursor -> { + final DimensionSelector selector1 = cursor.getColumnSelectorFactory() + .makeDimensionSelector(new DefaultDimensionSpec("dimt", "dimt")); + final DimensionSelector selector2 = cursor.getColumnSelectorFactory() + .makeDimensionSelector(new DefaultDimensionSpec("dimtarray1", "dimtarray1")); + final DimensionSelector selector3 = cursor.getColumnSelectorFactory() + .makeDimensionSelector(new DefaultDimensionSpec("dimtarray2", "dimtarray2")); + + Map row = new HashMap<>(); + row.put("dimt", selector1.defaultGetObject()); + row.put("dimtarray1", selector2.defaultGetObject()); + row.put("dimtarray2", selector3.defaultGetObject()); + cursor.advance(); + return row; + }) + .toList(); + Assert.assertEquals(1, transforms.size()); + Assert.assertEquals("bb", transforms.get(0).get("dimt")); + Assert.assertEquals(ImmutableList.of("b", "b"), transforms.get(0).get("dimtarray1")); + Assert.assertEquals(ImmutableList.of("anotherfoo", "arrayfoo"), transforms.get(0).get("dimtarray2")); Assert.assertEquals(1, segments.size()); Assert.assertEquals("test", segments.get(0).getDataSource()); diff --git a/processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java b/processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java index 2e0bc67ad173..97f6446da769 100644 --- a/processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java +++ b/processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java @@ -23,13 +23,19 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; +import org.apache.druid.common.config.NullHandling; import org.apache.druid.data.input.Row; import org.apache.druid.math.expr.Expr; +import org.apache.druid.math.expr.ExprEval; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.math.expr.Parser; import org.apache.druid.segment.column.ColumnHolder; +import org.apache.druid.segment.virtual.ExpressionSelectors; +import java.util.Arrays; +import java.util.List; import java.util.Objects; +import java.util.stream.Collectors; public class ExpressionTransform implements Transform { @@ -81,7 +87,7 @@ static class ExpressionRowFunction implements RowFunction @Override public Object eval(final Row row) { - return expr.eval(name -> getValueFromRow(row, name)).value(); + return ExpressionSelectors.coerceEvalToSelectorObject(expr.eval(name -> getValueFromRow(row, name))); } } @@ -90,7 +96,11 @@ private static Object getValueFromRow(final Row row, final String column) if (column.equals(ColumnHolder.TIME_COLUMN_NAME)) { return row.getTimestampFromEpoch(); } else { - return row.getRaw(column); + Object raw = row.getRaw(column); + if (raw instanceof List) { + return ExpressionSelectors.coerceListDimToStringArray((List) raw); + } + return raw; } } 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 1348029d9980..8991a29dd24c 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 @@ -108,12 +108,7 @@ public Object getObject() { // No need for null check on getObject() since baseSelector impls will never return null. ExprEval eval = baseSelector.getObject(); - if (eval.isArray()) { - return Arrays.stream(eval.asStringArray()) - .map(NullHandling::emptyToNullIfNeeded) - .collect(Collectors.toList()); - } - return eval.value(); + return coerceEvalToSelectorObject(eval); } @Override @@ -509,7 +504,7 @@ static Supplier supplierFromObjectSelector(final BaseObjectColumnValueSe /** * Selectors are not consistent in treatment of null, [], and [null], so coerce [] to [null] */ - private static Object coerceListDimToStringArray(List val) + public static Object coerceListDimToStringArray(List val) { Object[] arrayVal = val.stream().map(x -> x != null ? x.toString() : x).toArray(String[]::new); if (arrayVal.length > 0) { @@ -518,6 +513,20 @@ private static Object coerceListDimToStringArray(List val) return new String[]{null}; } + /** + * Coerces {@link ExprEval} value back to selector friendly {@link List} if the evaluated expression result is an + * array type + */ + public static Object coerceEvalToSelectorObject(ExprEval eval) + { + if (eval.isArray()) { + return Arrays.stream(eval.asStringArray()) + .map(NullHandling::emptyToNullIfNeeded) + .collect(Collectors.toList()); + } + return eval.value(); + } + /** * Returns pair of columns which are definitely multi-valued, or 'actual' arrays, and those which we are unable to * discern from the {@link ColumnSelectorFactory#getColumnCapabilities(String)}, or 'unknown' arrays. From a62b7ddb14ef766c0596e4f320a89923f2bc2922 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 25 Oct 2019 01:08:02 -0700 Subject: [PATCH 02/12] remove docs about being unsupported --- docs/misc/math-expr.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/misc/math-expr.md b/docs/misc/math-expr.md index 4f5259cbb593..27b799d0c72d 100644 --- a/docs/misc/math-expr.md +++ b/docs/misc/math-expr.md @@ -60,9 +60,6 @@ dialects. However, by using the `array_to_string` function, aggregations may be complete array, allowing the complete row to be preserved. Using `string_to_array` in an expression post-aggregator, allows transforming the stringified dimension back into the true native array type. -> Note that array functions are not currently supported at ingestion time with -> [`transformSpec`](../ingestion/index.md#transformspec). - The following built-in functions are available. From c334caf7c914eebe84f164ef962185661535d28e Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 25 Oct 2019 01:09:55 -0700 Subject: [PATCH 03/12] re-arrange test assert --- .../org/apache/druid/indexing/common/task/IndexTaskTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskTest.java index 184479c83bf8..f67e66493a64 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskTest.java @@ -288,6 +288,7 @@ public void testTransformSpec() throws Exception final List segments = runTask(indexTask).rhs; + Assert.assertEquals(1, segments.size()); DataSegment segment = segments.get(0); final File segmentFile = segmentLoader.getSegmentFiles(segment); @@ -324,7 +325,7 @@ public void testTransformSpec() throws Exception Assert.assertEquals("bb", transforms.get(0).get("dimt")); Assert.assertEquals(ImmutableList.of("b", "b"), transforms.get(0).get("dimtarray1")); Assert.assertEquals(ImmutableList.of("anotherfoo", "arrayfoo"), transforms.get(0).get("dimtarray2")); - Assert.assertEquals(1, segments.size()); + Assert.assertEquals("test", segments.get(0).getDataSource()); Assert.assertEquals(Intervals.of("2014/P1D"), segments.get(0).getInterval()); From dc8392228e2ab961ca4fc73902dd6308ec987d8d Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 25 Oct 2019 01:12:35 -0700 Subject: [PATCH 04/12] unused imports --- .../apache/druid/segment/transform/ExpressionTransform.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java b/processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java index 97f6446da769..c880bd223d3e 100644 --- a/processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java +++ b/processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java @@ -23,19 +23,15 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; -import org.apache.druid.common.config.NullHandling; import org.apache.druid.data.input.Row; import org.apache.druid.math.expr.Expr; -import org.apache.druid.math.expr.ExprEval; import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.math.expr.Parser; import org.apache.druid.segment.column.ColumnHolder; import org.apache.druid.segment.virtual.ExpressionSelectors; -import java.util.Arrays; import java.util.List; import java.util.Objects; -import java.util.stream.Collectors; public class ExpressionTransform implements Transform { From 671f4370a6b1c6bebd2e34a9489ecdde3213f3fc Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 25 Oct 2019 11:44:46 -0700 Subject: [PATCH 05/12] imports --- .../apache/druid/data/input/impl/JSONLowercaseParseSpec.java | 2 -- .../java/org/apache/druid/data/input/impl/JSONParseSpec.java | 1 - .../org/apache/druid/data/input/impl/JavaScriptParseSpec.java | 2 -- .../main/java/org/apache/druid/data/input/impl/ParseSpec.java | 2 -- .../java/org/apache/druid/data/input/impl/RegexParseSpec.java | 1 - 5 files changed, 8 deletions(-) diff --git a/core/src/main/java/org/apache/druid/data/input/impl/JSONLowercaseParseSpec.java b/core/src/main/java/org/apache/druid/data/input/impl/JSONLowercaseParseSpec.java index c833f2611f38..7422fccd1266 100644 --- a/core/src/main/java/org/apache/druid/data/input/impl/JSONLowercaseParseSpec.java +++ b/core/src/main/java/org/apache/druid/data/input/impl/JSONLowercaseParseSpec.java @@ -25,8 +25,6 @@ import org.apache.druid.java.util.common.parsers.JSONToLowerParser; import org.apache.druid.java.util.common.parsers.Parser; -import java.util.List; - /** * This class is only here for backwards compatibility */ diff --git a/core/src/main/java/org/apache/druid/data/input/impl/JSONParseSpec.java b/core/src/main/java/org/apache/druid/data/input/impl/JSONParseSpec.java index 7685c2f27af0..3a7136b2f8cf 100644 --- a/core/src/main/java/org/apache/druid/data/input/impl/JSONParseSpec.java +++ b/core/src/main/java/org/apache/druid/data/input/impl/JSONParseSpec.java @@ -28,7 +28,6 @@ import org.apache.druid.java.util.common.parsers.Parser; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Objects; diff --git a/core/src/main/java/org/apache/druid/data/input/impl/JavaScriptParseSpec.java b/core/src/main/java/org/apache/druid/data/input/impl/JavaScriptParseSpec.java index 7f48477565b8..fe6d80e5c678 100644 --- a/core/src/main/java/org/apache/druid/data/input/impl/JavaScriptParseSpec.java +++ b/core/src/main/java/org/apache/druid/data/input/impl/JavaScriptParseSpec.java @@ -27,8 +27,6 @@ import org.apache.druid.java.util.common.parsers.Parser; import org.apache.druid.js.JavaScriptConfig; -import java.util.List; - /** */ public class JavaScriptParseSpec extends ParseSpec diff --git a/core/src/main/java/org/apache/druid/data/input/impl/ParseSpec.java b/core/src/main/java/org/apache/druid/data/input/impl/ParseSpec.java index cbffbd868ac4..a90bebbb6b35 100644 --- a/core/src/main/java/org/apache/druid/data/input/impl/ParseSpec.java +++ b/core/src/main/java/org/apache/druid/data/input/impl/ParseSpec.java @@ -27,8 +27,6 @@ import org.apache.druid.guice.annotations.PublicApi; import org.apache.druid.java.util.common.parsers.Parser; -import java.util.List; - @ExtensionPoint @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "format") @JsonSubTypes(value = { diff --git a/core/src/main/java/org/apache/druid/data/input/impl/RegexParseSpec.java b/core/src/main/java/org/apache/druid/data/input/impl/RegexParseSpec.java index e1c2255e5dfb..e40d80ba4367 100644 --- a/core/src/main/java/org/apache/druid/data/input/impl/RegexParseSpec.java +++ b/core/src/main/java/org/apache/druid/data/input/impl/RegexParseSpec.java @@ -22,7 +22,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Optional; -import com.google.common.base.Preconditions; import org.apache.druid.java.util.common.parsers.Parser; import org.apache.druid.java.util.common.parsers.RegexParser; From 7d6d7c4bd63854fa365c17cd44a734395d0ef405 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 25 Oct 2019 21:26:22 -0700 Subject: [PATCH 06/12] fix tests --- .../data/input/impl/CSVParseSpecTest.java | 24 +----------------- .../input/impl/DelimitedParseSpecTest.java | 25 +------------------ 2 files changed, 2 insertions(+), 47 deletions(-) diff --git a/core/src/test/java/org/apache/druid/data/input/impl/CSVParseSpecTest.java b/core/src/test/java/org/apache/druid/data/input/impl/CSVParseSpecTest.java index f5fa4ee1b9bd..f31dc874303a 100644 --- a/core/src/test/java/org/apache/druid/data/input/impl/CSVParseSpecTest.java +++ b/core/src/test/java/org/apache/druid/data/input/impl/CSVParseSpecTest.java @@ -27,28 +27,6 @@ public class CSVParseSpecTest { - @Test(expected = IllegalArgumentException.class) - public void testColumnMissing() - { - @SuppressWarnings("unused") // expected exception - final ParseSpec spec = new CSVParseSpec( - new TimestampSpec( - "timestamp", - "auto", - null - ), - new DimensionsSpec( - DimensionsSpec.getDefaultSchemas(Arrays.asList("a", "b")), - new ArrayList<>(), - new ArrayList<>() - ), - ",", - Collections.singletonList("a"), - false, - 0 - ); - } - @Test(expected = IllegalArgumentException.class) public void testComma() { @@ -65,7 +43,7 @@ public void testComma() new ArrayList<>() ), ",", - Collections.singletonList("a"), + Collections.singletonList("a,"), false, 0 ); diff --git a/core/src/test/java/org/apache/druid/data/input/impl/DelimitedParseSpecTest.java b/core/src/test/java/org/apache/druid/data/input/impl/DelimitedParseSpecTest.java index 9b0786228d66..95c9e1021079 100644 --- a/core/src/test/java/org/apache/druid/data/input/impl/DelimitedParseSpecTest.java +++ b/core/src/test/java/org/apache/druid/data/input/impl/DelimitedParseSpecTest.java @@ -58,29 +58,6 @@ public void testSerde() throws IOException Assert.assertEquals(Collections.singletonList("abc"), serde.getDimensionsSpec().getDimensionNames()); } - @Test(expected = IllegalArgumentException.class) - public void testColumnMissing() - { - @SuppressWarnings("unused") // expected exception - final ParseSpec spec = new DelimitedParseSpec( - new TimestampSpec( - "timestamp", - "auto", - null - ), - new DimensionsSpec( - DimensionsSpec.getDefaultSchemas(Arrays.asList("a", "b")), - new ArrayList<>(), - new ArrayList<>() - ), - ",", - " ", - Collections.singletonList("a"), - false, - 0 - ); - } - @Test(expected = IllegalArgumentException.class) public void testComma() { @@ -98,7 +75,7 @@ public void testComma() ), ",", null, - Collections.singletonList("a"), + Collections.singletonList("a,"), false, 0 ); From 4cf6a89243b22fef08f0631b7cb7de9752f3357f Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 8 Nov 2019 13:34:32 -0800 Subject: [PATCH 07/12] preserve types --- .../druid/java/util/common/StringUtils.java | 2 +- .../indexing/common/task/IndexTaskTest.java | 30 ++++++++++++---- .../transform/ExpressionTransform.java | 2 +- .../segment/virtual/ExpressionSelectors.java | 36 +++++++++++++------ .../druid/sql/calcite/CalciteQueryTest.java | 2 +- 5 files changed, 51 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/org/apache/druid/java/util/common/StringUtils.java b/core/src/main/java/org/apache/druid/java/util/common/StringUtils.java index 4880c0f6cee2..d5e39e31e4d8 100644 --- a/core/src/main/java/org/apache/druid/java/util/common/StringUtils.java +++ b/core/src/main/java/org/apache/druid/java/util/common/StringUtils.java @@ -112,7 +112,7 @@ public static byte[] toUtf8(final String string) /** * Encodes "string" into the buffer "byteBuffer", using no more than the number of bytes remaining in the buffer. - * Will only encode whole characters. The byteBuffer's position and limit be changed during operation, but will + * Will only encode whole characters. The byteBuffer's position and limit may be changed during operation, but will * be reset before this method call ends. * * @return the number of bytes written, which may be shorter than the full encoded string length if there diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskTest.java index f67e66493a64..1372f2a7abe5 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/IndexTaskTest.java @@ -238,9 +238,9 @@ public void testTransformSpec() throws Exception File tmpFile = File.createTempFile("druid", "index", tmpDir); try (BufferedWriter writer = Files.newWriter(tmpFile, StandardCharsets.UTF_8)) { - writer.write("2014-01-01T00:00:10Z,a,an|array,1\n"); - writer.write("2014-01-01T01:00:20Z,b,another|array,1\n"); - writer.write("2014-01-01T02:00:30Z,c,and|another,1\n"); + writer.write("2014-01-01T00:00:10Z,a,an|array,1|2|3,1\n"); + writer.write("2014-01-01T01:00:20Z,b,another|array,3|4,1\n"); + writer.write("2014-01-01T02:00:30Z,c,and|another,0|1,1\n"); } IndexTask indexTask = new IndexTask( @@ -256,12 +256,23 @@ public void testTransformSpec() throws Exception null ), new DimensionsSpec( - DimensionsSpec.getDefaultSchemas(Arrays.asList("ts", "dim", "dim_array", "dimt", "dimtarray1", "dimtarray2")), + DimensionsSpec.getDefaultSchemas( + Arrays.asList( + "ts", + "dim", + "dim_array", + "dim_num_array", + "dimt", + "dimtarray1", + "dimtarray2", + "dimtnum_array" + ) + ), new ArrayList<>(), new ArrayList<>() ), "|", - Arrays.asList("ts", "dim", "dim_array", "val"), + Arrays.asList("ts", "dim", "dim_array", "dim_num_array", "val"), false, 0 ), @@ -270,7 +281,8 @@ public void testTransformSpec() throws Exception ImmutableList.of( new ExpressionTransform("dimt", "concat(dim,dim)", ExprMacroTable.nil()), new ExpressionTransform("dimtarray1", "array(dim, dim)", ExprMacroTable.nil()), - new ExpressionTransform("dimtarray2", "map(d -> concat(d, 'foo'), dim_array)", ExprMacroTable.nil()) + new ExpressionTransform("dimtarray2", "map(d -> concat(d, 'foo'), dim_array)", ExprMacroTable.nil()), + new ExpressionTransform("dimtnum_array", "map(d -> d + 3, dim_num_array)", ExprMacroTable.nil()) ) ), null, @@ -312,11 +324,15 @@ public void testTransformSpec() throws Exception .makeDimensionSelector(new DefaultDimensionSpec("dimtarray1", "dimtarray1")); final DimensionSelector selector3 = cursor.getColumnSelectorFactory() .makeDimensionSelector(new DefaultDimensionSpec("dimtarray2", "dimtarray2")); + final DimensionSelector selector4 = cursor.getColumnSelectorFactory() + .makeDimensionSelector(new DefaultDimensionSpec("dimtnum_array", "dimtnum_array")); + Map row = new HashMap<>(); row.put("dimt", selector1.defaultGetObject()); row.put("dimtarray1", selector2.defaultGetObject()); row.put("dimtarray2", selector3.defaultGetObject()); + row.put("dimtnum_array", selector4.defaultGetObject()); cursor.advance(); return row; }) @@ -325,7 +341,7 @@ public void testTransformSpec() throws Exception Assert.assertEquals("bb", transforms.get(0).get("dimt")); Assert.assertEquals(ImmutableList.of("b", "b"), transforms.get(0).get("dimtarray1")); Assert.assertEquals(ImmutableList.of("anotherfoo", "arrayfoo"), transforms.get(0).get("dimtarray2")); - + Assert.assertEquals(ImmutableList.of("6.0", "7.0"), transforms.get(0).get("dimtnum_array")); Assert.assertEquals("test", segments.get(0).getDataSource()); Assert.assertEquals(Intervals.of("2014/P1D"), segments.get(0).getInterval()); diff --git a/processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java b/processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java index c880bd223d3e..16bad318a7fc 100644 --- a/processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java +++ b/processing/src/main/java/org/apache/druid/segment/transform/ExpressionTransform.java @@ -94,7 +94,7 @@ private static Object getValueFromRow(final Row row, final String column) } else { Object raw = row.getRaw(column); if (raw instanceof List) { - return ExpressionSelectors.coerceListDimToStringArray((List) raw); + return ExpressionSelectors.coerceListToArray((List) raw); } return raw; } 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 041a12c3b4ac..1bd34df0548b 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 @@ -487,7 +487,7 @@ static Supplier supplierFromObjectSelector(final BaseObjectColumnValueSe if (val instanceof Number || val instanceof String) { return val; } else if (val instanceof List) { - return coerceListDimToStringArray((List) val); + return coerceListToArray((List) val); } else { return null; } @@ -496,7 +496,7 @@ static Supplier supplierFromObjectSelector(final BaseObjectColumnValueSe return () -> { final Object val = selector.getObject(); if (val != null) { - return coerceListDimToStringArray((List) val); + return coerceListToArray((List) val); } return null; }; @@ -509,11 +509,19 @@ static Supplier supplierFromObjectSelector(final BaseObjectColumnValueSe /** * Selectors are not consistent in treatment of null, [], and [null], so coerce [] to [null] */ - public static Object coerceListDimToStringArray(List val) + public static Object coerceListToArray(List val) { - Object[] arrayVal = val.stream().map(x -> x != null ? x.toString() : x).toArray(String[]::new); - if (arrayVal.length > 0) { - return arrayVal; + if (val != null && val.size() > 0) { + Object firstElement = val.get(0); + if (firstElement instanceof Long || firstElement instanceof Integer) { + return val.stream().toArray(Long[]::new); + } else if (firstElement instanceof Float) { + return val.stream().map(x -> ((Float) x).doubleValue()).toArray(Double[]::new); + } else if (firstElement instanceof Double) { + return val.stream().toArray(Double[]::new); + } else { + return val.stream().map(x -> x != null ? x.toString() : x).toArray(String[]::new); + } } return new String[]{null}; } @@ -522,14 +530,20 @@ public static Object coerceListDimToStringArray(List val) * Coerces {@link ExprEval} value back to selector friendly {@link List} if the evaluated expression result is an * array type */ + @Nullable public static Object coerceEvalToSelectorObject(ExprEval eval) { - if (eval.isArray()) { - return Arrays.stream(eval.asStringArray()) - .map(NullHandling::emptyToNullIfNeeded) - .collect(Collectors.toList()); + switch (eval.type()) { + case STRING_ARRAY: + return Arrays.stream(eval.asStringArray()) + .map(NullHandling::emptyToNullIfNeeded) + .collect(Collectors.toList()); + case DOUBLE_ARRAY: + case LONG_ARRAY: + return Arrays.stream(eval.asArray()).collect(Collectors.toList()); + default: + return eval.value(); } - return eval.value(); } /** 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 4b411da4a858..235e89b35092 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 @@ -8411,7 +8411,7 @@ public void testSelectConstantArrayExpressionFromTable() throws Exception .build() ), ImmutableList.of( - new Object[]{"[\"1\",\"2\"]", ""} + new Object[]{"[1,2]", ""} ) ); } From 0b1564036cceb8b60bf2a774dbf04192c3de516c Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 8 Nov 2019 17:11:50 -0800 Subject: [PATCH 08/12] suppress warning, fixes, add test --- .../segment/virtual/ExpressionSelectors.java | 6 ++- .../ExpressionColumnValueSelectorTest.java | 50 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) 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 1bd34df0548b..dc5a97412d64 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 @@ -509,12 +509,16 @@ static Supplier supplierFromObjectSelector(final BaseObjectColumnValueSe /** * Selectors are not consistent in treatment of null, [], and [null], so coerce [] to [null] */ + // suppressed because calling toArray creates Object[] instead of Long[] which makes ExprEval.bestEffortOf sad + @SuppressWarnings("SimplifyStreamApiCallChains") public static Object coerceListToArray(List val) { if (val != null && val.size() > 0) { Object firstElement = val.get(0); - if (firstElement instanceof Long || firstElement instanceof Integer) { + if (firstElement instanceof Long) { return val.stream().toArray(Long[]::new); + } else if(firstElement instanceof Integer) { + return val.stream().map(x -> ((Integer) x).longValue()).toArray(Long[]::new); } else if (firstElement instanceof Float) { return val.stream().map(x -> ((Float) x).doubleValue()).toArray(Double[]::new); } else if (firstElement instanceof Double) { diff --git a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionColumnValueSelectorTest.java b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionColumnValueSelectorTest.java index 079f692fc653..b5470015b161 100644 --- a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionColumnValueSelectorTest.java +++ b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionColumnValueSelectorTest.java @@ -22,6 +22,7 @@ import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import org.apache.druid.common.guava.SettableSupplier; +import org.apache.druid.math.expr.ExprEval; import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector; import org.apache.druid.segment.BaseSingleValueDimensionSelector; import org.apache.druid.segment.ColumnValueSelector; @@ -30,6 +31,7 @@ import org.junit.Assert; import org.junit.Test; +import java.util.ArrayList; import java.util.List; public class ExpressionColumnValueSelectorTest @@ -130,6 +132,54 @@ public void testSupplierFromObjectSelectorList() } + @Test + public void testCoerceListToArray() + { + List longList = ImmutableList.of(1L, 2L, 3L); + List intList = ImmutableList.of(1, 2, 3); + List floatList = ImmutableList.of(1.0f, 2.0f, 3.0f); + List doubleList = ImmutableList.of(1.0, 2.0, 3.0); + List stringList = ImmutableList.of("a", "b", "c"); + List withNulls = new ArrayList<>(); + withNulls.add("a"); + withNulls.add(null); + withNulls.add("c"); + Assert.assertArrayEquals(new Long[]{1L, 2L, 3L}, (Long[]) ExpressionSelectors.coerceListToArray(longList)); + Assert.assertArrayEquals(new Long[]{1L, 2L, 3L}, (Long[]) ExpressionSelectors.coerceListToArray(intList)); + Assert.assertArrayEquals(new Double[]{1.0, 2.0, 3.0}, (Double[]) ExpressionSelectors.coerceListToArray(floatList)); + Assert.assertArrayEquals(new Double[]{1.0, 2.0, 3.0}, (Double[]) ExpressionSelectors.coerceListToArray(doubleList)); + Assert.assertArrayEquals(new String[]{"a", "b", "c"}, (String[]) ExpressionSelectors.coerceListToArray(stringList)); + Assert.assertArrayEquals(new String[]{"a", null, "c"}, (String[]) ExpressionSelectors.coerceListToArray(withNulls)); + } + + @Test + public void testCoerceExprToValue() + { + Assert.assertEquals( + ImmutableList.of(1L, 2L, 3L), + ExpressionSelectors.coerceEvalToSelectorObject(ExprEval.ofLongArray(new Long[]{1L, 2L, 3L})) + ); + + Assert.assertEquals( + ImmutableList.of(1.0, 2.0, 3.0), + ExpressionSelectors.coerceEvalToSelectorObject(ExprEval.ofDoubleArray(new Double[]{1.0, 2.0, 3.0})) + ); + + Assert.assertEquals( + ImmutableList.of("a", "b", "c"), + ExpressionSelectors.coerceEvalToSelectorObject(ExprEval.ofStringArray(new String[]{"a", "b", "c"})) + ); + + List withNulls = new ArrayList<>(); + withNulls.add("a"); + withNulls.add(null); + withNulls.add("c"); + Assert.assertEquals( + withNulls, + ExpressionSelectors.coerceEvalToSelectorObject(ExprEval.ofStringArray(new String[]{"a", "", "c"})) + ); + } + private static DimensionSelector dimensionSelectorFromSupplier( final Supplier supplier ) From 71691234f338862002334ef51933996cb79ebc25 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 8 Nov 2019 17:17:45 -0800 Subject: [PATCH 09/12] formatting --- .../org/apache/druid/segment/virtual/ExpressionSelectors.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 dc5a97412d64..b9a5e2e637c9 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 @@ -517,7 +517,7 @@ public static Object coerceListToArray(List val) Object firstElement = val.get(0); if (firstElement instanceof Long) { return val.stream().toArray(Long[]::new); - } else if(firstElement instanceof Integer) { + } else if (firstElement instanceof Integer) { return val.stream().map(x -> ((Integer) x).longValue()).toArray(Long[]::new); } else if (firstElement instanceof Float) { return val.stream().map(x -> ((Float) x).doubleValue()).toArray(Double[]::new); From 537ef8a2c025a66cac490b6a63f1a740a211889d Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Fri, 8 Nov 2019 20:51:45 -0800 Subject: [PATCH 10/12] cleanup --- .../org/apache/druid/segment/virtual/ExpressionSelectors.java | 4 +--- .../segment/virtual/ExpressionColumnValueSelectorTest.java | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) 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 b9a5e2e637c9..08e906eb2be5 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 @@ -539,9 +539,7 @@ public static Object coerceEvalToSelectorObject(ExprEval eval) { switch (eval.type()) { case STRING_ARRAY: - return Arrays.stream(eval.asStringArray()) - .map(NullHandling::emptyToNullIfNeeded) - .collect(Collectors.toList()); + return Arrays.stream(eval.asStringArray()).collect(Collectors.toList()); case DOUBLE_ARRAY: case LONG_ARRAY: return Arrays.stream(eval.asArray()).collect(Collectors.toList()); diff --git a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionColumnValueSelectorTest.java b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionColumnValueSelectorTest.java index b5470015b161..17524683d2e5 100644 --- a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionColumnValueSelectorTest.java +++ b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionColumnValueSelectorTest.java @@ -176,7 +176,7 @@ public void testCoerceExprToValue() withNulls.add("c"); Assert.assertEquals( withNulls, - ExpressionSelectors.coerceEvalToSelectorObject(ExprEval.ofStringArray(new String[]{"a", "", "c"})) + ExpressionSelectors.coerceEvalToSelectorObject(ExprEval.ofStringArray(new String[]{"a", null, "c"})) ); } From e8d7dde92e56478ad3ff3e26c1e8a455452b083c Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Tue, 12 Nov 2019 15:39:32 -0800 Subject: [PATCH 11/12] better list to array type conversion and tests --- .../segment/virtual/ExpressionSelectors.java | 72 +++++++++++++++---- .../ExpressionColumnValueSelectorTest.java | 61 ++++++++++++++-- 2 files changed, 113 insertions(+), 20 deletions(-) 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 08e906eb2be5..6cb4df889b08 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 @@ -25,6 +25,7 @@ import com.google.common.collect.Iterables; import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.Pair; +import org.apache.druid.java.util.common.UOE; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprEval; import org.apache.druid.math.expr.Parser; @@ -509,27 +510,67 @@ static Supplier supplierFromObjectSelector(final BaseObjectColumnValueSe /** * Selectors are not consistent in treatment of null, [], and [null], so coerce [] to [null] */ - // suppressed because calling toArray creates Object[] instead of Long[] which makes ExprEval.bestEffortOf sad - @SuppressWarnings("SimplifyStreamApiCallChains") - public static Object coerceListToArray(List val) + public static Object coerceListToArray(@Nullable List val) { if (val != null && val.size() > 0) { - Object firstElement = val.get(0); - if (firstElement instanceof Long) { - return val.stream().toArray(Long[]::new); - } else if (firstElement instanceof Integer) { - return val.stream().map(x -> ((Integer) x).longValue()).toArray(Long[]::new); - } else if (firstElement instanceof Float) { - return val.stream().map(x -> ((Float) x).doubleValue()).toArray(Double[]::new); - } else if (firstElement instanceof Double) { - return val.stream().toArray(Double[]::new); - } else { - return val.stream().map(x -> x != null ? x.toString() : x).toArray(String[]::new); + Class coercedType = null; + + for (Object elem : val) { + if (elem != null) { + coercedType = convertType(coercedType, elem.getClass()); + } + } + + if (coercedType == Long.class || coercedType == Integer.class) { + return val.stream().map(x -> x != null ? ((Number) x).longValue() : null).toArray(Long[]::new); } + if (coercedType == Float.class || coercedType == Double.class) { + return val.stream().map(x -> x != null ? ((Number) x).doubleValue() : null).toArray(Double[]::new); + } + // default to string + return val.stream().map(x -> x != null ? x.toString() : null).toArray(String[]::new); } return new String[]{null}; } + private static Class convertType(@Nullable Class existing, Class next) + { + if (Number.class.isAssignableFrom(next) || next == String.class) { + if (existing == null) { + return next; + } + // string wins everything + if (existing == String.class) { + return existing; + } + if (next == String.class) { + return next; + } + // all numbers win over Integer + if (existing == Integer.class) { + return next; + } + if (existing == Float.class) { + // doubles win over floats + if (next == Double.class) { + return next; + } + return existing; + } + if (existing == Long.class) { + if (next == Integer.class) { + // long beats int + return existing; + } + // double and float win over longs + return next; + } + // otherwise double + return Double.class; + } + throw new UOE("Invalid array expression type: ", next); + } + /** * Coerces {@link ExprEval} value back to selector friendly {@link List} if the evaluated expression result is an * array type @@ -541,8 +582,9 @@ public static Object coerceEvalToSelectorObject(ExprEval eval) case STRING_ARRAY: return Arrays.stream(eval.asStringArray()).collect(Collectors.toList()); case DOUBLE_ARRAY: + return Arrays.stream(eval.asDoubleArray()).collect(Collectors.toList()); case LONG_ARRAY: - return Arrays.stream(eval.asArray()).collect(Collectors.toList()); + return Arrays.stream(eval.asLongArray()).collect(Collectors.toList()); default: return eval.value(); } diff --git a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionColumnValueSelectorTest.java b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionColumnValueSelectorTest.java index 17524683d2e5..0b7a66db20fe 100644 --- a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionColumnValueSelectorTest.java +++ b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionColumnValueSelectorTest.java @@ -136,20 +136,71 @@ public void testSupplierFromObjectSelectorList() public void testCoerceListToArray() { List longList = ImmutableList.of(1L, 2L, 3L); + Assert.assertArrayEquals(new Long[]{1L, 2L, 3L}, (Long[]) ExpressionSelectors.coerceListToArray(longList)); + List intList = ImmutableList.of(1, 2, 3); + Assert.assertArrayEquals(new Long[]{1L, 2L, 3L}, (Long[]) ExpressionSelectors.coerceListToArray(intList)); + List floatList = ImmutableList.of(1.0f, 2.0f, 3.0f); + Assert.assertArrayEquals(new Double[]{1.0, 2.0, 3.0}, (Double[]) ExpressionSelectors.coerceListToArray(floatList)); + List doubleList = ImmutableList.of(1.0, 2.0, 3.0); + Assert.assertArrayEquals(new Double[]{1.0, 2.0, 3.0}, (Double[]) ExpressionSelectors.coerceListToArray(doubleList)); + List stringList = ImmutableList.of("a", "b", "c"); + Assert.assertArrayEquals(new String[]{"a", "b", "c"}, (String[]) ExpressionSelectors.coerceListToArray(stringList)); + List withNulls = new ArrayList<>(); withNulls.add("a"); withNulls.add(null); withNulls.add("c"); - Assert.assertArrayEquals(new Long[]{1L, 2L, 3L}, (Long[]) ExpressionSelectors.coerceListToArray(longList)); - Assert.assertArrayEquals(new Long[]{1L, 2L, 3L}, (Long[]) ExpressionSelectors.coerceListToArray(intList)); - Assert.assertArrayEquals(new Double[]{1.0, 2.0, 3.0}, (Double[]) ExpressionSelectors.coerceListToArray(floatList)); - Assert.assertArrayEquals(new Double[]{1.0, 2.0, 3.0}, (Double[]) ExpressionSelectors.coerceListToArray(doubleList)); - Assert.assertArrayEquals(new String[]{"a", "b", "c"}, (String[]) ExpressionSelectors.coerceListToArray(stringList)); Assert.assertArrayEquals(new String[]{"a", null, "c"}, (String[]) ExpressionSelectors.coerceListToArray(withNulls)); + + List withNumberNulls = new ArrayList<>(); + withNumberNulls.add(1L); + withNumberNulls.add(null); + withNumberNulls.add(3L); + + Assert.assertArrayEquals(new Long[]{1L, null, 3L}, (Long[]) ExpressionSelectors.coerceListToArray(withNumberNulls)); + + List withStringMix = ImmutableList.of(1L, "b", 3L); + Assert.assertArrayEquals( + new String[]{"1", "b", "3"}, + (String[]) ExpressionSelectors.coerceListToArray(withStringMix) + ); + + List withIntsAndLongs = ImmutableList.of(1, 2L, 3); + Assert.assertArrayEquals( + new Long[]{1L, 2L, 3L}, + (Long[]) ExpressionSelectors.coerceListToArray(withIntsAndLongs) + ); + + List withFloatsAndLongs = ImmutableList.of(1, 2L, 3.0f); + Assert.assertArrayEquals( + new Double[]{1.0, 2.0, 3.0}, + (Double[]) ExpressionSelectors.coerceListToArray(withFloatsAndLongs) + ); + + List withDoublesAndLongs = ImmutableList.of(1, 2L, 3.0); + Assert.assertArrayEquals( + new Double[]{1.0, 2.0, 3.0}, + (Double[]) ExpressionSelectors.coerceListToArray(withDoublesAndLongs) + ); + + List withFloatsAndDoubles = ImmutableList.of(1L, 2.0f, 3.0); + Assert.assertArrayEquals( + new Double[]{1.0, 2.0, 3.0}, + (Double[]) ExpressionSelectors.coerceListToArray(withFloatsAndDoubles) + ); + + List withAllNulls = new ArrayList<>(); + withAllNulls.add(null); + withAllNulls.add(null); + withAllNulls.add(null); + Assert.assertArrayEquals( + new String[]{null, null, null}, + (String[]) ExpressionSelectors.coerceListToArray(withAllNulls) + ); } @Test From 6250b3337c6ed92fd847dc44f18966185426067c Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Tue, 12 Nov 2019 16:55:14 -0800 Subject: [PATCH 12/12] fix oops --- .../org/apache/druid/segment/virtual/ExpressionSelectors.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6cb4df889b08..91fcb6c4f121 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 @@ -568,7 +568,7 @@ private static Class convertType(@Nullable Class existing, Class next) // otherwise double return Double.class; } - throw new UOE("Invalid array expression type: ", next); + throw new UOE("Invalid array expression type: %s", next); } /**