diff --git a/processing/src/main/java/io/druid/query/aggregation/JavaScriptAggregatorFactory.java b/processing/src/main/java/io/druid/query/aggregation/JavaScriptAggregatorFactory.java index d54425d343b..0d33bf1ad30 100644 --- a/processing/src/main/java/io/druid/query/aggregation/JavaScriptAggregatorFactory.java +++ b/processing/src/main/java/io/druid/query/aggregation/JavaScriptAggregatorFactory.java @@ -278,7 +278,7 @@ public class JavaScriptAggregatorFactory implements AggregatorFactory final ObjectColumnSelector selector = selectorList[i]; if (selector != null) { final Object arg = selector.get(); - if (arg.getClass().isArray()) { + if (arg != null && arg.getClass().isArray()) { // Context.javaToJS on an array sort of works, although it returns false for Array.isArray(...) and // may have other issues too. Let's just copy the array and wrap that. final Object[] arrayAsObjectArray = new Object[Array.getLength(arg)]; diff --git a/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java b/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java index b3178d83df7..fd772ea4770 100644 --- a/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java +++ b/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java @@ -467,23 +467,29 @@ public class QueryableIndexStorageAdapter implements StorageAdapter if (cachedColumnVals instanceof DictionaryEncodedColumn) { final DictionaryEncodedColumn columnVals = (DictionaryEncodedColumn) cachedColumnVals; if (columnVals.hasMultipleValues()) { - return new ObjectColumnSelector() + return new ObjectColumnSelector() { @Override public Class classOfObject() { - return String[].class; + return Object.class; } @Override - public String[] get() + public Object get() { final IndexedInts multiValueRow = columnVals.getMultiValueRow(cursorOffset.getOffset()); - final String[] strings = new String[multiValueRow.size()]; - for (int i = 0 ; i < multiValueRow.size() ; i++) { - strings[i] = columnVals.lookupName(multiValueRow.get(i)); + if (multiValueRow.size() == 0) { + return null; + } else if (multiValueRow.size() == 1) { + return columnVals.lookupName(multiValueRow.get(1)); + } else { + final String[] strings = new String[multiValueRow.size()]; + for (int i = 0 ; i < multiValueRow.size() ; i++) { + strings[i] = columnVals.lookupName(multiValueRow.get(i)); + } + return strings; } - return strings; } }; } else { @@ -891,23 +897,29 @@ public class QueryableIndexStorageAdapter implements StorageAdapter if (cachedColumnVals instanceof DictionaryEncodedColumn) { final DictionaryEncodedColumn columnVals = (DictionaryEncodedColumn) cachedColumnVals; if (columnVals.hasMultipleValues()) { - return new ObjectColumnSelector() + return new ObjectColumnSelector() { @Override public Class classOfObject() { - return String[].class; + return Object.class; } @Override - public String[] get() + public Object get() { final IndexedInts multiValueRow = columnVals.getMultiValueRow(currRow); - final String[] strings = new String[multiValueRow.size()]; - for (int i = 0 ; i < multiValueRow.size() ; i++) { - strings[i] = columnVals.lookupName(multiValueRow.get(i)); + if (multiValueRow.size() == 0) { + return null; + } else if (multiValueRow.size() == 1) { + return columnVals.lookupName(multiValueRow.get(1)); + } else { + final String[] strings = new String[multiValueRow.size()]; + for (int i = 0 ; i < multiValueRow.size() ; i++) { + strings[i] = columnVals.lookupName(multiValueRow.get(i)); + } + return strings; } - return strings; } }; } else { diff --git a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java index d4c3dad7a7a..7ff26a789a8 100644 --- a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java +++ b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java @@ -408,11 +408,7 @@ public class IncrementalIndexStorageAdapter implements StorageAdapter @Override public Class classOfObject() { - if (currEntry.getKey().getDims()[dimensionIndex].length > 1) { - return String[].class; - } else { - return String.class; - } + return Object.class; } @Override diff --git a/processing/src/test/java/io/druid/query/aggregation/JavaScriptAggregatorTest.java b/processing/src/test/java/io/druid/query/aggregation/JavaScriptAggregatorTest.java index 36250b18e41..d53ea6d1235 100644 --- a/processing/src/test/java/io/druid/query/aggregation/JavaScriptAggregatorTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/JavaScriptAggregatorTest.java @@ -184,12 +184,12 @@ public class JavaScriptAggregatorTest @Test public void testAggregateStrings() { - final TestObjectColumnSelector ocs = new TestObjectColumnSelector("what", new String[]{"hey", "there"}); + final TestObjectColumnSelector ocs = new TestObjectColumnSelector("what", null, new String[]{"hey", "there"}); final JavaScriptAggregator agg = new JavaScriptAggregator( "billy", Collections.singletonList(ocs), JavaScriptAggregatorFactory.compileScript( - "function aggregate(current, a) { if (typeof a === 'string') { return current + 1; } else { return current + a.length; } }", + "function aggregate(current, a) { if (Array.isArray(a)) { return current + a.length; } else if (typeof a === 'string') { return current + 1; } else { return current; } }", scriptDoubleSum.get("fnReset"), scriptDoubleSum.get("fnCombine") ) @@ -211,6 +211,11 @@ public class JavaScriptAggregatorTest Assert.assertEquals(val, agg.get()); aggregate(ocs, agg); + Assert.assertEquals(val, agg.get()); + Assert.assertEquals(val, agg.get()); + Assert.assertEquals(val, agg.get()); + aggregate(ocs, agg); + val += 2; Assert.assertEquals(val, agg.get()); Assert.assertEquals(val, agg.get());