diff --git a/common/src/main/java/io/druid/math/expr/Expr.java b/common/src/main/java/io/druid/math/expr/Expr.java index 9688d509a59c..913f40244bc8 100644 --- a/common/src/main/java/io/druid/math/expr/Expr.java +++ b/common/src/main/java/io/druid/math/expr/Expr.java @@ -19,14 +19,18 @@ package io.druid.math.expr; +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import com.google.common.math.LongMath; import com.google.common.primitives.Ints; import io.druid.java.util.common.IAE; import io.druid.java.util.common.ISE; +import io.druid.java.util.common.guava.Comparators; import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.util.List; +import java.util.Objects; /** */ @@ -57,7 +61,8 @@ default Object getLiteralValue() interface ObjectBinding { - Number get(String name); + @Nullable + Object get(String name); } void visit(Visitor visitor); @@ -89,10 +94,10 @@ class LongExpr extends ConstantExpr public LongExpr(Long value) { - this.value = value; + this.value = Preconditions.checkNotNull(value, "value"); } - @Nullable + @Nonnull @Override public Object getLiteralValue() { @@ -119,7 +124,7 @@ class StringExpr extends ConstantExpr public StringExpr(String value) { - this.value = value; + this.value = Strings.emptyToNull(value); } @Nullable @@ -149,10 +154,10 @@ class DoubleExpr extends ConstantExpr public DoubleExpr(Double value) { - this.value = value; + this.value = Preconditions.checkNotNull(value, "value"); } - @Nullable + @Nonnull @Override public Object getLiteralValue() { @@ -357,19 +362,16 @@ public ExprEval eval(ObjectBinding bindings) { ExprEval leftVal = left.eval(bindings); ExprEval rightVal = right.eval(bindings); - if (leftVal.isNull() || rightVal.isNull()) { - return ExprEval.of(null); - } - if (leftVal.type() == ExprType.STRING || rightVal.type() == ExprType.STRING) { + if (leftVal.type() == ExprType.STRING && rightVal.type() == ExprType.STRING) { return evalString(leftVal.asString(), rightVal.asString()); - } - if (leftVal.type() == ExprType.LONG && rightVal.type() == ExprType.LONG) { + } else if (leftVal.type() == ExprType.LONG && rightVal.type() == ExprType.LONG) { return ExprEval.of(evalLong(leftVal.asLong(), rightVal.asLong())); + } else { + return ExprEval.of(evalDouble(leftVal.asDouble(), rightVal.asDouble())); } - return ExprEval.of(evalDouble(leftVal.asDouble(), rightVal.asDouble())); } - protected ExprEval evalString(String left, String right) + protected ExprEval evalString(@Nullable String left, @Nullable String right) { throw new IllegalArgumentException("unsupported type " + ExprType.STRING); } @@ -487,9 +489,9 @@ class BinPlusExpr extends BinaryEvalOpExprBase } @Override - protected ExprEval evalString(String left, String right) + protected ExprEval evalString(@Nullable String left, @Nullable String right) { - return ExprEval.of(left + right); + return ExprEval.of(Strings.nullToEmpty(left) + Strings.nullToEmpty(right)); } @Override @@ -513,9 +515,9 @@ class BinLtExpr extends BinaryEvalOpExprBase } @Override - protected ExprEval evalString(String left, String right) + protected ExprEval evalString(@Nullable String left, @Nullable String right) { - return ExprEval.of(left.compareTo(right) < 0, ExprType.LONG); + return ExprEval.of(Comparators.naturalNullsFirst().compare(left, right) < 0, ExprType.LONG); } @Override @@ -527,7 +529,8 @@ protected final long evalLong(long left, long right) @Override protected final double evalDouble(double left, double right) { - return Evals.asDouble(left < right); + // Use Double.compare for more consistent NaN handling. + return Evals.asDouble(Double.compare(left, right) < 0); } } @@ -539,9 +542,9 @@ class BinLeqExpr extends BinaryEvalOpExprBase } @Override - protected ExprEval evalString(String left, String right) + protected ExprEval evalString(@Nullable String left, @Nullable String right) { - return ExprEval.of(left.compareTo(right) <= 0, ExprType.LONG); + return ExprEval.of(Comparators.naturalNullsFirst().compare(left, right) <= 0, ExprType.LONG); } @Override @@ -553,7 +556,8 @@ protected final long evalLong(long left, long right) @Override protected final double evalDouble(double left, double right) { - return Evals.asDouble(left <= right); + // Use Double.compare for more consistent NaN handling. + return Evals.asDouble(Double.compare(left, right) <= 0); } } @@ -565,9 +569,9 @@ class BinGtExpr extends BinaryEvalOpExprBase } @Override - protected ExprEval evalString(String left, String right) + protected ExprEval evalString(@Nullable String left, @Nullable String right) { - return ExprEval.of(left.compareTo(right) > 0, ExprType.LONG); + return ExprEval.of(Comparators.naturalNullsFirst().compare(left, right) > 0, ExprType.LONG); } @Override @@ -579,7 +583,8 @@ protected final long evalLong(long left, long right) @Override protected final double evalDouble(double left, double right) { - return Evals.asDouble(left > right); + // Use Double.compare for more consistent NaN handling. + return Evals.asDouble(Double.compare(left, right) > 0); } } @@ -591,9 +596,9 @@ class BinGeqExpr extends BinaryEvalOpExprBase } @Override - protected ExprEval evalString(String left, String right) + protected ExprEval evalString(@Nullable String left, @Nullable String right) { - return ExprEval.of(left.compareTo(right) >= 0, ExprType.LONG); + return ExprEval.of(Comparators.naturalNullsFirst().compare(left, right) >= 0, ExprType.LONG); } @Override @@ -605,7 +610,8 @@ protected final long evalLong(long left, long right) @Override protected final double evalDouble(double left, double right) { - return Evals.asDouble(left >= right); + // Use Double.compare for more consistent NaN handling. + return Evals.asDouble(Double.compare(left, right) >= 0); } } @@ -617,9 +623,9 @@ class BinEqExpr extends BinaryEvalOpExprBase } @Override - protected ExprEval evalString(String left, String right) + protected ExprEval evalString(@Nullable String left, @Nullable String right) { - return ExprEval.of(left.equals(right), ExprType.LONG); + return ExprEval.of(Objects.equals(left, right), ExprType.LONG); } @Override @@ -643,9 +649,9 @@ class BinNeqExpr extends BinaryEvalOpExprBase } @Override - protected ExprEval evalString(String left, String right) + protected ExprEval evalString(@Nullable String left, @Nullable String right) { - return ExprEval.of(!left.equals(right), ExprType.LONG); + return ExprEval.of(!Objects.equals(left, right), ExprType.LONG); } @Override diff --git a/common/src/main/java/io/druid/math/expr/ExprEval.java b/common/src/main/java/io/druid/math/expr/ExprEval.java index d8962061f184..630347982a22 100644 --- a/common/src/main/java/io/druid/math/expr/ExprEval.java +++ b/common/src/main/java/io/druid/math/expr/ExprEval.java @@ -19,7 +19,11 @@ package io.druid.math.expr; +import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.primitives.Doubles; +import com.google.common.primitives.Ints; +import io.druid.common.guava.GuavaUtils; import io.druid.java.util.common.IAE; /** @@ -72,9 +76,9 @@ public static ExprEval bestEffortOf(Object val) } if (val instanceof Number) { if (val instanceof Float || val instanceof Double) { - return new DoubleExprEval((Number)val); + return new DoubleExprEval((Number) val); } - return new LongExprEval((Number)val); + return new LongExprEval((Number) val); } return new StringExprEval(val == null ? null : String.valueOf(val)); } @@ -98,11 +102,6 @@ public boolean isNull() return value == null; } - public Number numericValue() - { - return (Number) value; - } - public abstract int asInt(); public abstract long asLong(); @@ -120,7 +119,8 @@ public String asString() public abstract Expr toExpr(); - private static abstract class NumericExprEval extends ExprEval { + private static abstract class NumericExprEval extends ExprEval + { private NumericExprEval(Number value) { @@ -150,7 +150,7 @@ private static class DoubleExprEval extends NumericExprEval { private DoubleExprEval(Number value) { - super(value); + super(Preconditions.checkNotNull(value, "value")); } @Override @@ -182,7 +182,7 @@ public final ExprEval castTo(ExprType castTo) @Override public Expr toExpr() { - return new DoubleExpr(value == null ? null : value.doubleValue()); + return new DoubleExpr(value.doubleValue()); } } @@ -190,7 +190,7 @@ private static class LongExprEval extends NumericExprEval { private LongExprEval(Number value) { - super(value); + super(Preconditions.checkNotNull(value, "value")); } @Override @@ -222,7 +222,7 @@ public final ExprEval castTo(ExprType castTo) @Override public Expr toExpr() { - return new LongExpr(value == null ? null : value.longValue()); + return new LongExpr(value.longValue()); } } @@ -230,7 +230,7 @@ private static class StringExprEval extends ExprEval { private StringExprEval(String value) { - super(value); + super(Strings.emptyToNull(value)); } @Override @@ -239,28 +239,34 @@ public final ExprType type() return ExprType.STRING; } - @Override - public final boolean isNull() - { - return Strings.isNullOrEmpty(value); - } - @Override public final int asInt() { - return Integer.parseInt(value); + if (value == null) { + return 0; + } + + final Integer theInt = Ints.tryParse(value); + return theInt == null ? 0 : theInt; } @Override public final long asLong() { - return Long.parseLong(value); + // GuavaUtils.tryParseLong handles nulls, no need for special null handling here. + final Long theLong = GuavaUtils.tryParseLong(value); + return theLong == null ? 0L : theLong; } @Override public final double asDouble() { - return Double.parseDouble(value); + if (value == null) { + return 0.0; + } + + final Double theDouble = Doubles.tryParse(value); + return theDouble == null ? 0.0 : theDouble; } @Override diff --git a/common/src/main/java/io/druid/math/expr/Parser.java b/common/src/main/java/io/druid/math/expr/Parser.java index a52655bc6cd0..27ea8b64037a 100644 --- a/common/src/main/java/io/druid/math/expr/Parser.java +++ b/common/src/main/java/io/druid/math/expr/Parser.java @@ -148,30 +148,14 @@ public void visit(Expr expr) public static Expr.ObjectBinding withMap(final Map bindings) { - return new Expr.ObjectBinding() - { - @Override - public Number get(String name) - { - Number number = (Number)bindings.get(name); - if (number == null && !bindings.containsKey(name)) { - throw new RuntimeException("No binding found for " + name); - } - return number; - } - }; + return bindings::get; } - public static Expr.ObjectBinding withSuppliers(final Map> bindings) + public static Expr.ObjectBinding withSuppliers(final Map> bindings) { - return new Expr.ObjectBinding() - { - @Override - public Number get(String name) - { - Supplier supplier = bindings.get(name); - return supplier == null ? null : supplier.get(); - } + return (String name) -> { + Supplier supplier = bindings.get(name); + return supplier == null ? null : supplier.get(); }; } } diff --git a/processing/src/main/java/io/druid/query/Queries.java b/processing/src/main/java/io/druid/query/Queries.java index d62050965b42..956293c7a6ea 100644 --- a/processing/src/main/java/io/druid/query/Queries.java +++ b/processing/src/main/java/io/druid/query/Queries.java @@ -21,11 +21,12 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Lists; -import com.google.common.collect.Maps; import com.google.common.collect.Sets; import io.druid.query.aggregation.AggregatorFactory; import io.druid.query.aggregation.PostAggregator; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -34,8 +35,10 @@ */ public class Queries { - public static List decoratePostAggregators(List postAggs, - Map aggFactories) + public static List decoratePostAggregators( + List postAggs, + Map aggFactories + ) { List decorated = Lists.newArrayListWithExpectedSize(postAggs.size()); for (PostAggregator aggregator : postAggs) { @@ -44,34 +47,57 @@ public static List decoratePostAggregators(List return decorated; } + /** + * Returns decorated post-aggregators, based on original un-decorated post-aggregators. In addition, this method + * also verifies that there are no output name collisions, and that all of the post-aggregators' required input + * fields are present. + * + * @param otherOutputNames names of fields that will appear in the same output namespace as aggregators and + * post-aggregators. For most built-in query types, this is either empty, or the list of + * dimension output names. + * @param aggFactories aggregator factories for this query + * @param postAggs post-aggregators for this query + * + * @return decorated post-aggregators + * + * @throws NullPointerException if otherOutputNames or aggFactories is null + * @throws IllegalArgumentException if there are any output name collisions or missing post-aggregator inputs + */ public static List prepareAggregations( + List otherOutputNames, List aggFactories, List postAggs ) { + Preconditions.checkNotNull(otherOutputNames, "otherOutputNames cannot be null"); Preconditions.checkNotNull(aggFactories, "aggregations cannot be null"); - final Map aggsFactoryMap = Maps.newHashMap(); + final Set combinedOutputNames = new HashSet<>(); + combinedOutputNames.addAll(otherOutputNames); + + final Map aggsFactoryMap = new HashMap<>(); for (AggregatorFactory aggFactory : aggFactories) { - Preconditions.checkArgument(!aggsFactoryMap.containsKey(aggFactory.getName()), - "[%s] already defined", aggFactory.getName()); + Preconditions.checkArgument( + combinedOutputNames.add(aggFactory.getName()), + "[%s] already defined", aggFactory.getName() + ); aggsFactoryMap.put(aggFactory.getName(), aggFactory); } if (postAggs != null && !postAggs.isEmpty()) { - final Set combinedAggNames = Sets.newHashSet(aggsFactoryMap.keySet()); - List decorated = Lists.newArrayListWithExpectedSize(postAggs.size()); for (final PostAggregator postAgg : postAggs) { final Set dependencies = postAgg.getDependentFields(); - final Set missing = Sets.difference(dependencies, combinedAggNames); + final Set missing = Sets.difference(dependencies, combinedOutputNames); Preconditions.checkArgument( missing.isEmpty(), "Missing fields [%s] for postAggregator [%s]", missing, postAgg.getName() ); - Preconditions.checkArgument(combinedAggNames.add(postAgg.getName()), - "[%s] already defined", postAgg.getName()); + Preconditions.checkArgument( + combinedOutputNames.add(postAgg.getName()), + "[%s] already defined", postAgg.getName() + ); decorated.add(postAgg.decorate(aggsFactoryMap)); } diff --git a/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java index 46a7befb7bd5..a05f19d4226c 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/ArithmeticPostAggregator.java @@ -271,6 +271,7 @@ static Set getFns() public static enum Ordering implements Comparator { // ensures the following order: numeric > NaN > Infinite + // The name may be referenced via Ordering.valueOf(ordering) in the constructor. numericFirst { @Override public int compare(Double lhs, Double rhs) { diff --git a/processing/src/main/java/io/druid/query/aggregation/post/ExpressionPostAggregator.java b/processing/src/main/java/io/druid/query/aggregation/post/ExpressionPostAggregator.java index 06ccc7a3842a..090363d7a2aa 100644 --- a/processing/src/main/java/io/druid/query/aggregation/post/ExpressionPostAggregator.java +++ b/processing/src/main/java/io/druid/query/aggregation/post/ExpressionPostAggregator.java @@ -24,6 +24,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; +import io.druid.java.util.common.guava.Comparators; import io.druid.math.expr.Expr; import io.druid.math.expr.ExprMacroTable; import io.druid.math.expr.Parser; @@ -36,25 +37,22 @@ import java.util.Objects; import java.util.Set; -/** - */ public class ExpressionPostAggregator implements PostAggregator { - private static final Comparator DEFAULT_COMPARATOR = new Comparator() - { - @Override - public int compare(Number o1, Number o2) - { - if (o1 instanceof Long && o2 instanceof Long) { - return Long.compare(o1.longValue(), o2.longValue()); - } - return Double.compare(o1.doubleValue(), o2.doubleValue()); - } - }; + private static final Comparator DEFAULT_COMPARATOR = Comparator.nullsFirst( + (Comparable o1, Comparable o2) -> { + if (o1 instanceof Long && o2 instanceof Long) { + return Long.compare((long) o1, (long) o2); + } else if (o1 instanceof Number && o2 instanceof Number) { + return Double.compare(((Number) o1).doubleValue(), ((Number) o2).doubleValue()); + } else { + return o1.compareTo(o2); + } + }); private final String name; private final String expression; - private final Comparator comparator; + private final Comparator comparator; private final String ordering; private final ExprMacroTable macroTable; @@ -150,31 +148,29 @@ public byte[] getCacheKey() .build(); } - public static enum Ordering implements Comparator + public enum Ordering implements Comparator { // ensures the following order: numeric > NaN > Infinite + // The name may be referenced via Ordering.valueOf(ordering) in the constructor. numericFirst { @Override - public int compare(Number lhs, Number rhs) + public int compare(Comparable lhs, Comparable rhs) { if (lhs instanceof Long && rhs instanceof Long) { - return Long.compare(lhs.longValue(), rhs.longValue()); + return Long.compare(((Number) lhs).longValue(), ((Number) rhs).longValue()); + } else if (lhs instanceof Number && rhs instanceof Number) { + double d1 = ((Number) lhs).doubleValue(); + double d2 = ((Number) rhs).doubleValue(); + if (Double.isFinite(d1) && !Double.isFinite(d2)) { + return 1; + } + if (!Double.isFinite(d1) && Double.isFinite(d2)) { + return -1; + } + return Double.compare(d1, d2); + } else { + return Comparators.naturalNullsFirst().compare(lhs, rhs); } - double d1 = lhs.doubleValue(); - double d2 = rhs.doubleValue(); - if (isFinite(d1) && !isFinite(d2)) { - return 1; - } - if (!isFinite(d1) && isFinite(d2)) { - return -1; - } - return Double.compare(d1, d2); - } - - // Double.isFinite only exist in JDK8 - private boolean isFinite(double value) - { - return !Double.isInfinite(value) && !Double.isNaN(value); } } } 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 30efe3ac725d..dd1154cea36e 100644 --- a/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java +++ b/processing/src/main/java/io/druid/query/groupby/GroupByQuery.java @@ -75,6 +75,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; /** */ @@ -182,6 +183,7 @@ private GroupByQuery( } this.aggregatorSpecs = aggregatorSpecs == null ? ImmutableList.of() : aggregatorSpecs; this.postAggregatorSpecs = Queries.prepareAggregations( + this.dimensions.stream().map(DimensionSpec::getOutputName).collect(Collectors.toList()), this.aggregatorSpecs, postAggregatorSpecs == null ? ImmutableList.of() : postAggregatorSpecs ); diff --git a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQuery.java b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQuery.java index 82e6c74e52b5..2cfa00b6235e 100644 --- a/processing/src/main/java/io/druid/query/timeseries/TimeseriesQuery.java +++ b/processing/src/main/java/io/druid/query/timeseries/TimeseriesQuery.java @@ -71,6 +71,7 @@ public TimeseriesQuery( this.granularity = granularity; this.aggregatorSpecs = aggregatorSpecs == null ? ImmutableList.of() : aggregatorSpecs; this.postAggregatorSpecs = Queries.prepareAggregations( + ImmutableList.of(), this.aggregatorSpecs, postAggregatorSpecs == null ? ImmutableList.of() : postAggregatorSpecs ); diff --git a/processing/src/main/java/io/druid/query/topn/NumericTopNMetricSpec.java b/processing/src/main/java/io/druid/query/topn/NumericTopNMetricSpec.java index 1f165fc4e348..5950900cec03 100644 --- a/processing/src/main/java/io/druid/query/topn/NumericTopNMetricSpec.java +++ b/processing/src/main/java/io/druid/query/topn/NumericTopNMetricSpec.java @@ -55,7 +55,10 @@ public void verifyPreconditions(List aggregatorSpecs, List 0, "Must have at least one AggregatorFactory"); + Preconditions.checkArgument( + aggregatorSpecs.size() > 0 || postAggregatorSpecs.size() > 0, + "Must have at least one AggregatorFactory or PostAggregator" + ); final AggregatorFactory aggregator = Iterables.tryFind( aggregatorSpecs, diff --git a/processing/src/main/java/io/druid/query/topn/TopNQuery.java b/processing/src/main/java/io/druid/query/topn/TopNQuery.java index 4bdbf70b7a20..c7bfa1efa9fc 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQuery.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQuery.java @@ -81,6 +81,7 @@ public TopNQuery( this.granularity = granularity; this.aggregatorSpecs = aggregatorSpecs == null ? ImmutableList.of() : aggregatorSpecs; this.postAggregatorSpecs = Queries.prepareAggregations( + ImmutableList.of(dimensionSpec.getOutputName()), this.aggregatorSpecs, postAggregatorSpecs == null ? ImmutableList.of() 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 a9640ec82be9..a81873865f8d 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryEngine.java @@ -143,8 +143,9 @@ private TopNMapFn getMapFn( topNAlgorithm = new TimeExtractionTopNAlgorithm(capabilities, query); } else if (selector.isHasExtractionFn()) { topNAlgorithm = new DimExtractionTopNAlgorithm(capabilities, query); - } else if (columnCapabilities != null && columnCapabilities.getType() != ValueType.STRING) { - // force non-Strings to use DimExtraction for now, do a typed PooledTopN later + } else if (columnCapabilities != null && !(columnCapabilities.getType() == ValueType.STRING + && columnCapabilities.isDictionaryEncoded())) { + // Use DimExtraction for non-Strings and for non-dictionary-encoded Strings. topNAlgorithm = new DimExtractionTopNAlgorithm(capabilities, query); } else if (selector.isAggregateAllMetrics()) { topNAlgorithm = new PooledTopNAlgorithm(capabilities, query, bufferPool); 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 12e489a85a65..5b76ba2b4c12 100644 --- a/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java +++ b/processing/src/main/java/io/druid/query/topn/TopNQueryQueryToolChest.java @@ -249,11 +249,16 @@ public Map apply(DimensionAndMetricValueExtractor input) + 1 ); + // Put non-finalized aggregators before post-aggregators. for (int i = 0; i < aggFactoryNames.length; ++i) { final String name = aggFactoryNames[i]; values.put(name, input.getMetric(name)); } + // Put dimension, post-aggregators might depend on it. + values.put(dimension, input.getDimensionValue(dimension)); + + // Put post-aggregators. for (PostAggregator postAgg : postAggregators) { Object calculatedPostAgg = input.getMetric(postAgg.getName()); if (calculatedPostAgg != null) { @@ -262,13 +267,13 @@ public Map apply(DimensionAndMetricValueExtractor input) values.put(postAgg.getName(), postAgg.compute(values)); } } + + // Put finalized aggregators now that post-aggregators are done. for (int i = 0; i < aggFactoryNames.length; ++i) { final String name = aggFactoryNames[i]; values.put(name, fn.manipulate(aggregatorFactories[i], input.getMetric(name))); } - values.put(dimension, input.getDimensionValue(dimension)); - return values; } } diff --git a/processing/src/main/java/io/druid/segment/virtual/ExpressionObjectSelector.java b/processing/src/main/java/io/druid/segment/virtual/ExpressionObjectSelector.java index 239ff8a435ba..4ad9ca08cf93 100644 --- a/processing/src/main/java/io/druid/segment/virtual/ExpressionObjectSelector.java +++ b/processing/src/main/java/io/druid/segment/virtual/ExpressionObjectSelector.java @@ -23,22 +23,22 @@ import com.google.common.base.Preconditions; import com.google.common.base.Supplier; import com.google.common.collect.Maps; -import com.google.common.primitives.Doubles; -import io.druid.common.guava.GuavaUtils; import io.druid.math.expr.Expr; +import io.druid.math.expr.ExprEval; import io.druid.math.expr.Parser; +import io.druid.query.dimension.DefaultDimensionSpec; import io.druid.segment.ColumnSelectorFactory; -import io.druid.segment.FloatColumnSelector; -import io.druid.segment.LongColumnSelector; +import io.druid.segment.DimensionSelector; import io.druid.segment.ObjectColumnSelector; import io.druid.segment.column.ColumnCapabilities; import io.druid.segment.column.ValueType; +import io.druid.segment.data.IndexedInts; import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.util.Map; -public class ExpressionObjectSelector implements ObjectColumnSelector +public class ExpressionObjectSelector implements ObjectColumnSelector { private final Expr expression; private final Expr.ObjectBinding bindings; @@ -49,28 +49,32 @@ private ExpressionObjectSelector(Expr.ObjectBinding bindings, Expr expression) this.expression = Preconditions.checkNotNull(expression, "expression"); } - public static ExpressionObjectSelector from(ColumnSelectorFactory columnSelectorFactory, Expr expression) + static ExpressionObjectSelector from(ColumnSelectorFactory columnSelectorFactory, Expr expression) { return new ExpressionObjectSelector(createBindings(columnSelectorFactory, expression), expression); } private static Expr.ObjectBinding createBindings(ColumnSelectorFactory columnSelectorFactory, Expr expression) { - final Map> suppliers = Maps.newHashMap(); + final Map> suppliers = Maps.newHashMap(); for (String columnName : Parser.findRequiredBindings(expression)) { final ColumnCapabilities columnCapabilities = columnSelectorFactory.getColumnCapabilities(columnName); final ValueType nativeType = columnCapabilities != null ? columnCapabilities.getType() : null; - final Supplier supplier; + final Supplier supplier; if (nativeType == ValueType.FLOAT) { - supplier = supplierFromFloatSelector(columnSelectorFactory.makeFloatColumnSelector(columnName)); + supplier = columnSelectorFactory.makeFloatColumnSelector(columnName)::get; } else if (nativeType == ValueType.LONG) { - supplier = supplierFromLongSelector(columnSelectorFactory.makeLongColumnSelector(columnName)); + supplier = columnSelectorFactory.makeLongColumnSelector(columnName)::get; + } else if (nativeType == ValueType.STRING) { + supplier = supplierFromDimensionSelector( + columnSelectorFactory.makeDimensionSelector(new DefaultDimensionSpec(columnName, columnName)) + ); } else if (nativeType == null) { // Unknown ValueType. Try making an Object selector and see if that gives us anything useful. supplier = supplierFromObjectSelector(columnSelectorFactory.makeObjectColumnSelector(columnName)); } else { - // Unhandleable ValueType (possibly STRING or COMPLEX). + // Unhandleable ValueType (COMPLEX). supplier = null; } @@ -84,91 +88,61 @@ private static Expr.ObjectBinding createBindings(ColumnSelectorFactory columnSel @VisibleForTesting @Nonnull - static Supplier supplierFromFloatSelector(final FloatColumnSelector selector) + static Supplier supplierFromDimensionSelector(final DimensionSelector selector) { Preconditions.checkNotNull(selector, "selector"); - return new Supplier() - { - @Override - public Number get() - { - return selector.get(); - } - }; - } - - @VisibleForTesting - @Nonnull - static Supplier supplierFromLongSelector(final LongColumnSelector selector) - { - Preconditions.checkNotNull(selector, "selector"); - return new Supplier() - { - @Override - public Number get() - { - return selector.get(); + return () -> { + final IndexedInts row = selector.getRow(); + if (row.size() == 0) { + // Treat empty multi-value rows as nulls. + return null; + } else if (row.size() == 1) { + return selector.lookupName(row.get(0)); + } else { + // Can't handle multi-value rows in expressions. + // Treat them as nulls until we think of something better to do. + return null; } }; } @VisibleForTesting @Nullable - static Supplier supplierFromObjectSelector(final ObjectColumnSelector selector) + static Supplier supplierFromObjectSelector(final ObjectColumnSelector selector) { - final Class clazz = selector == null ? null : selector.classOfObject(); - if (selector != null && (clazz.isAssignableFrom(Number.class) - || clazz.isAssignableFrom(String.class) - || Number.class.isAssignableFrom(clazz))) { - // There may be numbers here. - return new Supplier() - { - @Override - public Number get() - { - return tryParse(selector.get()); - } - }; - } else { - // We know there are no numbers here. Use a null supplier. + if (selector == null) { return null; } - } - @Nullable - private static Number tryParse(final Object value) - { - if (value == null) { + final Class clazz = selector.classOfObject(); + if (Number.class.isAssignableFrom(clazz) || String.class.isAssignableFrom(clazz)) { + // Number, String supported as-is. + return selector::get; + } else if (clazz.isAssignableFrom(Number.class) || clazz.isAssignableFrom(String.class)) { + // Might be Numbers and Strings. Use a selector that double-checks. + return () -> { + final Object val = selector.get(); + if (val instanceof Number || val instanceof String) { + return val; + } else { + return null; + } + }; + } else { + // No numbers or strings. return null; } - - if (value instanceof Number) { - return (Number) value; - } - - final String stringValue = String.valueOf(value); - final Long longValue = GuavaUtils.tryParseLong(stringValue); - if (longValue != null) { - return longValue; - } - - final Double doubleValue = Doubles.tryParse(stringValue); - if (doubleValue != null) { - return doubleValue; - } - - return null; } @Override - public Class classOfObject() + public Class classOfObject() { - return Number.class; + return ExprEval.class; } @Override - public Number get() + public ExprEval get() { - return expression.eval(bindings).numericValue(); + return expression.eval(bindings); } } diff --git a/processing/src/main/java/io/druid/segment/virtual/ExpressionSelectors.java b/processing/src/main/java/io/druid/segment/virtual/ExpressionSelectors.java index 757f7deddef7..4defb7f422d1 100644 --- a/processing/src/main/java/io/druid/segment/virtual/ExpressionSelectors.java +++ b/processing/src/main/java/io/druid/segment/virtual/ExpressionSelectors.java @@ -19,7 +19,9 @@ package io.druid.segment.virtual; +import com.google.common.base.Strings; import io.druid.math.expr.Expr; +import io.druid.math.expr.ExprEval; import io.druid.query.extraction.ExtractionFn; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import io.druid.segment.ColumnSelectorFactory; @@ -54,8 +56,8 @@ class ExpressionLongColumnSelector implements LongColumnSelector @Override public long get() { - final Number number = baseSelector.get(); - return number != null ? number.longValue() : nullValue; + final ExprEval exprEval = baseSelector.get(); + return exprEval.isNull() ? nullValue : exprEval.asLong(); } @Override @@ -79,8 +81,8 @@ class ExpressionFloatColumnSelector implements FloatColumnSelector @Override public float get() { - final Number number = baseSelector.get(); - return number != null ? number.floatValue() : nullValue; + final ExprEval exprEval = baseSelector.get(); + return exprEval.isNull() ? nullValue : (float) exprEval.asDouble(); } @Override @@ -106,8 +108,7 @@ class DefaultExpressionDimensionSelector extends BaseSingleValueDimensionSelecto @Override protected String getValue() { - final Number number = baseSelector.get(); - return number == null ? null : String.valueOf(number); + return Strings.emptyToNull(baseSelector.get().asString()); } @Override @@ -123,7 +124,7 @@ class ExtractionExpressionDimensionSelector extends BaseSingleValueDimensionSele @Override protected String getValue() { - return extractionFn.apply(baseSelector.get()); + return extractionFn.apply(Strings.emptyToNull(baseSelector.get().asString())); } @Override diff --git a/processing/src/main/java/io/druid/segment/virtual/ExpressionVirtualColumn.java b/processing/src/main/java/io/druid/segment/virtual/ExpressionVirtualColumn.java index cfcf57e5b3df..cf9ce1e5f33b 100644 --- a/processing/src/main/java/io/druid/segment/virtual/ExpressionVirtualColumn.java +++ b/processing/src/main/java/io/druid/segment/virtual/ExpressionVirtualColumn.java @@ -43,21 +43,22 @@ public class ExpressionVirtualColumn implements VirtualColumn { - private static final ColumnCapabilities CAPABILITIES = new ColumnCapabilitiesImpl().setType(ValueType.FLOAT); - private final String name; private final String expression; + private final ValueType outputType; private final Expr parsedExpression; @JsonCreator public ExpressionVirtualColumn( @JsonProperty("name") String name, @JsonProperty("expression") String expression, + @JsonProperty("outputType") ValueType outputType, @JacksonInject ExprMacroTable macroTable ) { this.name = Preconditions.checkNotNull(name, "name"); this.expression = Preconditions.checkNotNull(expression, "expression"); + this.outputType = outputType != null ? outputType : ValueType.FLOAT; this.parsedExpression = Parser.parse(expression, macroTable); } @@ -74,6 +75,12 @@ public String getExpression() return expression; } + @JsonProperty + public ValueType getOutputType() + { + return outputType; + } + @Override public ObjectColumnSelector makeObjectColumnSelector( final String columnName, @@ -95,7 +102,7 @@ public Class classOfObject() @Override public Object get() { - return baseSelector.get(); + return baseSelector.get().value(); } }; } @@ -136,7 +143,7 @@ public LongColumnSelector makeLongColumnSelector( @Override public ColumnCapabilities capabilities(String columnName) { - return CAPABILITIES; + return new ColumnCapabilitiesImpl().setType(outputType); } @Override @@ -157,6 +164,7 @@ public byte[] getCacheKey() return new CacheKeyBuilder(VirtualColumnCacheHelper.CACHE_TYPE_ID_EXPRESSION) .appendString(name) .appendString(expression) + .appendString(outputType.toString()) .build(); } @@ -171,13 +179,14 @@ public boolean equals(final Object o) } final ExpressionVirtualColumn that = (ExpressionVirtualColumn) o; return Objects.equals(name, that.name) && - Objects.equals(expression, that.expression); + Objects.equals(expression, that.expression) && + outputType == that.outputType; } @Override public int hashCode() { - return Objects.hash(name, expression); + return Objects.hash(name, expression, outputType); } @Override @@ -186,7 +195,7 @@ public String toString() return "ExpressionVirtualColumn{" + "name='" + name + '\'' + ", expression='" + expression + '\'' + - ", parsedExpression=" + parsedExpression + + ", outputType=" + outputType + '}'; } } diff --git a/processing/src/test/java/io/druid/query/QueriesTest.java b/processing/src/test/java/io/druid/query/QueriesTest.java index ba4ddb516091..616fed76b913 100644 --- a/processing/src/test/java/io/druid/query/QueriesTest.java +++ b/processing/src/test/java/io/druid/query/QueriesTest.java @@ -19,6 +19,7 @@ package io.druid.query; +import com.google.common.collect.ImmutableList; import io.druid.query.aggregation.AggregatorFactory; import io.druid.query.aggregation.CountAggregatorFactory; import io.druid.query.aggregation.DoubleSumAggregatorFactory; @@ -59,7 +60,7 @@ public void testVerifyAggregations() throws Exception boolean exceptionOccured = false; try { - Queries.prepareAggregations(aggFactories, postAggs); + Queries.prepareAggregations(ImmutableList.of(), aggFactories, postAggs); } catch (IllegalArgumentException e) { exceptionOccured = true; @@ -91,7 +92,7 @@ public void testVerifyAggregationsMissingVal() throws Exception boolean exceptionOccured = false; try { - Queries.prepareAggregations(aggFactories, postAggs); + Queries.prepareAggregations(ImmutableList.of(), aggFactories, postAggs); } catch (IllegalArgumentException e) { exceptionOccured = true; @@ -145,7 +146,7 @@ public void testVerifyAggregationsMultiLevel() throws Exception boolean exceptionOccured = false; try { - Queries.prepareAggregations(aggFactories, postAggs); + Queries.prepareAggregations(ImmutableList.of(), aggFactories, postAggs); } catch (IllegalArgumentException e) { exceptionOccured = true; @@ -199,7 +200,7 @@ public void testVerifyAggregationsMultiLevelMissingVal() throws Exception boolean exceptionOccured = false; try { - Queries.prepareAggregations(aggFactories, postAggs); + Queries.prepareAggregations(ImmutableList.of(), aggFactories, postAggs); } catch (IllegalArgumentException e) { exceptionOccured = true; diff --git a/processing/src/test/java/io/druid/query/SchemaEvolutionTest.java b/processing/src/test/java/io/druid/query/SchemaEvolutionTest.java index b6f9c32e25ca..08fa28aa4d06 100644 --- a/processing/src/test/java/io/druid/query/SchemaEvolutionTest.java +++ b/processing/src/test/java/io/druid/query/SchemaEvolutionTest.java @@ -269,8 +269,9 @@ public void testNumericEvolutionTimeseriesAggregation() .build(); // Only string(1) + // Note: Expressions implicitly cast strings to numbers, leading to the a/b vs c/d difference. Assert.assertEquals( - timeseriesResult(ImmutableMap.of("a", 0L, "b", 0.0, "c", 0L, "d", 0.0)), + timeseriesResult(ImmutableMap.of("a", 0L, "b", 0.0, "c", 31L, "d", THIRTY_ONE_POINT_ONE)), runQuery(query, factory, ImmutableList.of(index1)) ); @@ -293,12 +294,13 @@ public void testNumericEvolutionTimeseriesAggregation() ); // string(1) + long(2) + float(3) + nonexistent(4) + // Note: Expressions implicitly cast strings to numbers, leading to the a/b vs c/d difference. Assert.assertEquals( timeseriesResult(ImmutableMap.of( "a", 31L * 2, "b", THIRTY_ONE_POINT_ONE + 31, - "c", 31L * 2, - "d", THIRTY_ONE_POINT_ONE + 31 + "c", 31L * 3, + "d", THIRTY_ONE_POINT_ONE * 2 + 31 )), runQuery(query, factory, ImmutableList.of(index1, index2, index3, index4)) ); diff --git a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java index 31b0fbc0adf8..867f62b657b0 100644 --- a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java @@ -446,6 +446,134 @@ public void testGroupBy() TestHelper.assertExpectedObjects(expectedResults, results, ""); } + @Test + public void testGroupByWithStringPostAggregator() + { + GroupByQuery query = GroupByQuery + .builder() + .setDataSource(QueryRunnerTestHelper.dataSource) + .setQuerySegmentSpec(QueryRunnerTestHelper.firstToThird) + .setDimensions(Lists.newArrayList(new DefaultDimensionSpec("quality", "alias"))) + .setAggregatorSpecs( + Arrays.asList( + QueryRunnerTestHelper.rowsCount, + new LongSumAggregatorFactory("idx", "index") + ) + ) + .setPostAggregatorSpecs( + ImmutableList.of( + new ExpressionPostAggregator("post", "alias + 'x'", null, TestExprMacroTable.INSTANCE) + ) + ) + .setGranularity(QueryRunnerTestHelper.dayGran) + .setLimitSpec( + new DefaultLimitSpec( + ImmutableList.of( + new OrderByColumnSpec("post", OrderByColumnSpec.Direction.DESCENDING) + ), + Integer.MAX_VALUE + ) + ) + .build(); + + List expectedResults = Arrays.asList( + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "travel", "post", "travelx", "rows", 1L, "idx", 119L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "technology", "post", "technologyx", "rows", 1L, "idx", 78L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "premium", "post", "premiumx", "rows", 3L, "idx", 2900L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "news", "post", "newsx", "rows", 1L, "idx", 121L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "mezzanine", "post", "mezzaninex", "rows", 3L, "idx", 2870L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "health", "post", "healthx", "rows", 1L, "idx", 120L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "entertainment", "post", "entertainmentx", "rows", 1L, "idx", 158L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "business", "post", "businessx", "rows", 1L, "idx", 118L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "automotive", "post", "automotivex", "rows", 1L, "idx", 135L), + + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "travel", "post", "travelx", "rows", 1L, "idx", 126L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "technology", "post", "technologyx", "rows", 1L, "idx", 97L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "premium", "post", "premiumx", "rows", 3L, "idx", 2505L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "news", "post", "newsx", "rows", 1L, "idx", 114L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "mezzanine", "post", "mezzaninex", "rows", 3L, "idx", 2447L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "health", "post", "healthx", "rows", 1L, "idx", 113L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "entertainment", "post", "entertainmentx", "rows", 1L, "idx", 166L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "business", "post", "businessx", "rows", 1L, "idx", 112L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "automotive", "post", "automotivex", "rows", 1L, "idx", 147L) + ); + + Iterable results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query); + TestHelper.assertExpectedObjects(expectedResults, results, ""); + } + + @Test + public void testGroupByWithStringVirtualColumn() + { + GroupByQuery query = GroupByQuery + .builder() + .setDataSource(QueryRunnerTestHelper.dataSource) + .setQuerySegmentSpec(QueryRunnerTestHelper.firstToThird) + .setVirtualColumns( + new ExpressionVirtualColumn( + "vc", + "quality + 'x'", + ValueType.STRING, + TestExprMacroTable.INSTANCE + ) + ) + .setDimensions(Lists.newArrayList(new DefaultDimensionSpec("vc", "alias"))) + .setAggregatorSpecs( + Arrays.asList( + QueryRunnerTestHelper.rowsCount, + new LongSumAggregatorFactory("idx", "index") + ) + ) + .setGranularity(QueryRunnerTestHelper.dayGran) + .build(); + + if (config.getDefaultStrategy().equals(GroupByStrategySelector.STRATEGY_V1)) { + expectedException.expect(UnsupportedOperationException.class); + expectedException.expectMessage("GroupBy v1 does not support dimension selectors with unknown cardinality."); + } + + List expectedResults = Arrays.asList( + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "automotivex", "rows", 1L, "idx", 135L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "businessx", "rows", 1L, "idx", 118L), + GroupByQueryRunnerTestHelper.createExpectedRow( + "2011-04-01", + "alias", + "entertainmentx", + "rows", + 1L, + "idx", + 158L + ), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "healthx", "rows", 1L, "idx", 120L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "mezzaninex", "rows", 3L, "idx", 2870L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "newsx", "rows", 1L, "idx", 121L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "premiumx", "rows", 3L, "idx", 2900L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "technologyx", "rows", 1L, "idx", 78L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "travelx", "rows", 1L, "idx", 119L), + + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "automotivex", "rows", 1L, "idx", 147L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "businessx", "rows", 1L, "idx", 112L), + GroupByQueryRunnerTestHelper.createExpectedRow( + "2011-04-02", + "alias", + "entertainmentx", + "rows", + 1L, + "idx", + 166L + ), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "healthx", "rows", 1L, "idx", 113L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "mezzaninex", "rows", 3L, "idx", 2447L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "newsx", "rows", 1L, "idx", 114L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "premiumx", "rows", 3L, "idx", 2505L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "technologyx", "rows", 1L, "idx", 97L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "travelx", "rows", 1L, "idx", 126L) + ); + + Iterable results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query); + TestHelper.assertExpectedObjects(expectedResults, results, ""); + } + @Test public void testGroupByWithDurationGranularity() { @@ -493,7 +621,7 @@ public void testGroupByWithDurationGranularity() public void testGroupByWithOutputNameCollisions() { expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Duplicate output name[alias]"); + expectedException.expectMessage("[alias] already defined"); GroupByQuery query = GroupByQuery .builder() @@ -2508,6 +2636,7 @@ public void testMergeResultsAcrossMultipleDaysWithLimitAndOrderByUsingMathExpres new ExpressionVirtualColumn( "expr", "index * 2 + indexMin / 10", + ValueType.FLOAT, TestExprMacroTable.INSTANCE ) ) @@ -2747,7 +2876,7 @@ public void testGroupByOrderLimit() throws Exception // Now try it with an expression virtual column. builder.setLimit(Integer.MAX_VALUE) .setVirtualColumns( - new ExpressionVirtualColumn("expr", "index / 2 + indexMin", TestExprMacroTable.INSTANCE) + new ExpressionVirtualColumn("expr", "index / 2 + indexMin", ValueType.FLOAT, TestExprMacroTable.INSTANCE) ) .setAggregatorSpecs( Arrays.asList( @@ -4337,7 +4466,7 @@ public void testDifferentGroupingSubquery() subquery = new GroupByQuery.Builder(subquery) .setVirtualColumns( - new ExpressionVirtualColumn("expr", "-index + 100", TestExprMacroTable.INSTANCE) + new ExpressionVirtualColumn("expr", "-index + 100", ValueType.FLOAT, TestExprMacroTable.INSTANCE) ) .setAggregatorSpecs( Arrays.asList( @@ -5439,7 +5568,7 @@ public void testSubqueryWithOuterVirtualColumns() .builder() .setDataSource(subquery) .setQuerySegmentSpec(QueryRunnerTestHelper.firstToThird) - .setVirtualColumns(new ExpressionVirtualColumn("expr", "1", TestExprMacroTable.INSTANCE)) + .setVirtualColumns(new ExpressionVirtualColumn("expr", "1", ValueType.FLOAT, TestExprMacroTable.INSTANCE)) .setDimensions(Lists.newArrayList()) .setAggregatorSpecs(ImmutableList.of(new LongSumAggregatorFactory("count", "expr"))) .setGranularity(QueryRunnerTestHelper.allGran) @@ -8612,7 +8741,7 @@ public Sequence run(Query query, Map responseContext) GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "travel", "rows", 2L, "idx", 243L) ); - Iterable results = Sequences.toList(mergedRunner.run(allGranQuery, context), Lists.newArrayList()); + Iterable results = Sequences.toList(mergedRunner.run(allGranQuery, context), Lists.newArrayList()); TestHelper.assertExpectedObjects(allGranExpectedResults, results, "merged"); } @@ -8706,7 +8835,7 @@ public Sequence run(Query query, Map responseContext) GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "premium", "market", "spot", "rows", 2L, "idx", 257L) ); - Iterable results = Sequences.toList(mergedRunner.run(allGranQuery, context), Lists.newArrayList()); + Iterable results = Sequences.toList(mergedRunner.run(allGranQuery, context), Lists.newArrayList()); TestHelper.assertExpectedObjects(allGranExpectedResults, results, "merged"); } @@ -8804,7 +8933,7 @@ public Sequence run(Query query, Map responseContext) GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "premium", "market", "spot", "rows", 2L, "idx", 257L) ); - Iterable results = Sequences.toList(mergedRunner.run(allGranQuery, context), Lists.newArrayList()); + Iterable results = Sequences.toList(mergedRunner.run(allGranQuery, context), Lists.newArrayList()); TestHelper.assertExpectedObjects(allGranExpectedResults, results, "merged"); } diff --git a/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java b/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java index ef04d0f04477..f84a427d4adc 100644 --- a/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java @@ -510,7 +510,7 @@ public void testFullOnSelectWithFilterOnVirtualColumn() .metrics(Lists.newArrayList(QueryRunnerTestHelper.indexMetric)) .pagingSpec(new PagingSpec(null, 10, true)) .virtualColumns( - new ExpressionVirtualColumn("expr", "index / 10.0", TestExprMacroTable.INSTANCE) + new ExpressionVirtualColumn("expr", "index / 10.0", ValueType.FLOAT, TestExprMacroTable.INSTANCE) ) .build(); diff --git a/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java b/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java index f6a43f4bc074..a1d2cda6bd66 100644 --- a/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java @@ -55,6 +55,7 @@ import io.druid.query.ordering.StringComparators; import io.druid.query.spec.MultipleIntervalSegmentSpec; import io.druid.segment.TestHelper; +import io.druid.segment.column.ValueType; import io.druid.segment.virtual.ExpressionVirtualColumn; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; @@ -421,6 +422,7 @@ public void testTimeseriesWithVirtualColumn() new ExpressionVirtualColumn( "expr", "index", + ValueType.FLOAT, TestExprMacroTable.INSTANCE ) ) 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 385ef1f5c6a0..bc3f4f3243eb 100644 --- a/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/topn/TopNQueryRunnerTest.java @@ -62,10 +62,12 @@ import io.druid.query.aggregation.hyperloglog.HyperUniqueFinalizingPostAggregator; import io.druid.query.aggregation.hyperloglog.HyperUniquesAggregatorFactory; import io.druid.query.aggregation.last.LongLastAggregatorFactory; +import io.druid.query.aggregation.post.ExpressionPostAggregator; import io.druid.query.dimension.DefaultDimensionSpec; import io.druid.query.dimension.DimensionSpec; import io.druid.query.dimension.ExtractionDimensionSpec; import io.druid.query.dimension.ListFilteredDimensionSpec; +import io.druid.query.expression.TestExprMacroTable; import io.druid.query.extraction.DimExtractionFn; import io.druid.query.extraction.ExtractionFn; import io.druid.query.extraction.JavaScriptExtractionFn; @@ -416,6 +418,78 @@ public void testFullOnTopNOverPostAggs() assertExpectedResults(expectedResults, query); } + @Test + public void testFullOnTopNOverPostAggsOnDimension() + { + TopNQuery query = new TopNQueryBuilder() + .dataSource(QueryRunnerTestHelper.dataSource) + .granularity(QueryRunnerTestHelper.allGran) + .dimension(QueryRunnerTestHelper.marketDimension) + .metric("dimPostAgg") + .threshold(4) + .intervals(QueryRunnerTestHelper.fullOnInterval) + .aggregators( + Lists.newArrayList( + Iterables.concat( + QueryRunnerTestHelper.commonAggregators, + Lists.newArrayList( + new DoubleMaxAggregatorFactory("maxIndex", "index"), + new DoubleMinAggregatorFactory("minIndex", "index") + ) + ) + ) + ) + .postAggregators( + ImmutableList.of( + new ExpressionPostAggregator( + "dimPostAgg", + "market + 'x'", + null, + TestExprMacroTable.INSTANCE + ) + ) + ) + .build(); + + List> expectedResults = Arrays.asList( + new Result( + new DateTime("2011-01-12T00:00:00.000Z"), + new TopNResultValue( + Arrays.>asList( + ImmutableMap.builder() + .put(QueryRunnerTestHelper.marketDimension, "upfront") + .put("dimPostAgg", "upfrontx") + .put("rows", 186L) + .put("index", 192046.1060180664D) + .put("uniques", QueryRunnerTestHelper.UNIQUES_2) + .put("maxIndex", 1870.06103515625D) + .put("minIndex", 545.9906005859375D) + .build(), + ImmutableMap.builder() + .put(QueryRunnerTestHelper.marketDimension, "total_market") + .put("dimPostAgg", "total_marketx") + .put("rows", 186L) + .put("index", 215679.82879638672D) + .put("uniques", QueryRunnerTestHelper.UNIQUES_2) + .put("maxIndex", 1743.9217529296875D) + .put("minIndex", 792.3260498046875D) + .build(), + ImmutableMap.builder() + .put(QueryRunnerTestHelper.marketDimension, "spot") + .put("dimPostAgg", "spotx") + .put("rows", 837L) + .put("index", 95606.57232284546D) + .put("uniques", QueryRunnerTestHelper.UNIQUES_9) + .put("maxIndex", 277.2735290527344D) + .put("minIndex", 59.02102279663086D) + .build() + ) + ) + ) + ); + assertExpectedResults(expectedResults, query); + } + @Test public void testFullOnTopNOverUniques() { @@ -4211,7 +4285,7 @@ public void testFullOnTopNLongVirtualColumn() ) ) .postAggregators(Arrays.asList(QueryRunnerTestHelper.addRowsIndexConstant)) - .virtualColumns(new ExpressionVirtualColumn("ql_expr", "qualityLong", ExprMacroTable.nil())) + .virtualColumns(new ExpressionVirtualColumn("ql_expr", "qualityLong", ValueType.LONG, ExprMacroTable.nil())) .build(); List> expectedResults = Arrays.asList( @@ -4262,6 +4336,61 @@ public void testFullOnTopNLongVirtualColumn() assertExpectedResults(expectedResults, query); } + @Test + public void testTopNStringVirtualColumn() + { + TopNQuery query = new TopNQueryBuilder() + .dataSource(QueryRunnerTestHelper.dataSource) + .granularity(QueryRunnerTestHelper.allGran) + .virtualColumns( + new ExpressionVirtualColumn( + "vc", + "market + ' ' + market", + ValueType.STRING, + TestExprMacroTable.INSTANCE + ) + ) + .dimension("vc") + .metric("rows") + .threshold(4) + .intervals(QueryRunnerTestHelper.firstToThird) + .aggregators(QueryRunnerTestHelper.commonAggregators) + .postAggregators(Arrays.asList(QueryRunnerTestHelper.addRowsIndexConstant)) + .build(); + + List> expectedResults = Arrays.asList( + new Result<>( + new DateTime("2011-04-01T00:00:00.000Z"), + new TopNResultValue( + Arrays.>asList( + ImmutableMap.of( + "vc", "spot spot", + "rows", 18L, + "index", 2231.8768157958984D, + "addRowsIndexConstant", 2250.8768157958984D, + "uniques", QueryRunnerTestHelper.UNIQUES_9 + ), + ImmutableMap.of( + "vc", "total_market total_market", + "rows", 4L, + "index", 5351.814697265625D, + "addRowsIndexConstant", 5356.814697265625D, + "uniques", QueryRunnerTestHelper.UNIQUES_2 + ), + ImmutableMap.of( + "vc", "upfront upfront", + "rows", 4L, + "index", 4875.669677734375D, + "addRowsIndexConstant", 4880.669677734375D, + "uniques", QueryRunnerTestHelper.UNIQUES_2 + ) + ) + ) + ) + ); + assertExpectedResults(expectedResults, query); + } + @Test public void testFullOnTopNLongColumnWithExFn() { diff --git a/processing/src/test/java/io/druid/segment/TestIndex.java b/processing/src/test/java/io/druid/segment/TestIndex.java index d20da2a05b89..9fd84655a9e4 100644 --- a/processing/src/test/java/io/druid/segment/TestIndex.java +++ b/processing/src/test/java/io/druid/segment/TestIndex.java @@ -42,6 +42,7 @@ import io.druid.query.aggregation.hyperloglog.HyperUniquesAggregatorFactory; import io.druid.query.aggregation.hyperloglog.HyperUniquesSerde; import io.druid.query.expression.TestExprMacroTable; +import io.druid.segment.column.ValueType; import io.druid.segment.incremental.IncrementalIndex; import io.druid.segment.incremental.IncrementalIndexSchema; import io.druid.segment.incremental.OnheapIncrementalIndex; @@ -113,7 +114,7 @@ public class TestIndex private static final Interval DATA_INTERVAL = new Interval("2011-01-12T00:00:00.000Z/2011-05-01T00:00:00.000Z"); private static final VirtualColumns VIRTUAL_COLUMNS = VirtualColumns.create( Collections.singletonList( - new ExpressionVirtualColumn("expr", "index + 10", TestExprMacroTable.INSTANCE) + new ExpressionVirtualColumn("expr", "index + 10", ValueType.FLOAT, TestExprMacroTable.INSTANCE) ) ); public static final AggregatorFactory[] METRIC_AGGS = new AggregatorFactory[]{ diff --git a/processing/src/test/java/io/druid/segment/filter/BaseFilterTest.java b/processing/src/test/java/io/druid/segment/filter/BaseFilterTest.java index 3c15dce1f582..8d18672b8168 100644 --- a/processing/src/test/java/io/druid/segment/filter/BaseFilterTest.java +++ b/processing/src/test/java/io/druid/segment/filter/BaseFilterTest.java @@ -84,7 +84,7 @@ public abstract class BaseFilterTest { private static final VirtualColumns VIRTUAL_COLUMNS = VirtualColumns.create( ImmutableList.of( - new ExpressionVirtualColumn("expr", "1.0 + 0.1", TestExprMacroTable.INSTANCE) + new ExpressionVirtualColumn("expr", "1.0 + 0.1", ValueType.FLOAT, TestExprMacroTable.INSTANCE) ) ); diff --git a/processing/src/test/java/io/druid/segment/virtual/ExpressionObjectSelectorTest.java b/processing/src/test/java/io/druid/segment/virtual/ExpressionObjectSelectorTest.java index de634cf69710..82c54d5544b7 100644 --- a/processing/src/test/java/io/druid/segment/virtual/ExpressionObjectSelectorTest.java +++ b/processing/src/test/java/io/druid/segment/virtual/ExpressionObjectSelectorTest.java @@ -31,11 +31,29 @@ public class ExpressionObjectSelectorTest { + @Test + public void testSupplierFromDimensionSelector() + { + final SettableSupplier settableSupplier = new SettableSupplier<>(); + final Supplier supplier = ExpressionObjectSelector.supplierFromDimensionSelector( + dimensionSelectorFromSupplier(settableSupplier) + ); + + Assert.assertNotNull(supplier); + Assert.assertEquals(null, supplier.get()); + + settableSupplier.set(null); + Assert.assertEquals(null, supplier.get()); + + settableSupplier.set("1234"); + Assert.assertEquals("1234", supplier.get()); + } + @Test public void testSupplierFromObjectSelectorObject() { final SettableSupplier settableSupplier = new SettableSupplier<>(); - final Supplier supplier = ExpressionObjectSelector.supplierFromObjectSelector( + final Supplier supplier = ExpressionObjectSelector.supplierFromObjectSelector( objectSelectorFromSupplier(settableSupplier, Object.class) ); @@ -49,17 +67,17 @@ public void testSupplierFromObjectSelectorObject() Assert.assertEquals(1L, supplier.get()); settableSupplier.set("1234"); - Assert.assertEquals(1234L, supplier.get()); + Assert.assertEquals("1234", supplier.get()); settableSupplier.set("1.234"); - Assert.assertEquals(1.234d, supplier.get()); + Assert.assertEquals("1.234", supplier.get()); } @Test public void testSupplierFromObjectSelectorNumber() { final SettableSupplier settableSupplier = new SettableSupplier<>(); - final Supplier supplier = ExpressionObjectSelector.supplierFromObjectSelector( + final Supplier supplier = ExpressionObjectSelector.supplierFromObjectSelector( objectSelectorFromSupplier(settableSupplier, Number.class) ); @@ -78,7 +96,7 @@ public void testSupplierFromObjectSelectorNumber() public void testSupplierFromObjectSelectorString() { final SettableSupplier settableSupplier = new SettableSupplier<>(); - final Supplier supplier = ExpressionObjectSelector.supplierFromObjectSelector( + final Supplier supplier = ExpressionObjectSelector.supplierFromObjectSelector( objectSelectorFromSupplier(settableSupplier, String.class) ); @@ -86,17 +104,17 @@ public void testSupplierFromObjectSelectorString() Assert.assertEquals(null, supplier.get()); settableSupplier.set("1.1"); - Assert.assertEquals(1.1d, supplier.get()); + Assert.assertEquals("1.1", supplier.get()); settableSupplier.set("1"); - Assert.assertEquals(1L, supplier.get()); + Assert.assertEquals("1", supplier.get()); } @Test public void testSupplierFromObjectSelectorList() { final SettableSupplier settableSupplier = new SettableSupplier<>(); - final Supplier supplier = ExpressionObjectSelector.supplierFromObjectSelector( + final Supplier supplier = ExpressionObjectSelector.supplierFromObjectSelector( objectSelectorFromSupplier(settableSupplier, List.class) ); diff --git a/processing/src/test/java/io/druid/segment/virtual/ExpressionVirtualColumnTest.java b/processing/src/test/java/io/druid/segment/virtual/ExpressionVirtualColumnTest.java index 220c0b4653f6..3d71e7bec62e 100644 --- a/processing/src/test/java/io/druid/segment/virtual/ExpressionVirtualColumnTest.java +++ b/processing/src/test/java/io/druid/segment/virtual/ExpressionVirtualColumnTest.java @@ -34,6 +34,7 @@ import io.druid.segment.FloatColumnSelector; import io.druid.segment.LongColumnSelector; import io.druid.segment.ObjectColumnSelector; +import io.druid.segment.column.ValueType; import org.junit.Assert; import org.junit.Test; @@ -41,30 +42,49 @@ public class ExpressionVirtualColumnTest { private static final InputRow ROW0 = new MapBasedInputRow( 0, - ImmutableList.of(), - ImmutableMap.of() + ImmutableList.of(), + ImmutableMap.of() ); private static final InputRow ROW1 = new MapBasedInputRow( 0, - ImmutableList.of(), - ImmutableMap.of("x", 4) + ImmutableList.of(), + ImmutableMap.of("x", 4) ); private static final InputRow ROW2 = new MapBasedInputRow( 0, - ImmutableList.of(), - ImmutableMap.of("x", 2.1, "y", 3L) + ImmutableList.of(), + ImmutableMap.of("x", 2.1, "y", 3L, "z", "foobar") + ); + private static final InputRow ROW3 = new MapBasedInputRow( + 0, + ImmutableList.of(), + ImmutableMap.of("x", 2L, "y", 3L, "z", "foobar") ); private static final ExpressionVirtualColumn XPLUSY = new ExpressionVirtualColumn( "expr", "x + y", + ValueType.FLOAT, TestExprMacroTable.INSTANCE ); private static final ExpressionVirtualColumn CONSTANT_LIKE = new ExpressionVirtualColumn( "expr", "like('foo', 'f%')", + ValueType.FLOAT, + TestExprMacroTable.INSTANCE + ); + private static final ExpressionVirtualColumn ZLIKE = new ExpressionVirtualColumn( + "expr", + "like(z, 'f%')", + ValueType.FLOAT, + TestExprMacroTable.INSTANCE + ); + private static final ExpressionVirtualColumn ZCONCATX = new ExpressionVirtualColumn( + "expr", + "z + cast(x, 'string')", + ValueType.STRING, TestExprMacroTable.INSTANCE ); private static final TestColumnSelectorFactory COLUMN_SELECTOR_FACTORY = new TestColumnSelectorFactory(); @@ -78,10 +98,13 @@ public void testObjectSelector() Assert.assertEquals(null, selector.get()); COLUMN_SELECTOR_FACTORY.setRow(ROW1); - Assert.assertEquals(null, selector.get()); + Assert.assertEquals(4.0d, selector.get()); COLUMN_SELECTOR_FACTORY.setRow(ROW2); Assert.assertEquals(5.1d, selector.get()); + + COLUMN_SELECTOR_FACTORY.setRow(ROW3); + Assert.assertEquals(5L, selector.get()); } @Test @@ -93,10 +116,31 @@ public void testLongSelector() Assert.assertEquals(0L, selector.get()); COLUMN_SELECTOR_FACTORY.setRow(ROW1); - Assert.assertEquals(0L, selector.get()); + Assert.assertEquals(4L, selector.get()); COLUMN_SELECTOR_FACTORY.setRow(ROW2); Assert.assertEquals(5L, selector.get()); + + COLUMN_SELECTOR_FACTORY.setRow(ROW3); + Assert.assertEquals(5L, selector.get()); + } + + @Test + public void testLongSelectorUsingStringFunction() + { + final LongColumnSelector selector = ZCONCATX.makeLongColumnSelector("expr", COLUMN_SELECTOR_FACTORY); + + COLUMN_SELECTOR_FACTORY.setRow(ROW0); + Assert.assertEquals(0L, selector.get()); + + COLUMN_SELECTOR_FACTORY.setRow(ROW1); + Assert.assertEquals(4L, selector.get()); + + COLUMN_SELECTOR_FACTORY.setRow(ROW2); + Assert.assertEquals(0L, selector.get()); + + COLUMN_SELECTOR_FACTORY.setRow(ROW3); + Assert.assertEquals(0L, selector.get()); } @Test @@ -108,17 +152,20 @@ public void testFloatSelector() Assert.assertEquals(0.0f, selector.get(), 0.0f); COLUMN_SELECTOR_FACTORY.setRow(ROW1); - Assert.assertEquals(0.0f, selector.get(), 0.0f); + Assert.assertEquals(4.0f, selector.get(), 0.0f); COLUMN_SELECTOR_FACTORY.setRow(ROW2); Assert.assertEquals(5.1f, selector.get(), 0.0f); + + COLUMN_SELECTOR_FACTORY.setRow(ROW3); + Assert.assertEquals(5.0f, selector.get(), 0.0f); } @Test public void testDimensionSelector() { final DimensionSelector selector = XPLUSY.makeDimensionSelector( - new DefaultDimensionSpec("expr", "x"), + new DefaultDimensionSpec("expr", "expr"), COLUMN_SELECTOR_FACTORY ); @@ -133,16 +180,49 @@ public void testDimensionSelector() Assert.assertEquals(null, selector.lookupName(selector.getRow().get(0))); COLUMN_SELECTOR_FACTORY.setRow(ROW1); - Assert.assertEquals(true, nullMatcher.matches()); + Assert.assertEquals(false, nullMatcher.matches()); Assert.assertEquals(false, fiveMatcher.matches()); - Assert.assertEquals(false, nonNullMatcher.matches()); - Assert.assertEquals(null, selector.lookupName(selector.getRow().get(0))); + Assert.assertEquals(true, nonNullMatcher.matches()); + Assert.assertEquals("4.0", selector.lookupName(selector.getRow().get(0))); COLUMN_SELECTOR_FACTORY.setRow(ROW2); Assert.assertEquals(false, nullMatcher.matches()); Assert.assertEquals(false, fiveMatcher.matches()); Assert.assertEquals(true, nonNullMatcher.matches()); Assert.assertEquals("5.1", selector.lookupName(selector.getRow().get(0))); + + COLUMN_SELECTOR_FACTORY.setRow(ROW3); + Assert.assertEquals(false, nullMatcher.matches()); + Assert.assertEquals(true, fiveMatcher.matches()); + Assert.assertEquals(true, nonNullMatcher.matches()); + Assert.assertEquals("5", selector.lookupName(selector.getRow().get(0))); + } + + @Test + public void testDimensionSelectorUsingStringFunction() + { + final DimensionSelector selector = ZCONCATX.makeDimensionSelector( + new DefaultDimensionSpec("expr", "expr"), + COLUMN_SELECTOR_FACTORY + ); + + Assert.assertNotNull(selector); + + COLUMN_SELECTOR_FACTORY.setRow(ROW0); + Assert.assertEquals(1, selector.getRow().size()); + Assert.assertEquals(null, selector.lookupName(selector.getRow().get(0))); + + COLUMN_SELECTOR_FACTORY.setRow(ROW1); + Assert.assertEquals(1, selector.getRow().size()); + Assert.assertEquals("4", selector.lookupName(selector.getRow().get(0))); + + COLUMN_SELECTOR_FACTORY.setRow(ROW2); + Assert.assertEquals(1, selector.getRow().size()); + Assert.assertEquals("foobar2.1", selector.lookupName(selector.getRow().get(0))); + + COLUMN_SELECTOR_FACTORY.setRow(ROW3); + Assert.assertEquals(1, selector.getRow().size()); + Assert.assertEquals("foobar2", selector.lookupName(selector.getRow().get(0))); } @Test @@ -164,16 +244,22 @@ public void testDimensionSelectorWithExtraction() Assert.assertEquals(null, selector.lookupName(selector.getRow().get(0))); COLUMN_SELECTOR_FACTORY.setRow(ROW1); - Assert.assertEquals(true, nullMatcher.matches()); + Assert.assertEquals(false, nullMatcher.matches()); Assert.assertEquals(false, fiveMatcher.matches()); - Assert.assertEquals(false, nonNullMatcher.matches()); - Assert.assertEquals(null, selector.lookupName(selector.getRow().get(0))); + Assert.assertEquals(true, nonNullMatcher.matches()); + Assert.assertEquals("4", selector.lookupName(selector.getRow().get(0))); COLUMN_SELECTOR_FACTORY.setRow(ROW2); Assert.assertEquals(false, nullMatcher.matches()); Assert.assertEquals(true, fiveMatcher.matches()); Assert.assertEquals(true, nonNullMatcher.matches()); Assert.assertEquals("5", selector.lookupName(selector.getRow().get(0))); + + COLUMN_SELECTOR_FACTORY.setRow(ROW3); + Assert.assertEquals(false, nullMatcher.matches()); + Assert.assertEquals(true, fiveMatcher.matches()); + Assert.assertEquals(true, nonNullMatcher.matches()); + Assert.assertEquals("5", selector.lookupName(selector.getRow().get(0))); } @Test @@ -185,9 +271,30 @@ public void testLongSelectorWithConstantLikeExprMacro() Assert.assertEquals(1L, selector.get()); } + @Test + public void testLongSelectorWithZLikeExprMacro() + { + final LongColumnSelector selector = ZLIKE.makeLongColumnSelector("expr", COLUMN_SELECTOR_FACTORY); + + COLUMN_SELECTOR_FACTORY.setRow(ROW0); + Assert.assertEquals(0L, selector.get()); + + COLUMN_SELECTOR_FACTORY.setRow(ROW1); + Assert.assertEquals(0L, selector.get()); + + COLUMN_SELECTOR_FACTORY.setRow(ROW2); + Assert.assertEquals(1L, selector.get()); + + COLUMN_SELECTOR_FACTORY.setRow(ROW3); + Assert.assertEquals(1L, selector.get()); + } + @Test public void testRequiredColumns() { Assert.assertEquals(ImmutableList.of("x", "y"), XPLUSY.requiredColumns()); + Assert.assertEquals(ImmutableList.of(), CONSTANT_LIKE.requiredColumns()); + Assert.assertEquals(ImmutableList.of("z"), ZLIKE.requiredColumns()); + Assert.assertEquals(ImmutableList.of("z", "x"), ZCONCATX.requiredColumns()); } } diff --git a/processing/src/test/java/io/druid/segment/virtual/VirtualColumnsTest.java b/processing/src/test/java/io/druid/segment/virtual/VirtualColumnsTest.java index be16dc9e7f93..979a055c0e7f 100644 --- a/processing/src/test/java/io/druid/segment/virtual/VirtualColumnsTest.java +++ b/processing/src/test/java/io/druid/segment/virtual/VirtualColumnsTest.java @@ -127,6 +127,7 @@ public void testTimeNotAllowed() final ExpressionVirtualColumn expr = new ExpressionVirtualColumn( "__time", "x + y", + ValueType.FLOAT, TestExprMacroTable.INSTANCE ); @@ -142,12 +143,14 @@ public void testDuplicateNameDetection() final ExpressionVirtualColumn expr = new ExpressionVirtualColumn( "expr", "x + y", + ValueType.FLOAT, TestExprMacroTable.INSTANCE ); final ExpressionVirtualColumn expr2 = new ExpressionVirtualColumn( "expr", "x * 2", + ValueType.FLOAT, TestExprMacroTable.INSTANCE ); @@ -163,12 +166,14 @@ public void testCycleDetection() final ExpressionVirtualColumn expr = new ExpressionVirtualColumn( "expr", "x + expr2", + ValueType.FLOAT, TestExprMacroTable.INSTANCE ); final ExpressionVirtualColumn expr2 = new ExpressionVirtualColumn( "expr2", "expr * 2", + ValueType.FLOAT, TestExprMacroTable.INSTANCE ); @@ -183,13 +188,13 @@ public void testGetCacheKey() throws Exception { final VirtualColumns virtualColumns = VirtualColumns.create( ImmutableList.of( - new ExpressionVirtualColumn("expr", "x + y", TestExprMacroTable.INSTANCE) + new ExpressionVirtualColumn("expr", "x + y", ValueType.FLOAT, TestExprMacroTable.INSTANCE) ) ); final VirtualColumns virtualColumns2 = VirtualColumns.create( ImmutableList.of( - new ExpressionVirtualColumn("expr", "x + y", TestExprMacroTable.INSTANCE) + new ExpressionVirtualColumn("expr", "x + y", ValueType.FLOAT, TestExprMacroTable.INSTANCE) ) ); @@ -202,13 +207,13 @@ public void testEqualsAndHashCode() throws Exception { final VirtualColumns virtualColumns = VirtualColumns.create( ImmutableList.of( - new ExpressionVirtualColumn("expr", "x + y", TestExprMacroTable.INSTANCE) + new ExpressionVirtualColumn("expr", "x + y", ValueType.FLOAT, TestExprMacroTable.INSTANCE) ) ); final VirtualColumns virtualColumns2 = VirtualColumns.create( ImmutableList.of( - new ExpressionVirtualColumn("expr", "x + y", TestExprMacroTable.INSTANCE) + new ExpressionVirtualColumn("expr", "x + y", ValueType.FLOAT, TestExprMacroTable.INSTANCE) ) ); @@ -227,8 +232,8 @@ public void testSerde() throws Exception { final ObjectMapper mapper = TestHelper.getJsonMapper(); final ImmutableList theColumns = ImmutableList.of( - new ExpressionVirtualColumn("expr", "x + y", TestExprMacroTable.INSTANCE), - new ExpressionVirtualColumn("expr2", "x + z", TestExprMacroTable.INSTANCE) + new ExpressionVirtualColumn("expr", "x + y", ValueType.FLOAT, TestExprMacroTable.INSTANCE), + new ExpressionVirtualColumn("expr2", "x + z", ValueType.FLOAT, TestExprMacroTable.INSTANCE) ); final VirtualColumns virtualColumns = VirtualColumns.create(theColumns); @@ -254,6 +259,7 @@ private VirtualColumns makeVirtualColumns() final ExpressionVirtualColumn expr = new ExpressionVirtualColumn( "expr", "1", + ValueType.FLOAT, TestExprMacroTable.INSTANCE ); final DottyVirtualColumn dotty = new DottyVirtualColumn("foo");