From 23ba6f7ad7dc92903567a8458994e9d8731621cf Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Sat, 25 Aug 2018 14:31:46 -0700 Subject: [PATCH] Fix four bugs with numeric dimension output types. (#6220) * Fix four bugs with numeric dimension output types. This patch includes the following bug fixes: - TopNColumnSelectorStrategyFactory: Cast dimension values to the output type during dimExtractionScanAndAggregate instead of updateDimExtractionResults. This fixes a bug where, for example, grouping on doubles-cast-to-longs would fail to merge two doubles that should have been combined into the same long value. - TopNQueryEngine: Use DimExtractionTopNAlgorithm when treating string columns as numeric dimensions. This fixes a similar bug: grouping on string-cast-to-long would fail to merge two strings that should have been combined. - GroupByQuery: Cast numeric types to the expected output type before comparing them in compareDimsForLimitPushDown. This fixes #6123. - GroupByQueryQueryToolChest: Convert Jackson-deserialized dimension values into the proper output type. This fixes an inconsistency between results that came from cache vs. not-cache: for example, Jackson sometimes deserializes integers as Integers and sometimes as Longs. And the following code-cleanup changes, related to the fixes above: - DimensionHandlerUtils: Introduce convertObjectToType, compareObjectsAsType, and converterFromTypeToType to make it easier to handle casting operations. - TopN in general: Rename various "dimName" variables to "dimValue" where they actually represent dimension values. The old names were confusing. * Remove unused imports. --- .../io/druid/query/groupby/GroupByQuery.java | 50 +++---- .../groupby/GroupByQueryQueryToolChest.java | 10 +- .../epinephelinae/GroupByQueryEngineV2.java | 20 +-- .../epinephelinae/RowBasedGrouperHelper.java | 23 +-- .../topn/DimExtractionTopNAlgorithm.java | 14 -- .../io/druid/query/topn/DimValHolder.java | 20 +-- .../druid/query/topn/PooledTopNAlgorithm.java | 27 +--- .../topn/TopNLexicographicResultBuilder.java | 52 ++----- .../java/io/druid/query/topn/TopNMapFn.java | 32 +---- .../query/topn/TopNNumericResultBuilder.java | 71 ++++------ .../io/druid/query/topn/TopNQueryEngine.java | 4 + .../query/topn/TopNQueryQueryToolChest.java | 12 +- .../druid/query/topn/TopNResultBuilder.java | 2 +- .../NumericTopNColumnSelectorStrategy.java | 57 +++++--- .../StringTopNColumnSelectorStrategy.java | 51 +++---- .../types/TopNColumnSelectorStrategy.java | 11 +- .../TopNColumnSelectorStrategyFactory.java | 31 +++- .../druid/segment/DimensionHandlerUtils.java | 68 ++++++++- .../druid/query/topn/TopNQueryRunnerTest.java | 132 ++++++++++++++++++ 19 files changed, 371 insertions(+), 316 deletions(-) diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java b/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java index e96b5427d48..d5aa047676d 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java @@ -379,7 +379,7 @@ public class GroupByQuery extends BaseQuery final List orderedFieldNames = new ArrayList<>(); final Set dimsInOrderBy = new HashSet<>(); final List needsReverseList = new ArrayList<>(); - final List isNumericField = new ArrayList<>(); + final List dimensionTypes = new ArrayList<>(); final List comparators = new ArrayList<>(); for (OrderByColumnSpec orderSpec : limitSpec.getColumns()) { @@ -391,7 +391,7 @@ public class GroupByQuery extends BaseQuery dimsInOrderBy.add(dimIndex); needsReverseList.add(needsReverse); final ValueType type = dimensions.get(dimIndex).getOutputType(); - isNumericField.add(ValueType.isNumeric(type)); + dimensionTypes.add(type); comparators.add(orderSpec.getDimensionComparator()); } } @@ -401,7 +401,7 @@ public class GroupByQuery extends BaseQuery orderedFieldNames.add(dimensions.get(i).getOutputName()); needsReverseList.add(false); final ValueType type = dimensions.get(i).getOutputType(); - isNumericField.add(ValueType.isNumeric(type)); + dimensionTypes.add(type); comparators.add(StringComparators.LEXICOGRAPHIC); } } @@ -418,7 +418,7 @@ public class GroupByQuery extends BaseQuery return compareDimsForLimitPushDown( orderedFieldNames, needsReverseList, - isNumericField, + dimensionTypes, comparators, lhs, rhs @@ -436,7 +436,7 @@ public class GroupByQuery extends BaseQuery final int cmp = compareDimsForLimitPushDown( orderedFieldNames, needsReverseList, - isNumericField, + dimensionTypes, comparators, lhs, rhs @@ -465,7 +465,7 @@ public class GroupByQuery extends BaseQuery return compareDimsForLimitPushDown( orderedFieldNames, needsReverseList, - isNumericField, + dimensionTypes, comparators, lhs, rhs @@ -532,28 +532,12 @@ public class GroupByQuery extends BaseQuery private static int compareDims(List dimensions, Row lhs, Row rhs) { for (DimensionSpec dimension : dimensions) { - final int dimCompare; - if (dimension.getOutputType() == ValueType.LONG) { - dimCompare = Comparators.naturalNullsFirst().compare( - DimensionHandlerUtils.convertObjectToLong(lhs.getRaw(dimension.getOutputName())), - DimensionHandlerUtils.convertObjectToLong(rhs.getRaw(dimension.getOutputName())) - ); - } else if (dimension.getOutputType() == ValueType.FLOAT) { - dimCompare = Comparators.naturalNullsFirst().compare( - DimensionHandlerUtils.convertObjectToFloat(lhs.getRaw(dimension.getOutputName())), - DimensionHandlerUtils.convertObjectToFloat(rhs.getRaw(dimension.getOutputName())) - ); - } else if (dimension.getOutputType() == ValueType.DOUBLE) { - dimCompare = Comparators.naturalNullsFirst().compare( - DimensionHandlerUtils.convertObjectToDouble(lhs.getRaw(dimension.getOutputName())), - DimensionHandlerUtils.convertObjectToDouble(rhs.getRaw(dimension.getOutputName())) - ); - } else { - dimCompare = ((Ordering) Comparators.naturalNullsFirst()).compare( - lhs.getRaw(dimension.getOutputName()), - rhs.getRaw(dimension.getOutputName()) - ); - } + //noinspection unchecked + final int dimCompare = DimensionHandlerUtils.compareObjectsAsType( + lhs.getRaw(dimension.getOutputName()), + rhs.getRaw(dimension.getOutputName()), + dimension.getOutputType() + ); if (dimCompare != 0) { return dimCompare; } @@ -565,7 +549,7 @@ public class GroupByQuery extends BaseQuery private static int compareDimsForLimitPushDown( final List fields, final List needsReverseList, - final List isNumericField, + final List dimensionTypes, final List comparators, Row lhs, Row rhs @@ -574,17 +558,15 @@ public class GroupByQuery extends BaseQuery for (int i = 0; i < fields.size(); i++) { final String fieldName = fields.get(i); final StringComparator comparator = comparators.get(i); + final ValueType dimensionType = dimensionTypes.get(i); final int dimCompare; final Object lhsObj = lhs.getRaw(fieldName); final Object rhsObj = rhs.getRaw(fieldName); - if (isNumericField.get(i)) { + if (ValueType.isNumeric(dimensionType)) { if (comparator.equals(StringComparators.NUMERIC)) { - dimCompare = ((Ordering) Comparators.naturalNullsFirst()).compare( - lhsObj, - rhsObj - ); + dimCompare = DimensionHandlerUtils.compareObjectsAsType(lhsObj, rhsObj, dimensionType); } else { dimCompare = comparator.compare(String.valueOf(lhsObj), String.valueOf(rhsObj)); } diff --git a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java index ffdb9514176..7465d196e05 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQueryQueryToolChest.java @@ -58,6 +58,7 @@ import io.druid.query.extraction.ExtractionFn; import io.druid.query.groupby.resource.GroupByQueryResource; import io.druid.query.groupby.strategy.GroupByStrategy; import io.druid.query.groupby.strategy.GroupByStrategySelector; +import io.druid.segment.DimensionHandlerUtils; import org.joda.time.DateTime; import javax.annotation.Nullable; @@ -457,8 +458,13 @@ public class GroupByQueryQueryToolChest extends QueryToolChest event = Maps.newLinkedHashMap(); Iterator dimsIter = dims.iterator(); while (dimsIter.hasNext() && results.hasNext()) { - final DimensionSpec factory = dimsIter.next(); - event.put(factory.getOutputName(), results.next()); + final DimensionSpec dimensionSpec = dimsIter.next(); + + // Must convert generic Jackson-deserialized type into the proper type. + event.put( + dimensionSpec.getOutputName(), + DimensionHandlerUtils.convertObjectToType(results.next(), dimensionSpec.getOutputType()) + ); } Iterator aggsIter = aggs.iterator(); diff --git a/processing/src/main/java/io/druid/query/groupby/epinephelinae/GroupByQueryEngineV2.java b/processing/src/main/java/io/druid/query/groupby/epinephelinae/GroupByQueryEngineV2.java index 848086d0bdb..06fb34a8d7c 100644 --- a/processing/src/main/java/io/druid/query/groupby/epinephelinae/GroupByQueryEngineV2.java +++ b/processing/src/main/java/io/druid/query/groupby/epinephelinae/GroupByQueryEngineV2.java @@ -692,25 +692,7 @@ public class GroupByQueryEngineV2 final ValueType outputType = dimSpec.getOutputType(); rowMap.compute( dimSpec.getOutputName(), - (dimName, baseVal) -> { - switch (outputType) { - case STRING: - baseVal = DimensionHandlerUtils.convertObjectToString(baseVal); - break; - case LONG: - baseVal = DimensionHandlerUtils.convertObjectToLong(baseVal); - break; - case FLOAT: - baseVal = DimensionHandlerUtils.convertObjectToFloat(baseVal); - break; - case DOUBLE: - baseVal = DimensionHandlerUtils.convertObjectToDouble(baseVal); - break; - default: - throw new IAE("Unsupported type: " + outputType); - } - return baseVal; - } + (dimName, baseVal) -> DimensionHandlerUtils.convertObjectToType(baseVal, outputType) ); } } diff --git a/processing/src/main/java/io/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java b/processing/src/main/java/io/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java index 4b15af2e725..2ae1dde0e97 100644 --- a/processing/src/main/java/io/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java +++ b/processing/src/main/java/io/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java @@ -593,29 +593,10 @@ public class RowBasedGrouperHelper { final Function[] functions = new Function[valueTypes.size()]; for (int i = 0; i < functions.length; i++) { - ValueType type = valueTypes.get(i); // Subquery post-aggs aren't added to the rowSignature (see rowSignatureFor() in GroupByQueryHelper) because // their types aren't known, so default to String handling. - type = type == null ? ValueType.STRING : type; - switch (type) { - case STRING: - functions[i] = input -> DimensionHandlerUtils.convertObjectToString(input); - break; - - case LONG: - functions[i] = input -> DimensionHandlerUtils.convertObjectToLong(input); - break; - - case FLOAT: - functions[i] = input -> DimensionHandlerUtils.convertObjectToFloat(input); - break; - - case DOUBLE: - functions[i] = input -> DimensionHandlerUtils.convertObjectToDouble(input); - break; - default: - throw new IAE("invalid type: [%s]", type); - } + final ValueType type = valueTypes.get(i) == null ? ValueType.STRING : valueTypes.get(i); + functions[i] = input -> DimensionHandlerUtils.convertObjectToType(input, type); } return functions; } diff --git a/processing/src/main/java/io/druid/query/topn/DimExtractionTopNAlgorithm.java b/processing/src/main/java/io/druid/query/topn/DimExtractionTopNAlgorithm.java index 42d5a4a2799..e7b441a1beb 100644 --- a/processing/src/main/java/io/druid/query/topn/DimExtractionTopNAlgorithm.java +++ b/processing/src/main/java/io/druid/query/topn/DimExtractionTopNAlgorithm.java @@ -19,7 +19,6 @@ package io.druid.query.topn; -import com.google.common.base.Function; import io.druid.query.ColumnSelectorPlus; import io.druid.query.aggregation.Aggregator; import io.druid.query.topn.types.TopNColumnSelectorStrategy; @@ -110,14 +109,8 @@ public class DimExtractionTopNAlgorithm ) { final ColumnSelectorPlus selectorPlus = params.getSelectorPlus(); - final boolean needsResultTypeConversion = needsResultTypeConversion(params); - final Function valueTransformer = TopNMapFn.getValueTransformer( - query.getDimensionSpec().getOutputType() - ); - selectorPlus.getColumnSelectorStrategy().updateDimExtractionResults( aggregatesStore, - needsResultTypeConversion ? valueTransformer : null, resultBuilder ); } @@ -136,11 +129,4 @@ public class DimExtractionTopNAlgorithm public void cleanup(TopNParams params) { } - - private boolean needsResultTypeConversion(TopNParams params) - { - ColumnSelectorPlus selectorPlus = params.getSelectorPlus(); - TopNColumnSelectorStrategy strategy = selectorPlus.getColumnSelectorStrategy(); - return query.getDimensionSpec().getOutputType() != strategy.getValueType(); - } } diff --git a/processing/src/main/java/io/druid/query/topn/DimValHolder.java b/processing/src/main/java/io/druid/query/topn/DimValHolder.java index dacbd9626a3..3b212bbcf33 100644 --- a/processing/src/main/java/io/druid/query/topn/DimValHolder.java +++ b/processing/src/main/java/io/druid/query/topn/DimValHolder.java @@ -26,19 +26,19 @@ import java.util.Map; public class DimValHolder { private final Object topNMetricVal; - private final Comparable dimName; + private final Comparable dimValue; private final Object dimValIndex; private final Map metricValues; public DimValHolder( Object topNMetricVal, - Comparable dimName, + Comparable dimValue, Object dimValIndex, Map metricValues ) { this.topNMetricVal = topNMetricVal; - this.dimName = dimName; + this.dimValue = dimValue; this.dimValIndex = dimValIndex; this.metricValues = metricValues; } @@ -48,9 +48,9 @@ public class DimValHolder return topNMetricVal; } - public Comparable getDimName() + public Comparable getDimValue() { - return dimName; + return dimValue; } public Object getDimValIndex() @@ -66,14 +66,14 @@ public class DimValHolder public static class Builder { private Object topNMetricVal; - private Comparable dimName; + private Comparable dimValue; private Object dimValIndex; private Map metricValues; public Builder() { topNMetricVal = null; - dimName = null; + dimValue = null; dimValIndex = null; metricValues = null; } @@ -84,9 +84,9 @@ public class DimValHolder return this; } - public Builder withDimName(Comparable dimName) + public Builder withDimValue(Comparable dimValue) { - this.dimName = dimName; + this.dimValue = dimValue; return this; } @@ -104,7 +104,7 @@ public class DimValHolder public DimValHolder build() { - return new DimValHolder(topNMetricVal, dimName, dimValIndex, metricValues); + return new DimValHolder(topNMetricVal, dimValue, dimValIndex, metricValues); } } } diff --git a/processing/src/main/java/io/druid/query/topn/PooledTopNAlgorithm.java b/processing/src/main/java/io/druid/query/topn/PooledTopNAlgorithm.java index eb92dfbd891..bd37474e658 100644 --- a/processing/src/main/java/io/druid/query/topn/PooledTopNAlgorithm.java +++ b/processing/src/main/java/io/druid/query/topn/PooledTopNAlgorithm.java @@ -20,7 +20,6 @@ package io.druid.query.topn; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Function; import com.google.common.collect.ImmutableMap; import io.druid.collections.NonBlockingPool; import io.druid.collections.ResourceHolder; @@ -37,7 +36,6 @@ import io.druid.segment.Cursor; import io.druid.segment.DimensionSelector; import io.druid.segment.FilteredOffset; import io.druid.segment.StorageAdapter; -import io.druid.segment.column.ValueType; import io.druid.segment.data.IndexedInts; import io.druid.segment.data.Offset; import io.druid.segment.historical.HistoricalColumnSelector; @@ -736,10 +734,6 @@ public class PooledTopNAlgorithm final int[] aggregatorSizes = params.getAggregatorSizes(); final DimensionSelector dimSelector = params.getDimSelector(); - final ValueType outType = query.getDimensionSpec().getOutputType(); - final boolean needsResultConversion = outType != ValueType.STRING; - final Function valueTransformer = TopNMapFn.getValueTransformer(outType); - for (int i = 0; i < positions.length; i++) { int position = positions[i]; if (position >= 0) { @@ -749,14 +743,9 @@ public class PooledTopNAlgorithm position += aggregatorSizes[j]; } - Object retVal = dimSelector.lookupName(i); - if (needsResultConversion) { - retVal = valueTransformer.apply(retVal); - } - - + // Output type must be STRING in order for PooledTopNAlgorithm to make sense; so no need to convert value. resultBuilder.addEntry( - (Comparable) retVal, + dimSelector.lookupName(i), i, vals ); @@ -854,18 +843,6 @@ public class PooledTopNAlgorithm private int numValuesPerPass; private TopNMetricSpecBuilder arrayProvider; - public Builder() - { - selectorPlus = null; - cursor = null; - resultsBufHolder = null; - resultsBuf = null; - aggregatorSizes = null; - numBytesPerRecord = 0; - numValuesPerPass = 0; - arrayProvider = null; - } - public Builder withSelectorPlus(ColumnSelectorPlus selectorPlus) { this.selectorPlus = selectorPlus; diff --git a/processing/src/main/java/io/druid/query/topn/TopNLexicographicResultBuilder.java b/processing/src/main/java/io/druid/query/topn/TopNLexicographicResultBuilder.java index 7d5f4650c7b..a5f884090b2 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNLexicographicResultBuilder.java +++ b/processing/src/main/java/io/druid/query/topn/TopNLexicographicResultBuilder.java @@ -19,7 +19,6 @@ package io.druid.query.topn; -import com.google.common.base.Function; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import io.druid.query.Result; @@ -66,32 +65,22 @@ public class TopNLexicographicResultBuilder implements TopNResultBuilder this.threshold = threshold; this.pQueue = new PriorityQueue<>( threshold + 1, - new Comparator() - { - @Override - public int compare( - DimValHolder o1, - DimValHolder o2 - ) - { - return comparator.compare(o2.getDimName(), o1.getDimName()); - } - } + (o1, o2) -> comparator.compare(o2.getDimValue(), o1.getDimValue()) ); } @Override public TopNResultBuilder addEntry( - Comparable dimNameObj, + Comparable dimValueObj, Object dimValIndex, Object[] metricVals ) { - final String dimName = Objects.toString(dimNameObj, null); + final String dimValue = Objects.toString(dimValueObj, null); final Map metricValues = Maps.newHashMapWithExpectedSize(metricVals.length + 1); - if (shouldAdd(dimName)) { - metricValues.put(dimSpec.getOutputName(), dimName); + if (shouldAdd(dimValue)) { + metricValues.put(dimSpec.getOutputName(), dimValueObj); final int extra = metricVals.length % LOOP_UNROLL_COUNT; switch (extra) { case 7: @@ -126,7 +115,7 @@ public class TopNLexicographicResultBuilder implements TopNResultBuilder metricValues.put(aggFactoryNames[i + 7], metricVals[i + 7]); } - pQueue.add(new DimValHolder.Builder().withDimName(dimName).withMetricValues(metricValues).build()); + pQueue.add(new DimValHolder.Builder().withDimValue(dimValue).withMetricValues(metricValues).build()); if (pQueue.size() > threshold) { pQueue.poll(); } @@ -143,7 +132,7 @@ public class TopNLexicographicResultBuilder implements TopNResultBuilder if (shouldAdd(dimensionValue)) { pQueue.add( - new DimValHolder.Builder().withDimName(dimensionValue) + new DimValHolder.Builder().withDimValue(dimensionValue) .withMetricValues(dimensionAndMetricValueExtractor.getBaseObject()) .build() ); @@ -167,30 +156,11 @@ public class TopNLexicographicResultBuilder implements TopNResultBuilder final DimValHolder[] holderValueArray = pQueue.toArray(new DimValHolder[0]); Arrays.sort( holderValueArray, - new Comparator() - { - @Override - public int compare(DimValHolder o1, DimValHolder o2) - { - return comparator.compare(o1.getDimName(), o2.getDimName()); - } - } - + (o1, o2) -> comparator.compare(o1.getDimValue(), o2.getDimValue()) ); - return new Result( - timestamp, new TopNResultValue( - Lists.transform( - Arrays.asList(holderValueArray), - new Function() - { - @Override - public Object apply(DimValHolder dimValHolder) - { - return dimValHolder.getMetricValues(); - } - } - ) - ) + return new Result<>( + timestamp, + new TopNResultValue(Lists.transform(Arrays.asList(holderValueArray), DimValHolder::getMetricValues)) ); } diff --git a/processing/src/main/java/io/druid/query/topn/TopNMapFn.java b/processing/src/main/java/io/druid/query/topn/TopNMapFn.java index 4725ff8d4b9..9ca4c5a273b 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNMapFn.java +++ b/processing/src/main/java/io/druid/query/topn/TopNMapFn.java @@ -19,46 +19,16 @@ package io.druid.query.topn; -import com.google.common.base.Function; -import io.druid.java.util.common.IAE; import io.druid.query.ColumnSelectorPlus; import io.druid.query.Result; import io.druid.query.topn.types.TopNColumnSelectorStrategyFactory; import io.druid.segment.Cursor; import io.druid.segment.DimensionHandlerUtils; -import io.druid.segment.column.ValueType; import javax.annotation.Nullable; -import java.util.Objects; public class TopNMapFn { - public static Function getValueTransformer(ValueType outputType) - { - switch (outputType) { - case STRING: - return STRING_TRANSFORMER; - case LONG: - return LONG_TRANSFORMER; - case FLOAT: - return FLOAT_TRANSFORMER; - case DOUBLE: - return DOUBLE_TRANSFORMER; - default: - throw new IAE("invalid type: %s", outputType); - } - } - - private static Function STRING_TRANSFORMER = Objects::toString; - - private static Function LONG_TRANSFORMER = DimensionHandlerUtils::convertObjectToLong; - - private static Function FLOAT_TRANSFORMER = DimensionHandlerUtils::convertObjectToFloat; - - private static Function DOUBLE_TRANSFORMER = DimensionHandlerUtils::convertObjectToDouble; - - private static final TopNColumnSelectorStrategyFactory STRATEGY_FACTORY = new TopNColumnSelectorStrategyFactory(); - private final TopNQuery query; private final TopNAlgorithm topNAlgorithm; @@ -75,7 +45,7 @@ public class TopNMapFn public Result apply(final Cursor cursor, final @Nullable TopNQueryMetrics queryMetrics) { final ColumnSelectorPlus selectorPlus = DimensionHandlerUtils.createColumnSelectorPlus( - STRATEGY_FACTORY, + new TopNColumnSelectorStrategyFactory(query.getDimensionSpec().getOutputType()), query.getDimensionSpec(), cursor.getColumnSelectorFactory() ); diff --git a/processing/src/main/java/io/druid/query/topn/TopNNumericResultBuilder.java b/processing/src/main/java/io/druid/query/topn/TopNNumericResultBuilder.java index e828e9ee1ee..f1a35725e3f 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNNumericResultBuilder.java +++ b/processing/src/main/java/io/druid/query/topn/TopNNumericResultBuilder.java @@ -19,7 +19,6 @@ package io.druid.query.topn; -import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -48,9 +47,9 @@ public class TopNNumericResultBuilder implements TopNResultBuilder private final String metricName; private final List postAggs; private final PriorityQueue pQueue; - private final Comparator dimValComparator; + private final Comparator dimValHolderComparator; private final String[] aggFactoryNames; - private static final Comparator dimNameComparator = new Comparator() + private static final Comparator dimValueComparator = new Comparator() { @Override public int compare(Comparable o1, Comparable o2) @@ -65,6 +64,7 @@ public class TopNNumericResultBuilder implements TopNResultBuilder } else if (null == o2) { retval = 1; } else { + //noinspection unchecked retval = o1.compareTo(o2); } return retval; @@ -91,30 +91,26 @@ public class TopNNumericResultBuilder implements TopNResultBuilder this.postAggs = AggregatorUtil.pruneDependentPostAgg(postAggs, this.metricName); this.threshold = threshold; this.metricComparator = comparator; - this.dimValComparator = new Comparator() - { - @Override - public int compare(DimValHolder d1, DimValHolder d2) - { - int retVal = metricComparator.compare(d1.getTopNMetricVal(), d2.getTopNMetricVal()); + this.dimValHolderComparator = (d1, d2) -> { + //noinspection unchecked + int retVal = metricComparator.compare(d1.getTopNMetricVal(), d2.getTopNMetricVal()); - if (retVal == 0) { - retVal = dimNameComparator.compare(d1.getDimName(), d2.getDimName()); - } - - return retVal; + if (retVal == 0) { + retVal = dimValueComparator.compare(d1.getDimValue(), d2.getDimValue()); } + + return retVal; }; // The logic in addEntry first adds, then removes if needed. So it can at any point have up to threshold + 1 entries. - pQueue = new PriorityQueue<>(this.threshold + 1, this.dimValComparator); + pQueue = new PriorityQueue<>(this.threshold + 1, dimValHolderComparator); } private static final int LOOP_UNROLL_COUNT = 8; @Override public TopNNumericResultBuilder addEntry( - Comparable dimName, + Comparable dimValueObj, Object dimValIndex, Object[] metricVals ) @@ -126,7 +122,7 @@ public class TopNNumericResultBuilder implements TopNResultBuilder final Map metricValues = Maps.newHashMapWithExpectedSize(metricVals.length + postAggs.size() + 1); - metricValues.put(dimSpec.getOutputName(), dimName); + metricValues.put(dimSpec.getOutputName(), dimValueObj); final int extra = metricVals.length % LOOP_UNROLL_COUNT; @@ -173,7 +169,7 @@ public class TopNNumericResultBuilder implements TopNResultBuilder if (shouldAdd(topNMetricVal)) { DimValHolder dimValHolder = new DimValHolder.Builder() .withTopNMetricVal(topNMetricVal) - .withDimName(dimName) + .withDimValue(dimValueObj) .withDimValIndex(dimValIndex) .withMetricValues(metricValues) .build(); @@ -202,7 +198,7 @@ public class TopNNumericResultBuilder implements TopNResultBuilder if (shouldAdd(dimValue)) { final DimValHolder valHolder = new DimValHolder.Builder() .withTopNMetricVal(dimValue) - .withDimName((Comparable) dimensionAndMetricValueExtractor.getDimensionValue(dimSpec.getOutputName())) + .withDimValue((Comparable) dimensionAndMetricValueExtractor.getDimensionValue(dimSpec.getOutputName())) .withMetricValues(dimensionAndMetricValueExtractor.getBaseObject()) .build(); pQueue.add(valHolder); @@ -224,39 +220,24 @@ public class TopNNumericResultBuilder implements TopNResultBuilder { final DimValHolder[] holderValueArray = pQueue.toArray(new DimValHolder[0]); Arrays.sort( - holderValueArray, new Comparator() - { - @Override - public int compare(DimValHolder d1, DimValHolder d2) - { - // Values flipped compared to earlier - int retVal = metricComparator.compare(d2.getTopNMetricVal(), d1.getTopNMetricVal()); + holderValueArray, + (d1, d2) -> { + // Metric values flipped compared to dimValueHolderComparator. - if (retVal == 0) { - retVal = dimNameComparator.compare(d1.getDimName(), d2.getDimName()); - } + //noinspection unchecked + int retVal = metricComparator.compare(d2.getTopNMetricVal(), d1.getTopNMetricVal()); - return retVal; + if (retVal == 0) { + retVal = dimValueComparator.compare(d1.getDimValue(), d2.getDimValue()); } + + return retVal; } ); List holderValues = Arrays.asList(holderValueArray); // Pull out top aggregated values - final List> values = Lists.transform( - holderValues, - new Function>() - { - @Override - public Map apply(DimValHolder valHolder) - { - return valHolder.getMetricValues(); - } - } - ); - return new Result( - timestamp, - new TopNResultValue(values) - ); + final List> values = Lists.transform(holderValues, DimValHolder::getMetricValues); + return new Result<>(timestamp, new TopNResultValue(values)); } } diff --git a/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java b/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java index 82057973149..96b2a3d65b3 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java @@ -142,6 +142,10 @@ public class TopNQueryEngine && columnCapabilities.isDictionaryEncoded())) { // Use DimExtraction for non-Strings and for non-dictionary-encoded Strings. topNAlgorithm = new DimExtractionTopNAlgorithm(adapter, query); + } else if (query.getDimensionSpec().getOutputType() != ValueType.STRING) { + // Use DimExtraction when the dimension output type is a non-String. (It's like an extractionFn: there can be + // a many-to-one mapping, since numeric types can't represent all possible values of other types.) + topNAlgorithm = new DimExtractionTopNAlgorithm(adapter, query); } else if (selector.isAggregateAllMetrics()) { topNAlgorithm = new PooledTopNAlgorithm(adapter, query, bufferPool); } else if (selector.isAggregateTopNMetricFirst() || query.getContextBoolean("doAggregateTopNMetricFirst", false)) { diff --git a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java index 2d094abde47..36331f67ed3 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java @@ -50,6 +50,7 @@ import io.druid.query.aggregation.PostAggregator; import io.druid.query.cache.CacheKeyBuilder; import io.druid.query.dimension.DefaultDimensionSpec; import io.druid.query.dimension.DimensionSpec; +import io.druid.segment.DimensionHandlerUtils; import org.joda.time.DateTime; import javax.annotation.Nullable; @@ -388,11 +389,6 @@ public class TopNQueryQueryToolChest extends QueryToolChest inputIter = results.iterator(); DateTime timestamp = granularity.toDateTime(((Number) inputIter.next()).longValue()); - // Need a value transformer to convert generic Jackson-deserialized type into the proper type. - final Function dimValueTransformer = TopNMapFn.getValueTransformer( - query.getDimensionSpec().getOutputType() - ); - while (inputIter.hasNext()) { List result = (List) inputIter.next(); Map vals = Maps.newLinkedHashMap(); @@ -400,7 +396,11 @@ public class TopNQueryQueryToolChest extends QueryToolChest aggIter = aggs.iterator(); Iterator resultIter = result.iterator(); - vals.put(query.getDimensionSpec().getOutputName(), dimValueTransformer.apply(resultIter.next())); + // Must convert generic Jackson-deserialized type into the proper type. + vals.put( + query.getDimensionSpec().getOutputName(), + DimensionHandlerUtils.convertObjectToType(resultIter.next(), query.getDimensionSpec().getOutputType()) + ); while (aggIter.hasNext() && resultIter.hasNext()) { final AggregatorFactory factory = aggIter.next(); diff --git a/processing/src/main/java/io/druid/query/topn/TopNResultBuilder.java b/processing/src/main/java/io/druid/query/topn/TopNResultBuilder.java index d5d40ab7fcc..4fba6772c13 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNResultBuilder.java +++ b/processing/src/main/java/io/druid/query/topn/TopNResultBuilder.java @@ -28,7 +28,7 @@ import java.util.Iterator; public interface TopNResultBuilder { TopNResultBuilder addEntry( - Comparable dimNameObj, + Comparable dimValueObj, Object dimValIndex, Object[] metricVals ); diff --git a/processing/src/main/java/io/druid/query/topn/types/NumericTopNColumnSelectorStrategy.java b/processing/src/main/java/io/druid/query/topn/types/NumericTopNColumnSelectorStrategy.java index 3672dabebb4..7648e1a3bdc 100644 --- a/processing/src/main/java/io/druid/query/topn/types/NumericTopNColumnSelectorStrategy.java +++ b/processing/src/main/java/io/druid/query/topn/types/NumericTopNColumnSelectorStrategy.java @@ -19,7 +19,7 @@ package io.druid.query.topn.types; -import com.google.common.base.Function; +import io.druid.java.util.common.IAE; import io.druid.query.aggregation.Aggregator; import io.druid.query.topn.BaseTopNAlgorithm; import io.druid.query.topn.TopNParams; @@ -29,6 +29,7 @@ import io.druid.segment.BaseDoubleColumnValueSelector; import io.druid.segment.BaseFloatColumnValueSelector; import io.druid.segment.BaseLongColumnValueSelector; import io.druid.segment.Cursor; +import io.druid.segment.DimensionHandlerUtils; import io.druid.segment.StorageAdapter; import io.druid.segment.column.ValueType; import it.unimi.dsi.fastutil.ints.Int2ObjectMap; @@ -37,12 +38,32 @@ import it.unimi.dsi.fastutil.longs.Long2ObjectMap; import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap; import java.util.Map; +import java.util.function.Function; public abstract class NumericTopNColumnSelectorStrategy< ValueSelectorType, DimExtractionAggregateStoreType extends Map> implements TopNColumnSelectorStrategy { + public static TopNColumnSelectorStrategy ofType(final ValueType selectorType, final ValueType dimensionType) + { + final Function> converter = DimensionHandlerUtils.converterFromTypeToType( + selectorType, + dimensionType + ); + + switch (selectorType) { + case LONG: + return new OfLong(converter); + case FLOAT: + return new OfFloat(converter); + case DOUBLE: + return new OfDouble(converter); + default: + throw new IAE("No strategy for type[%s]", selectorType); + } + } + @Override public int getCardinality(ValueSelectorType selector) { @@ -132,7 +153,6 @@ public abstract class NumericTopNColumnSelectorStrategy< @Override public void updateDimExtractionResults( final DimExtractionAggregateStoreType aggregatesStore, - final Function valueTransformer, final TopNResultBuilder resultBuilder ) { @@ -144,11 +164,7 @@ public abstract class NumericTopNColumnSelectorStrategy< vals[i] = aggs[i].get(); } - Comparable key = convertAggregatorStoreKeyToColumnValue(entry.getKey()); - if (valueTransformer != null) { - key = (Comparable) valueTransformer.apply(key); - } - + final Comparable key = convertAggregatorStoreKeyToColumnValue(entry.getKey()); resultBuilder.addEntry(key, key, vals); } } @@ -159,10 +175,11 @@ public abstract class NumericTopNColumnSelectorStrategy< static class OfFloat extends NumericTopNColumnSelectorStrategy> { - @Override - public ValueType getValueType() + private final Function> converter; + + OfFloat(final Function> converter) { - return ValueType.FLOAT; + this.converter = converter; } @Override @@ -174,7 +191,7 @@ public abstract class NumericTopNColumnSelectorStrategy< @Override Comparable convertAggregatorStoreKeyToColumnValue(Object aggregatorStoreKey) { - return Float.intBitsToFloat((Integer) aggregatorStoreKey); + return converter.apply(Float.intBitsToFloat((Integer) aggregatorStoreKey)); } @Override @@ -193,10 +210,11 @@ public abstract class NumericTopNColumnSelectorStrategy< static class OfLong extends NumericTopNColumnSelectorStrategy> { - @Override - public ValueType getValueType() + private final Function> converter; + + OfLong(final Function> converter) { - return ValueType.LONG; + this.converter = converter; } @Override @@ -208,7 +226,7 @@ public abstract class NumericTopNColumnSelectorStrategy< @Override Comparable convertAggregatorStoreKeyToColumnValue(Object aggregatorStoreKey) { - return (Long) aggregatorStoreKey; + return converter.apply(aggregatorStoreKey); } @Override @@ -227,10 +245,11 @@ public abstract class NumericTopNColumnSelectorStrategy< static class OfDouble extends NumericTopNColumnSelectorStrategy> { - @Override - public ValueType getValueType() + private final Function> converter; + + OfDouble(final Function> converter) { - return ValueType.DOUBLE; + this.converter = converter; } @Override @@ -242,7 +261,7 @@ public abstract class NumericTopNColumnSelectorStrategy< @Override Comparable convertAggregatorStoreKeyToColumnValue(Object aggregatorStoreKey) { - return Double.longBitsToDouble((Long) aggregatorStoreKey); + return converter.apply(Double.longBitsToDouble((Long) aggregatorStoreKey)); } @Override diff --git a/processing/src/main/java/io/druid/query/topn/types/StringTopNColumnSelectorStrategy.java b/processing/src/main/java/io/druid/query/topn/types/StringTopNColumnSelectorStrategy.java index 20f9b920e14..ed89bbda1b0 100644 --- a/processing/src/main/java/io/druid/query/topn/types/StringTopNColumnSelectorStrategy.java +++ b/processing/src/main/java/io/druid/query/topn/types/StringTopNColumnSelectorStrategy.java @@ -19,36 +19,38 @@ package io.druid.query.topn.types; -import com.google.common.base.Function; -import com.google.common.collect.Maps; import io.druid.query.aggregation.Aggregator; import io.druid.query.topn.BaseTopNAlgorithm; import io.druid.query.topn.TopNParams; import io.druid.query.topn.TopNQuery; import io.druid.query.topn.TopNResultBuilder; import io.druid.segment.Cursor; +import io.druid.segment.DimensionHandlerUtils; import io.druid.segment.DimensionSelector; import io.druid.segment.StorageAdapter; import io.druid.segment.column.ValueType; import io.druid.segment.data.IndexedInts; +import java.util.HashMap; import java.util.Map; +import java.util.function.Function; public class StringTopNColumnSelectorStrategy - implements TopNColumnSelectorStrategy> + implements TopNColumnSelectorStrategy> { + private final Function> dimensionValueConverter; + + public StringTopNColumnSelectorStrategy(final ValueType dimensionType) + { + this.dimensionValueConverter = DimensionHandlerUtils.converterFromTypeToType(ValueType.STRING, dimensionType); + } + @Override public int getCardinality(DimensionSelector selector) { return selector.getValueCardinality(); } - @Override - public ValueType getValueType() - { - return ValueType.STRING; - } - @Override public Aggregator[][] getDimExtractionRowSelector(TopNQuery query, TopNParams params, StorageAdapter storageAdapter) { @@ -71,9 +73,9 @@ public class StringTopNColumnSelectorStrategy } @Override - public Map makeDimExtractionAggregateStore() + public Map makeDimExtractionAggregateStore() { - return Maps.newHashMap(); + return new HashMap<>(); } @Override @@ -82,7 +84,7 @@ public class StringTopNColumnSelectorStrategy DimensionSelector selector, Cursor cursor, Aggregator[][] rowSelector, - Map aggregatesStore + Map aggregatesStore ) { if (selector.getValueCardinality() != DimensionSelector.CARDINALITY_UNKNOWN) { @@ -94,12 +96,11 @@ public class StringTopNColumnSelectorStrategy @Override public void updateDimExtractionResults( - final Map aggregatesStore, - final Function valueTransformer, + final Map aggregatesStore, final TopNResultBuilder resultBuilder ) { - for (Map.Entry entry : aggregatesStore.entrySet()) { + for (Map.Entry entry : aggregatesStore.entrySet()) { Aggregator[] aggs = entry.getValue(); if (aggs != null) { Object[] vals = new Object[aggs.length]; @@ -107,16 +108,8 @@ public class StringTopNColumnSelectorStrategy vals[i] = aggs[i].get(); } - Comparable key = entry.getKey(); - if (valueTransformer != null) { - key = (Comparable) valueTransformer.apply(key); - } - - resultBuilder.addEntry( - key, - key, - vals - ); + final Comparable key = dimensionValueConverter.apply(entry.getKey()); + resultBuilder.addEntry(key, key, vals); } } } @@ -126,7 +119,7 @@ public class StringTopNColumnSelectorStrategy Cursor cursor, DimensionSelector selector, Aggregator[][] rowSelector, - Map aggregatesStore + Map aggregatesStore ) { long processedRows = 0; @@ -136,7 +129,7 @@ public class StringTopNColumnSelectorStrategy final int dimIndex = dimValues.get(i); Aggregator[] theAggregators = rowSelector[dimIndex]; if (theAggregators == null) { - final String key = selector.lookupName(dimIndex); + final Comparable key = dimensionValueConverter.apply(selector.lookupName(dimIndex)); theAggregators = aggregatesStore.get(key); if (theAggregators == null) { theAggregators = BaseTopNAlgorithm.makeAggregators(cursor, query.getAggregatorSpecs()); @@ -159,7 +152,7 @@ public class StringTopNColumnSelectorStrategy TopNQuery query, Cursor cursor, DimensionSelector selector, - Map aggregatesStore + Map aggregatesStore ) { long processedRows = 0; @@ -167,7 +160,7 @@ public class StringTopNColumnSelectorStrategy final IndexedInts dimValues = selector.getRow(); for (int i = 0, size = dimValues.size(); i < size; ++i) { final int dimIndex = dimValues.get(i); - final String key = selector.lookupName(dimIndex); + final Comparable key = dimensionValueConverter.apply(selector.lookupName(dimIndex)); Aggregator[] theAggregators = aggregatesStore.get(key); if (theAggregators == null) { diff --git a/processing/src/main/java/io/druid/query/topn/types/TopNColumnSelectorStrategy.java b/processing/src/main/java/io/druid/query/topn/types/TopNColumnSelectorStrategy.java index 1aae847323b..40f499ee03f 100644 --- a/processing/src/main/java/io/druid/query/topn/types/TopNColumnSelectorStrategy.java +++ b/processing/src/main/java/io/druid/query/topn/types/TopNColumnSelectorStrategy.java @@ -19,7 +19,6 @@ package io.druid.query.topn.types; -import com.google.common.base.Function; import io.druid.query.aggregation.Aggregator; import io.druid.query.dimension.ColumnSelectorStrategy; import io.druid.query.topn.TopNParams; @@ -27,9 +26,7 @@ import io.druid.query.topn.TopNQuery; import io.druid.query.topn.TopNResultBuilder; import io.druid.segment.Cursor; import io.druid.segment.StorageAdapter; -import io.druid.segment.column.ValueType; -import javax.annotation.Nullable; import java.util.Map; public interface TopNColumnSelectorStrategy @@ -39,8 +36,6 @@ public interface TopNColumnSelectorStrategy valueTransformer, TopNResultBuilder resultBuilder ); } diff --git a/processing/src/main/java/io/druid/query/topn/types/TopNColumnSelectorStrategyFactory.java b/processing/src/main/java/io/druid/query/topn/types/TopNColumnSelectorStrategyFactory.java index 492d27efbcb..08e76ff919e 100644 --- a/processing/src/main/java/io/druid/query/topn/types/TopNColumnSelectorStrategyFactory.java +++ b/processing/src/main/java/io/druid/query/topn/types/TopNColumnSelectorStrategyFactory.java @@ -19,6 +19,7 @@ package io.druid.query.topn.types; +import com.google.common.base.Preconditions; import io.druid.java.util.common.IAE; import io.druid.query.dimension.ColumnSelectorStrategyFactory; import io.druid.segment.ColumnValueSelector; @@ -27,23 +28,39 @@ import io.druid.segment.column.ValueType; public class TopNColumnSelectorStrategyFactory implements ColumnSelectorStrategyFactory { + private final ValueType dimensionType; + + public TopNColumnSelectorStrategyFactory(final ValueType dimensionType) + { + this.dimensionType = Preconditions.checkNotNull(dimensionType, "dimensionType"); + } + @Override public TopNColumnSelectorStrategy makeColumnSelectorStrategy( ColumnCapabilities capabilities, ColumnValueSelector selector ) { - ValueType type = capabilities.getType(); - switch (type) { + final ValueType selectorType = capabilities.getType(); + + switch (selectorType) { case STRING: - return new StringTopNColumnSelectorStrategy(); + // Return strategy that reads strings and outputs dimensionTypes. + return new StringTopNColumnSelectorStrategy(dimensionType); case LONG: - return new NumericTopNColumnSelectorStrategy.OfLong(); case FLOAT: - return new NumericTopNColumnSelectorStrategy.OfFloat(); case DOUBLE: - return new NumericTopNColumnSelectorStrategy.OfDouble(); + if (ValueType.isNumeric(dimensionType)) { + // Return strategy that aggregates using the _output_ type, because this allows us to collapse values + // properly (numeric types cannot represent all values of other numeric types). + return NumericTopNColumnSelectorStrategy.ofType(dimensionType, dimensionType); + } else { + // Return strategy that aggregates using the _input_ type. Here we are assuming that the output type can + // represent all possible values of the input type. This will be true for STRING, which is the only + // non-numeric type currently supported. + return NumericTopNColumnSelectorStrategy.ofType(selectorType, dimensionType); + } default: - throw new IAE("Cannot create query type helper from invalid type [%s]", type); + throw new IAE("Cannot create query type helper from invalid type [%s]", selectorType); } } } diff --git a/processing/src/main/java/io/druid/segment/DimensionHandlerUtils.java b/processing/src/main/java/io/druid/segment/DimensionHandlerUtils.java index a823e25675f..c78715b442b 100644 --- a/processing/src/main/java/io/druid/segment/DimensionHandlerUtils.java +++ b/processing/src/main/java/io/druid/segment/DimensionHandlerUtils.java @@ -19,12 +19,14 @@ package io.druid.segment; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.primitives.Doubles; import com.google.common.primitives.Floats; import io.druid.common.guava.GuavaUtils; import io.druid.data.input.impl.DimensionSchema.MultiValueHandling; import io.druid.java.util.common.IAE; +import io.druid.java.util.common.guava.Comparators; import io.druid.java.util.common.parsers.ParseException; import io.druid.query.ColumnSelectorPlus; import io.druid.query.dimension.ColumnSelectorStrategy; @@ -38,6 +40,7 @@ import javax.annotation.Nullable; import java.math.BigDecimal; import java.util.ArrayList; import java.util.List; +import java.util.function.Function; public final class DimensionHandlerUtils { @@ -132,9 +135,10 @@ public final class DimensionHandlerUtils * in a query engine. See GroupByStrategyFactory for a reference. * * @param The strategy type created by the provided strategy factory. - * @param strategyFactory A factory provided by query engines that generates type-handling strategies - * @param dimensionSpecs The set of columns to generate ColumnSelectorPlus objects for - * @param columnSelectorFactory Used to create value selectors for columns. + * @param strategyFactory A factory provided by query engines that generates type-handling strategies + * @param dimensionSpecs The set of columns to generate ColumnSelectorPlus objects for + * @param columnSelectorFactory Used to create value selectors for columns. + * * @return An array of ColumnSelectorPlus objects, in the order of the columns specified in dimensionSpecs */ public static @@ -302,6 +306,64 @@ public final class DimensionHandlerUtils } } + @Nullable + public static Comparable convertObjectToType( + @Nullable final Object obj, + final ValueType type, + final boolean reportParseExceptions + ) + { + Preconditions.checkNotNull(type, "type"); + + switch (type) { + case LONG: + return convertObjectToLong(obj, reportParseExceptions); + case FLOAT: + return convertObjectToFloat(obj, reportParseExceptions); + case DOUBLE: + return convertObjectToDouble(obj, reportParseExceptions); + case STRING: + return convertObjectToString(obj); + default: + throw new IAE("Type[%s] is not supported for dimensions!", type); + } + } + + public static int compareObjectsAsType( + @Nullable final Object lhs, + @Nullable final Object rhs, + final ValueType type + ) + { + //noinspection unchecked + return Comparators.naturalNullsFirst().compare( + convertObjectToType(lhs, type), + convertObjectToType(rhs, type) + ); + } + + @Nullable + public static Comparable convertObjectToType( + @Nullable final Object obj, + final ValueType type + ) + { + return convertObjectToType(obj, Preconditions.checkNotNull(type, "type"), false); + } + + public static Function> converterFromTypeToType( + final ValueType fromType, + final ValueType toType + ) + { + if (fromType == toType) { + //noinspection unchecked + return (Function) Function.identity(); + } else { + return obj -> convertObjectToType(obj, toType); + } + } + @Nullable public static Double convertObjectToDouble(@Nullable Object valObj) { diff --git a/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java b/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java index e347b95c02e..8dc5c159650 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java @@ -5133,6 +5133,138 @@ public class TopNQueryRunnerTest assertExpectedResults(expectedResults, query); } + @Test + public void testSortOnDoubleAsLong() + { + TopNQuery query = new TopNQueryBuilder() + .dataSource(QueryRunnerTestHelper.dataSource) + .granularity(QueryRunnerTestHelper.allGran) + .dimension(new DefaultDimensionSpec("index", "index_alias", ValueType.LONG)) + .metric(new DimensionTopNMetricSpec(null, StringComparators.NUMERIC)) + .threshold(4) + .intervals(QueryRunnerTestHelper.fullOnInterval) + .build(); + + List> expectedResults = Collections.singletonList( + new Result<>( + DateTimes.of("2011-01-12T00:00:00.000Z"), + new TopNResultValue( + Arrays.>asList( + ImmutableMap.builder() + .put("index_alias", 59L) + .build(), + ImmutableMap.builder() + .put("index_alias", 67L) + .build(), + ImmutableMap.builder() + .put("index_alias", 68L) + .build(), + ImmutableMap.builder() + .put("index_alias", 69L) + .build() + ) + ) + ) + ); + assertExpectedResults(expectedResults, query); + } + + @Test + public void testSortOnTimeAsLong() + { + TopNQuery query = new TopNQueryBuilder() + .dataSource(QueryRunnerTestHelper.dataSource) + .granularity(QueryRunnerTestHelper.allGran) + .dimension(new DefaultDimensionSpec("__time", "__time_alias", ValueType.LONG)) + .metric(new DimensionTopNMetricSpec(null, StringComparators.NUMERIC)) + .threshold(4) + .intervals(QueryRunnerTestHelper.fullOnInterval) + .build(); + + List> expectedResults = Collections.singletonList( + new Result<>( + DateTimes.of("2011-01-12T00:00:00.000Z"), + new TopNResultValue( + Arrays.>asList( + ImmutableMap.builder() + .put("__time_alias", DateTimes.of("2011-01-12T00:00:00.000Z").getMillis()) + .build(), + ImmutableMap.builder() + .put("__time_alias", DateTimes.of("2011-01-13T00:00:00.000Z").getMillis()) + .build(), + ImmutableMap.builder() + .put("__time_alias", DateTimes.of("2011-01-14T00:00:00.000Z").getMillis()) + .build(), + ImmutableMap.builder() + .put("__time_alias", DateTimes.of("2011-01-15T00:00:00.000Z").getMillis()) + .build() + ) + ) + ) + ); + assertExpectedResults(expectedResults, query); + } + + @Test + public void testSortOnStringAsDouble() + { + TopNQuery query = new TopNQueryBuilder() + .dataSource(QueryRunnerTestHelper.dataSource) + .granularity(QueryRunnerTestHelper.allGran) + .dimension(new DefaultDimensionSpec("market", "alias", ValueType.DOUBLE)) + .metric(new DimensionTopNMetricSpec(null, StringComparators.NUMERIC)) + .threshold(4) + .intervals(QueryRunnerTestHelper.fullOnInterval) + .build(); + + final Map nullAliasMap = new HashMap<>(); + nullAliasMap.put("alias", null); + + List> expectedResults = Collections.singletonList( + new Result<>( + DateTimes.of("2011-01-12T00:00:00.000Z"), + new TopNResultValue(Collections.singletonList(nullAliasMap)) + ) + ); + assertExpectedResults(expectedResults, query); + } + + @Test + public void testSortOnDoubleAsDouble() + { + TopNQuery query = new TopNQueryBuilder() + .dataSource(QueryRunnerTestHelper.dataSource) + .granularity(QueryRunnerTestHelper.allGran) + .dimension(new DefaultDimensionSpec("index", "index_alias", ValueType.DOUBLE)) + .metric(new DimensionTopNMetricSpec(null, StringComparators.NUMERIC)) + .threshold(4) + .intervals(QueryRunnerTestHelper.fullOnInterval) + .build(); + + List> expectedResults = Collections.singletonList( + new Result<>( + DateTimes.of("2011-01-12T00:00:00.000Z"), + new TopNResultValue( + Arrays.>asList( + ImmutableMap.builder() + .put("index_alias", 59.021022d) + .build(), + ImmutableMap.builder() + .put("index_alias", 59.266595d) + .build(), + ImmutableMap.builder() + .put("index_alias", 67.73117d) + .build(), + ImmutableMap.builder() + .put("index_alias", 68.573162d) + .build() + ) + ) + ) + ); + assertExpectedResults(expectedResults, query); + } + @Test public void testFullOnTopNLongTimeColumnWithExFn() {