From 3c63cff57abc6d3c64d521c365b7f87db4197f26 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Thu, 5 Jan 2017 18:06:38 -0800 Subject: [PATCH] Remove makeMathExpressionSelector from ColumnSelectorFactory. (#3815) * Remove makeMathExpressionSelector from ColumnSelectorFactory. * Add @Nullable annotations in places, fix Number.class check. * Break up createBindings, add tests. * Add null check. --- .../query/aggregation/AggregatorUtil.java | 27 +-- .../RowBasedColumnSelectorFactory.java | 42 +---- .../druid/segment/ColumnSelectorFactory.java | 1 - .../druid/segment/NumericColumnSelector.java | 27 --- .../segment/QueryableIndexStorageAdapter.java | 60 +----- .../java/io/druid/segment/StorageAdapter.java | 13 ++ .../segment/incremental/IncrementalIndex.java | 8 +- .../IncrementalIndexStorageAdapter.java | 57 +----- .../incremental/OnheapIncrementalIndex.java | 9 +- .../virtual/ExpressionObjectSelector.java | 174 ++++++++++++++++++ .../segment/virtual/ExpressionSelectors.java | 77 ++++++++ .../aggregation/FilteredAggregatorTest.java | 7 - .../aggregation/JavaScriptAggregatorTest.java | 7 - .../TestColumnSelectorFactory.java | 7 - .../virtual/ExpressionObjectSelectorTest.java | 158 ++++++++++++++++ 15 files changed, 436 insertions(+), 238 deletions(-) delete mode 100644 processing/src/main/java/io/druid/segment/NumericColumnSelector.java create mode 100644 processing/src/main/java/io/druid/segment/virtual/ExpressionObjectSelector.java create mode 100644 processing/src/main/java/io/druid/segment/virtual/ExpressionSelectors.java create mode 100644 processing/src/test/java/io/druid/segment/virtual/ExpressionObjectSelectorTest.java diff --git a/processing/src/main/java/io/druid/query/aggregation/AggregatorUtil.java b/processing/src/main/java/io/druid/query/aggregation/AggregatorUtil.java index e106ef828c5..6e3f8f1ec55 100644 --- a/processing/src/main/java/io/druid/query/aggregation/AggregatorUtil.java +++ b/processing/src/main/java/io/druid/query/aggregation/AggregatorUtil.java @@ -20,11 +20,12 @@ package io.druid.query.aggregation; import com.google.common.collect.Lists; +import io.druid.java.util.common.Pair; +import io.druid.math.expr.Parser; import io.druid.segment.ColumnSelectorFactory; import io.druid.segment.FloatColumnSelector; import io.druid.segment.LongColumnSelector; -import io.druid.segment.NumericColumnSelector; -import io.druid.java.util.common.Pair; +import io.druid.segment.virtual.ExpressionSelectors; import java.util.HashSet; import java.util.LinkedList; @@ -100,16 +101,7 @@ public class AggregatorUtil return metricFactory.makeFloatColumnSelector(fieldName); } if (fieldName == null && fieldExpression != null) { - final NumericColumnSelector numeric = metricFactory.makeMathExpressionSelector(fieldExpression); - return new FloatColumnSelector() - { - @Override - public float get() - { - final Number number = numeric.get(); - return number == null ? nullValue : number.floatValue(); - } - }; + return ExpressionSelectors.makeFloatColumnSelector(metricFactory, Parser.parse(fieldExpression), nullValue); } throw new IllegalArgumentException("Must have a valid, non-null fieldName or expression"); } @@ -125,16 +117,7 @@ public class AggregatorUtil return metricFactory.makeLongColumnSelector(fieldName); } if (fieldName == null && fieldExpression != null) { - final NumericColumnSelector numeric = metricFactory.makeMathExpressionSelector(fieldExpression); - return new LongColumnSelector() - { - @Override - public long get() - { - final Number number = numeric.get(); - return number == null ? nullValue : number.longValue(); - } - }; + return ExpressionSelectors.makeLongColumnSelector(metricFactory, Parser.parse(fieldExpression), nullValue); } throw new IllegalArgumentException("Must have a valid, non-null fieldName or expression"); } diff --git a/processing/src/main/java/io/druid/query/groupby/RowBasedColumnSelectorFactory.java b/processing/src/main/java/io/druid/query/groupby/RowBasedColumnSelectorFactory.java index 378543e48f8..c13f9f2c866 100644 --- a/processing/src/main/java/io/druid/query/groupby/RowBasedColumnSelectorFactory.java +++ b/processing/src/main/java/io/druid/query/groupby/RowBasedColumnSelectorFactory.java @@ -22,18 +22,13 @@ package io.druid.query.groupby; import com.google.common.base.Strings; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Maps; import io.druid.data.input.Row; -import io.druid.math.expr.Evals; -import io.druid.math.expr.Expr; -import io.druid.math.expr.Parser; import io.druid.query.dimension.DimensionSpec; import io.druid.query.extraction.ExtractionFn; import io.druid.segment.ColumnSelectorFactory; import io.druid.segment.DimensionSelector; import io.druid.segment.FloatColumnSelector; import io.druid.segment.LongColumnSelector; -import io.druid.segment.NumericColumnSelector; import io.druid.segment.ObjectColumnSelector; import io.druid.segment.column.Column; import io.druid.segment.column.ColumnCapabilities; @@ -247,38 +242,7 @@ public class RowBasedColumnSelectorFactory implements ColumnSelectorFactory } } - @Override - public NumericColumnSelector makeMathExpressionSelector(String expression) - { - final Expr parsed = Parser.parse(expression); - - final List required = Parser.findRequiredBindings(parsed); - final Map> values = Maps.newHashMapWithExpectedSize(required.size()); - - for (final String columnName : required) { - values.put( - columnName, new Supplier() - { - @Override - public Number get() - { - return Evals.toNumber(row.get().getRaw(columnName)); - } - } - ); - } - final Expr.ObjectBinding binding = Parser.withSuppliers(values); - - return new NumericColumnSelector() - { - @Override - public Number get() - { - return parsed.eval(binding).numericValue(); - } - }; - } - + @Nullable @Override public ColumnCapabilities getColumnCapabilities(String columnName) { @@ -289,9 +253,7 @@ public class RowBasedColumnSelectorFactory implements ColumnSelectorFactory final ValueType valueType = rowSignature.get(columnName); // Do _not_ set isDictionaryEncoded or hasBitmapIndexes, because Row-based columns do not have those things. - return valueType != null - ? new ColumnCapabilitiesImpl().setType(valueType) - : new ColumnCapabilitiesImpl().setType(ValueType.STRING); + return valueType != null ? new ColumnCapabilitiesImpl().setType(valueType) : null; } } } diff --git a/processing/src/main/java/io/druid/segment/ColumnSelectorFactory.java b/processing/src/main/java/io/druid/segment/ColumnSelectorFactory.java index afb38476c04..f550fef14e5 100644 --- a/processing/src/main/java/io/druid/segment/ColumnSelectorFactory.java +++ b/processing/src/main/java/io/druid/segment/ColumnSelectorFactory.java @@ -31,6 +31,5 @@ public interface ColumnSelectorFactory public FloatColumnSelector makeFloatColumnSelector(String columnName); public LongColumnSelector makeLongColumnSelector(String columnName); public ObjectColumnSelector makeObjectColumnSelector(String columnName); - public NumericColumnSelector makeMathExpressionSelector(String expression); public ColumnCapabilities getColumnCapabilities(String columnName); } diff --git a/processing/src/main/java/io/druid/segment/NumericColumnSelector.java b/processing/src/main/java/io/druid/segment/NumericColumnSelector.java deleted file mode 100644 index 576211d38e4..00000000000 --- a/processing/src/main/java/io/druid/segment/NumericColumnSelector.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Licensed to Metamarkets Group Inc. (Metamarkets) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. Metamarkets licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package io.druid.segment; - -/** - */ -public interface NumericColumnSelector -{ - Number get(); -} diff --git a/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java b/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java index 835aa357b83..4750ba771da 100644 --- a/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java +++ b/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java @@ -21,7 +21,6 @@ package io.druid.segment; import com.google.common.base.Function; import com.google.common.base.Predicates; -import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -31,8 +30,6 @@ import io.druid.collections.bitmap.ImmutableBitmap; import io.druid.granularity.QueryGranularity; import io.druid.java.util.common.guava.Sequence; import io.druid.java.util.common.guava.Sequences; -import io.druid.math.expr.Expr; -import io.druid.math.expr.Parser; import io.druid.query.QueryInterruptedException; import io.druid.query.dimension.DimensionSpec; import io.druid.query.extraction.ExtractionFn; @@ -58,6 +55,7 @@ import org.joda.time.DateTime; import org.joda.time.Interval; import org.roaringbitmap.IntIterator; +import javax.annotation.Nullable; import java.io.Closeable; import java.io.IOException; import java.util.ArrayList; @@ -800,61 +798,7 @@ public class QueryableIndexStorageAdapter implements StorageAdapter }; } - @Override - public NumericColumnSelector makeMathExpressionSelector(String expression) - { - final Expr parsed = Parser.parse(expression); - final List required = Parser.findRequiredBindings(parsed); - - final Map> values = Maps.newHashMapWithExpectedSize(required.size()); - for (String columnName : index.getColumnNames()) { - if (!required.contains(columnName)) { - continue; - } - final GenericColumn column = index.getColumn(columnName).getGenericColumn(); - if (column == null) { - continue; - } - closer.register(column); - if (column.getType() == ValueType.FLOAT) { - values.put( - columnName, new Supplier() - { - @Override - public Number get() - { - return column.getFloatSingleValueRow(cursorOffset.getOffset()); - } - } - ); - } else if (column.getType() == ValueType.LONG) { - values.put( - columnName, new Supplier() - { - @Override - public Number get() - { - return column.getLongSingleValueRow(cursorOffset.getOffset()); - } - } - ); - } else { - throw new UnsupportedOperationException( - "Not supported type " + column.getType() + " for column " + columnName - ); - } - } - final Expr.ObjectBinding binding = Parser.withSuppliers(values); - return new NumericColumnSelector() - { - @Override - public Number get() - { - return parsed.eval(binding).numericValue(); - } - }; - } - + @Nullable @Override public ColumnCapabilities getColumnCapabilities(String columnName) { diff --git a/processing/src/main/java/io/druid/segment/StorageAdapter.java b/processing/src/main/java/io/druid/segment/StorageAdapter.java index 603c8dde741..82b181a9bed 100644 --- a/processing/src/main/java/io/druid/segment/StorageAdapter.java +++ b/processing/src/main/java/io/druid/segment/StorageAdapter.java @@ -24,6 +24,7 @@ import io.druid.segment.data.Indexed; import org.joda.time.DateTime; import org.joda.time.Interval; +import javax.annotation.Nullable; import java.util.Map; /** @@ -49,7 +50,19 @@ public interface StorageAdapter extends CursorFactory public Comparable getMinValue(String column); public Comparable getMaxValue(String column); public Capabilities getCapabilities(); + + /** + * Returns capabilities of a particular column, if known. May be null if the column doesn't exist, or if + * the column does exist but the capabilities are unknown. The latter is possible with dynamically discovered + * columns. + * + * @param column column name + * + * @return capabilities, or null + */ + @Nullable public ColumnCapabilities getColumnCapabilities(String column); + public Map getDimensionHandlers(); /** diff --git a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndex.java b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndex.java index 3660099c300..431ebe18df6 100644 --- a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndex.java +++ b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndex.java @@ -52,7 +52,6 @@ import io.druid.segment.DimensionSelector; import io.druid.segment.FloatColumnSelector; import io.druid.segment.LongColumnSelector; import io.druid.segment.Metadata; -import io.druid.segment.NumericColumnSelector; import io.druid.segment.ObjectColumnSelector; import io.druid.segment.column.Column; import io.druid.segment.column.ColumnCapabilities; @@ -162,17 +161,12 @@ public abstract class IncrementalIndex implements Iterable, return baseSelectorFactory.makeDimensionSelector(dimensionSpec); } + @Nullable @Override public ColumnCapabilities getColumnCapabilities(String columnName) { return baseSelectorFactory.getColumnCapabilities(columnName); } - - @Override - public NumericColumnSelector makeMathExpressionSelector(String expression) - { - return baseSelectorFactory.makeMathExpressionSelector(expression); - } }; } diff --git a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java index f52739356f5..364445a0f1f 100644 --- a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java +++ b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java @@ -20,16 +20,12 @@ package io.druid.segment.incremental; import com.google.common.base.Function; -import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterators; import com.google.common.collect.Lists; -import com.google.common.collect.Maps; import io.druid.granularity.QueryGranularity; import io.druid.java.util.common.guava.Sequence; import io.druid.java.util.common.guava.Sequences; -import io.druid.math.expr.Expr; -import io.druid.math.expr.Parser; import io.druid.query.QueryInterruptedException; import io.druid.query.dimension.DefaultDimensionSpec; import io.druid.query.dimension.DimensionSpec; @@ -45,7 +41,6 @@ import io.druid.segment.FloatColumnSelector; import io.druid.segment.LongColumnSelector; import io.druid.segment.Metadata; import io.druid.segment.NullDimensionSelector; -import io.druid.segment.NumericColumnSelector; import io.druid.segment.ObjectColumnSelector; import io.druid.segment.SingleScanTimeDimSelector; import io.druid.segment.StorageAdapter; @@ -61,8 +56,8 @@ import io.druid.segment.filter.BooleanValueMatcher; import org.joda.time.DateTime; import org.joda.time.Interval; +import javax.annotation.Nullable; import java.util.Iterator; -import java.util.List; import java.util.Map; /** @@ -540,6 +535,7 @@ public class IncrementalIndexStorageAdapter implements StorageAdapter } } + @Nullable @Override public ColumnCapabilities getColumnCapabilities(String columnName) { @@ -553,55 +549,6 @@ public class IncrementalIndexStorageAdapter implements StorageAdapter } return capabilities; } - - @Override - public NumericColumnSelector makeMathExpressionSelector(String expression) - { - final Expr parsed = Parser.parse(expression); - - final List required = Parser.findRequiredBindings(parsed); - final Map> values = Maps.newHashMapWithExpectedSize(required.size()); - - for (String columnName : index.getMetricNames()) { - if (!required.contains(columnName)) { - continue; - } - ValueType type = index.getCapabilities(columnName).getType(); - if (type == ValueType.FLOAT) { - final int metricIndex = index.getMetricIndex(columnName); - values.put( - columnName, new Supplier() - { - @Override - public Number get() - { - return index.getMetricFloatValue(currEntry.getValue(), metricIndex); - } - } - ); - } else if (type == ValueType.LONG) { - final int metricIndex = index.getMetricIndex(columnName); - values.put( - columnName, new Supplier() - { - @Override - public Number get() - { - return index.getMetricLongValue(currEntry.getValue(), metricIndex); - } - } - ); - } - } - final Expr.ObjectBinding binding = Parser.withSuppliers(values); - return new NumericColumnSelector() { - @Override - public Number get() - { - return parsed.eval(binding).numericValue(); - } - }; - } }; } } diff --git a/processing/src/main/java/io/druid/segment/incremental/OnheapIncrementalIndex.java b/processing/src/main/java/io/druid/segment/incremental/OnheapIncrementalIndex.java index a278ec19bf6..d5be53ae9a6 100644 --- a/processing/src/main/java/io/druid/segment/incremental/OnheapIncrementalIndex.java +++ b/processing/src/main/java/io/druid/segment/incremental/OnheapIncrementalIndex.java @@ -33,10 +33,10 @@ import io.druid.segment.ColumnSelectorFactory; import io.druid.segment.DimensionSelector; import io.druid.segment.FloatColumnSelector; import io.druid.segment.LongColumnSelector; -import io.druid.segment.NumericColumnSelector; import io.druid.segment.ObjectColumnSelector; import io.druid.segment.column.ColumnCapabilities; +import javax.annotation.Nullable; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -403,17 +403,12 @@ public class OnheapIncrementalIndex extends IncrementalIndex } } + @Nullable @Override public ColumnCapabilities getColumnCapabilities(String columnName) { return delegate.getColumnCapabilities(columnName); } - - @Override - public NumericColumnSelector makeMathExpressionSelector(String expression) - { - return delegate.makeMathExpressionSelector(expression); - } } } diff --git a/processing/src/main/java/io/druid/segment/virtual/ExpressionObjectSelector.java b/processing/src/main/java/io/druid/segment/virtual/ExpressionObjectSelector.java new file mode 100644 index 00000000000..239ff8a435b --- /dev/null +++ b/processing/src/main/java/io/druid/segment/virtual/ExpressionObjectSelector.java @@ -0,0 +1,174 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.druid.segment.virtual; + +import com.google.common.annotations.VisibleForTesting; +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.Parser; +import io.druid.segment.ColumnSelectorFactory; +import io.druid.segment.FloatColumnSelector; +import io.druid.segment.LongColumnSelector; +import io.druid.segment.ObjectColumnSelector; +import io.druid.segment.column.ColumnCapabilities; +import io.druid.segment.column.ValueType; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import java.util.Map; + +public class ExpressionObjectSelector implements ObjectColumnSelector +{ + private final Expr expression; + private final Expr.ObjectBinding bindings; + + private ExpressionObjectSelector(Expr.ObjectBinding bindings, Expr expression) + { + this.bindings = Preconditions.checkNotNull(bindings, "bindings"); + this.expression = Preconditions.checkNotNull(expression, "expression"); + } + + public 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(); + for (String columnName : Parser.findRequiredBindings(expression)) { + final ColumnCapabilities columnCapabilities = columnSelectorFactory.getColumnCapabilities(columnName); + final ValueType nativeType = columnCapabilities != null ? columnCapabilities.getType() : null; + final Supplier supplier; + + if (nativeType == ValueType.FLOAT) { + supplier = supplierFromFloatSelector(columnSelectorFactory.makeFloatColumnSelector(columnName)); + } else if (nativeType == ValueType.LONG) { + supplier = supplierFromLongSelector(columnSelectorFactory.makeLongColumnSelector(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). + supplier = null; + } + + if (supplier != null) { + suppliers.put(columnName, supplier); + } + } + + return Parser.withSuppliers(suppliers); + } + + @VisibleForTesting + @Nonnull + static Supplier supplierFromFloatSelector(final FloatColumnSelector 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(); + } + }; + } + + @VisibleForTesting + @Nullable + 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. + return null; + } + } + + @Nullable + private static Number tryParse(final Object value) + { + if (value == null) { + 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() + { + return Number.class; + } + + @Override + public Number get() + { + return expression.eval(bindings).numericValue(); + } +} diff --git a/processing/src/main/java/io/druid/segment/virtual/ExpressionSelectors.java b/processing/src/main/java/io/druid/segment/virtual/ExpressionSelectors.java new file mode 100644 index 00000000000..7273d95759e --- /dev/null +++ b/processing/src/main/java/io/druid/segment/virtual/ExpressionSelectors.java @@ -0,0 +1,77 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.druid.segment.virtual; + +import io.druid.math.expr.Expr; +import io.druid.segment.ColumnSelectorFactory; +import io.druid.segment.FloatColumnSelector; +import io.druid.segment.LongColumnSelector; + +public class ExpressionSelectors +{ + private ExpressionSelectors() + { + // No instantiation. + } + + public static ExpressionObjectSelector makeObjectColumnSelector( + final ColumnSelectorFactory columnSelectorFactory, + final Expr expression + ) + { + return ExpressionObjectSelector.from(columnSelectorFactory, expression); + } + + public static LongColumnSelector makeLongColumnSelector( + final ColumnSelectorFactory columnSelectorFactory, + final Expr expression, + final long nullValue + ) + { + final ExpressionObjectSelector baseSelector = ExpressionObjectSelector.from(columnSelectorFactory, expression); + return new LongColumnSelector() + { + @Override + public long get() + { + final Number number = baseSelector.get(); + return number != null ? number.longValue() : nullValue; + } + }; + } + + public static FloatColumnSelector makeFloatColumnSelector( + final ColumnSelectorFactory columnSelectorFactory, + final Expr expression, + final float nullValue + ) + { + final ExpressionObjectSelector baseSelector = ExpressionObjectSelector.from(columnSelectorFactory, expression); + return new FloatColumnSelector() + { + @Override + public float get() + { + final Number number = baseSelector.get(); + return number != null ? number.floatValue() : nullValue; + } + }; + } +} diff --git a/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java b/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java index 042c5ed34a6..f3a86bab175 100644 --- a/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java @@ -40,7 +40,6 @@ import io.druid.segment.ColumnSelectorFactory; import io.druid.segment.DimensionSelector; import io.druid.segment.FloatColumnSelector; import io.druid.segment.LongColumnSelector; -import io.druid.segment.NumericColumnSelector; import io.druid.segment.ObjectColumnSelector; import io.druid.segment.column.ColumnCapabilities; import io.druid.segment.column.ColumnCapabilitiesImpl; @@ -183,12 +182,6 @@ public class FilteredAggregatorTest } return caps; } - - @Override - public NumericColumnSelector makeMathExpressionSelector(String expression) - { - throw new UnsupportedOperationException(); - } }; } diff --git a/processing/src/test/java/io/druid/query/aggregation/JavaScriptAggregatorTest.java b/processing/src/test/java/io/druid/query/aggregation/JavaScriptAggregatorTest.java index 3bb6d26fe65..9c1ecb5ca98 100644 --- a/processing/src/test/java/io/druid/query/aggregation/JavaScriptAggregatorTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/JavaScriptAggregatorTest.java @@ -28,7 +28,6 @@ import io.druid.segment.ColumnSelectorFactory; import io.druid.segment.DimensionSelector; import io.druid.segment.FloatColumnSelector; import io.druid.segment.LongColumnSelector; -import io.druid.segment.NumericColumnSelector; import io.druid.segment.ObjectColumnSelector; import io.druid.segment.column.ColumnCapabilities; import org.junit.Assert; @@ -77,12 +76,6 @@ public class JavaScriptAggregatorTest { return null; } - - @Override - public NumericColumnSelector makeMathExpressionSelector(String expression) - { - return null; - } }; static { diff --git a/processing/src/test/java/io/druid/query/groupby/epinephelinae/TestColumnSelectorFactory.java b/processing/src/test/java/io/druid/query/groupby/epinephelinae/TestColumnSelectorFactory.java index 8373698976c..77c57c0bf3b 100644 --- a/processing/src/test/java/io/druid/query/groupby/epinephelinae/TestColumnSelectorFactory.java +++ b/processing/src/test/java/io/druid/query/groupby/epinephelinae/TestColumnSelectorFactory.java @@ -25,7 +25,6 @@ import io.druid.segment.ColumnSelectorFactory; import io.druid.segment.DimensionSelector; import io.druid.segment.FloatColumnSelector; import io.druid.segment.LongColumnSelector; -import io.druid.segment.NumericColumnSelector; import io.druid.segment.ObjectColumnSelector; import io.druid.segment.column.ColumnCapabilities; @@ -89,12 +88,6 @@ public class TestColumnSelectorFactory implements ColumnSelectorFactory }; } - @Override - public NumericColumnSelector makeMathExpressionSelector(String expression) - { - throw new UnsupportedOperationException("expression is not supported in current context"); - } - @Override public ColumnCapabilities getColumnCapabilities(String columnName) { diff --git a/processing/src/test/java/io/druid/segment/virtual/ExpressionObjectSelectorTest.java b/processing/src/test/java/io/druid/segment/virtual/ExpressionObjectSelectorTest.java new file mode 100644 index 00000000000..b2064c30dc6 --- /dev/null +++ b/processing/src/test/java/io/druid/segment/virtual/ExpressionObjectSelectorTest.java @@ -0,0 +1,158 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.druid.segment.virtual; + +import com.google.common.base.Supplier; +import io.druid.common.guava.SettableSupplier; +import io.druid.segment.FloatColumnSelector; +import io.druid.segment.LongColumnSelector; +import io.druid.segment.ObjectColumnSelector; +import org.junit.Assert; +import org.junit.Test; + +import java.util.List; + +public class ExpressionObjectSelectorTest +{ + @Test + public void testSupplierFromLongSelector() + { + final Supplier supplier = ExpressionObjectSelector.supplierFromLongSelector( + new LongColumnSelector() + { + @Override + public long get() + { + return 1L; + } + } + ); + + Assert.assertEquals(1L, supplier.get()); + } + + @Test + public void testSupplierFromFloatSelector() + { + final Supplier supplier = ExpressionObjectSelector.supplierFromFloatSelector( + new FloatColumnSelector() + { + @Override + public float get() + { + return 0.1f; + } + } + ); + + Assert.assertEquals(0.1f, supplier.get()); + } + + @Test + public void testSupplierFromObjectSelectorObject() + { + final SettableSupplier settableSupplier = new SettableSupplier<>(); + final Supplier supplier = ExpressionObjectSelector.supplierFromObjectSelector( + selectorFromSupplier(settableSupplier, Object.class) + ); + + Assert.assertNotNull(supplier); + Assert.assertEquals(null, supplier.get()); + + settableSupplier.set(1.1f); + Assert.assertEquals(1.1f, supplier.get()); + + settableSupplier.set(1L); + Assert.assertEquals(1L, supplier.get()); + + settableSupplier.set("1234"); + Assert.assertEquals(1234L, supplier.get()); + + settableSupplier.set("1.234"); + Assert.assertEquals(1.234d, supplier.get()); + } + + @Test + public void testSupplierFromObjectSelectorNumber() + { + final SettableSupplier settableSupplier = new SettableSupplier<>(); + final Supplier supplier = ExpressionObjectSelector.supplierFromObjectSelector( + selectorFromSupplier(settableSupplier, Number.class) + ); + + + Assert.assertNotNull(supplier); + Assert.assertEquals(null, supplier.get()); + + settableSupplier.set(1.1f); + Assert.assertEquals(1.1f, supplier.get()); + + settableSupplier.set(1L); + Assert.assertEquals(1L, supplier.get()); + } + + @Test + public void testSupplierFromObjectSelectorString() + { + final SettableSupplier settableSupplier = new SettableSupplier<>(); + final Supplier supplier = ExpressionObjectSelector.supplierFromObjectSelector( + selectorFromSupplier(settableSupplier, String.class) + ); + + Assert.assertNotNull(supplier); + Assert.assertEquals(null, supplier.get()); + + settableSupplier.set("1.1"); + Assert.assertEquals(1.1d, supplier.get()); + + settableSupplier.set("1"); + Assert.assertEquals(1L, supplier.get()); + } + + @Test + public void testSupplierFromObjectSelectorList() + { + final SettableSupplier settableSupplier = new SettableSupplier<>(); + final Supplier supplier = ExpressionObjectSelector.supplierFromObjectSelector( + selectorFromSupplier(settableSupplier, List.class) + ); + + // List can't be a number, so supplierFromObjectSelector should return null. + Assert.assertNull(supplier); + } + + private static ObjectColumnSelector selectorFromSupplier(final Supplier supplier, final Class clazz) + { + return new ObjectColumnSelector() + { + @Override + public Class classOfObject() + { + return clazz; + } + + @Override + public T get() + { + return supplier.get(); + } + }; + } +}