This is an automated email from the ASF dual-hosted git repository. jihoonson pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-druid.git
The following commit(s) were added to refs/heads/master by this push: new a2a1a1c Hide NullDimensionSelector from public (#6480) a2a1a1c is described below commit a2a1a1c2c9c494959911ae09d65cf68c73b7f41e Author: Roman Leventov <leventov...@gmail.com> AuthorDate: Fri Nov 2 12:38:21 2018 +0100 Hide NullDimensionSelector from public (#6480) --- .../ArrayOfDoublesSketchAggregatorFactory.java | 5 +- .../druid/query/search/SearchQueryRunner.java | 3 +- .../druid/segment/ConstantDimensionSelector.java | 4 +- .../apache/druid/segment/DimensionSelector.java | 140 +++++++++++++++++++++ .../druid/segment/DimensionSelectorUtils.java | 30 ----- .../druid/segment/NullDimensionSelector.java | 128 ------------------- .../QueryableIndexColumnSelectorFactory.java | 4 +- .../IncrementalIndexColumnSelectorFactory.java | 5 +- .../druid/segment/virtual/ExpressionSelectors.java | 6 +- .../segment/ConstantDimensionSelectorTest.java | 8 +- 10 files changed, 155 insertions(+), 178 deletions(-) diff --git a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchAggregatorFactory.java b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchAggregatorFactory.java index 8168fdd..0b63c31 100644 --- a/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchAggregatorFactory.java +++ b/extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/tuple/ArrayOfDoublesSketchAggregatorFactory.java @@ -40,7 +40,6 @@ import org.apache.druid.segment.BaseObjectColumnValueSelector; import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.ColumnValueSelector; import org.apache.druid.segment.DimensionSelector; -import org.apache.druid.segment.DimensionSelectorUtils; import org.apache.druid.segment.NilColumnValueSelector; import javax.annotation.Nullable; @@ -101,7 +100,7 @@ public class ArrayOfDoublesSketchAggregatorFactory extends AggregatorFactory // input is raw data (key and array of values), use build aggregator final DimensionSelector keySelector = metricFactory .makeDimensionSelector(new DefaultDimensionSpec(fieldName, fieldName)); - if (DimensionSelectorUtils.isNilSelector(keySelector)) { + if (DimensionSelector.isNilSelector(keySelector)) { return new ArrayOfDoublesSketchNoOpAggregator(numberOfValues); } final List<BaseDoubleColumnValueSelector> valueSelectors = new ArrayList<>(); @@ -131,7 +130,7 @@ public class ArrayOfDoublesSketchAggregatorFactory extends AggregatorFactory // input is raw data (key and array of values), use build aggregator final DimensionSelector keySelector = metricFactory .makeDimensionSelector(new DefaultDimensionSpec(fieldName, fieldName)); - if (DimensionSelectorUtils.isNilSelector(keySelector)) { + if (DimensionSelector.isNilSelector(keySelector)) { return new ArrayOfDoublesSketchNoOpBufferAggregator(numberOfValues); } final List<BaseDoubleColumnValueSelector> valueSelectors = new ArrayList<>(); diff --git a/processing/src/main/java/org/apache/druid/query/search/SearchQueryRunner.java b/processing/src/main/java/org/apache/druid/query/search/SearchQueryRunner.java index a29a92c..26d7843 100644 --- a/processing/src/main/java/org/apache/druid/query/search/SearchQueryRunner.java +++ b/processing/src/main/java/org/apache/druid/query/search/SearchQueryRunner.java @@ -41,7 +41,6 @@ import org.apache.druid.segment.BaseFloatColumnValueSelector; import org.apache.druid.segment.BaseLongColumnValueSelector; import org.apache.druid.segment.ColumnValueSelector; import org.apache.druid.segment.DimensionSelector; -import org.apache.druid.segment.DimensionSelectorUtils; import org.apache.druid.segment.Segment; import org.apache.druid.segment.column.ColumnCapabilities; import org.apache.druid.segment.column.ValueType; @@ -127,7 +126,7 @@ public class SearchQueryRunner implements QueryRunner<Result<SearchResultValue>> final Object2IntRBTreeMap<SearchHit> set ) { - if (!DimensionSelectorUtils.isNilSelector(selector)) { + if (!DimensionSelector.isNilSelector(selector)) { final IndexedInts row = selector.getRow(); for (int i = 0, rowSize = row.size(); i < rowSize; ++i) { final String dimVal = selector.lookupName(row.get(i)); diff --git a/processing/src/main/java/org/apache/druid/segment/ConstantDimensionSelector.java b/processing/src/main/java/org/apache/druid/segment/ConstantDimensionSelector.java index a50afc3..758aad8 100644 --- a/processing/src/main/java/org/apache/druid/segment/ConstantDimensionSelector.java +++ b/processing/src/main/java/org/apache/druid/segment/ConstantDimensionSelector.java @@ -35,11 +35,11 @@ public class ConstantDimensionSelector implements SingleValueHistoricalDimension { private final String value; - public ConstantDimensionSelector(final String value) + ConstantDimensionSelector(final String value) { if (NullHandling.isNullOrEquivalent(value)) { // There's an optimized implementation for nulls that callers should use instead. - throw new IllegalArgumentException("Use NullDimensionSelector or DimensionSelectorUtils.constantSelector"); + throw new IllegalArgumentException("Use DimensionSelector.constant(null)"); } this.value = value; diff --git a/processing/src/main/java/org/apache/druid/segment/DimensionSelector.java b/processing/src/main/java/org/apache/druid/segment/DimensionSelector.java index d4ffe6c..1c9bf97 100644 --- a/processing/src/main/java/org/apache/druid/segment/DimensionSelector.java +++ b/processing/src/main/java/org/apache/druid/segment/DimensionSelector.java @@ -20,11 +20,17 @@ package org.apache.druid.segment; import com.google.common.base.Predicate; +import org.apache.druid.common.config.NullHandling; import org.apache.druid.guice.annotations.PublicApi; +import org.apache.druid.query.extraction.ExtractionFn; import org.apache.druid.query.filter.ValueMatcher; import org.apache.druid.query.monomorphicprocessing.CalledFromHotLoop; import org.apache.druid.query.monomorphicprocessing.HotLoopCallee; +import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector; import org.apache.druid.segment.data.IndexedInts; +import org.apache.druid.segment.data.ZeroIndexedInts; +import org.apache.druid.segment.filter.BooleanValueMatcher; +import org.apache.druid.segment.historical.SingleValueHistoricalDimensionSelector; import javax.annotation.Nullable; import java.util.Arrays; @@ -196,4 +202,138 @@ public interface DimensionSelector extends ColumnValueSelector<Object>, HotLoopC return Arrays.asList(strings); } } + + static DimensionSelector constant(@Nullable final String value) + { + if (NullHandling.isNullOrEquivalent(value)) { + return NullDimensionSelectorHolder.NULL_DIMENSION_SELECTOR; + } else { + return new ConstantDimensionSelector(value); + } + } + + static DimensionSelector constant(@Nullable final String value, @Nullable final ExtractionFn extractionFn) + { + if (extractionFn == null) { + return constant(value); + } else { + return constant(extractionFn.apply(value)); + } + } + + /** + * Checks if the given selector constantly returns null. This method could be used in the beginning of execution of + * some queries and making some aggregations for heuristic shortcuts. + */ + static boolean isNilSelector(final DimensionSelector selector) + { + return selector.nameLookupPossibleInAdvance() + && selector.getValueCardinality() == 1 + && selector.lookupName(0) == null; + } + + /** + * This class not a public API. It is needed solely to make NULL_DIMENSION_SELECTOR and NullDimensionSelector private + * and inaccessible even from classes in the same package. It could be removed, when Druid is updated to at least Java + * 9, that supports private members in interfaces. + */ + class NullDimensionSelectorHolder + { + private static final NullDimensionSelector NULL_DIMENSION_SELECTOR = new NullDimensionSelector(); + + /** + * This class is specially made a nested member of {@link DimensionSelector} interface, and accessible only through + * calling DimensionSelector.constant(null), so that it's impossible to mistakely use NullDimensionSelector in + * instanceof statements. {@link #isNilSelector} method should be used instead. + */ + private static class NullDimensionSelector implements SingleValueHistoricalDimensionSelector, IdLookup + { + private NullDimensionSelector() + { + // Singleton. + } + + @Override + public IndexedInts getRow() + { + return ZeroIndexedInts.instance(); + } + + @Override + public int getRowValue(int offset) + { + return 0; + } + + @Override + public IndexedInts getRow(int offset) + { + return getRow(); + } + + @Override + public ValueMatcher makeValueMatcher(@Nullable String value) + { + return BooleanValueMatcher.of(value == null); + } + + @Override + public ValueMatcher makeValueMatcher(Predicate<String> predicate) + { + return BooleanValueMatcher.of(predicate.apply(null)); + } + + @Override + public int getValueCardinality() + { + return 1; + } + + @Override + @Nullable + public String lookupName(int id) + { + assert id == 0 : "id = " + id; + return null; + } + + @Override + public boolean nameLookupPossibleInAdvance() + { + return true; + } + + @Nullable + @Override + public IdLookup idLookup() + { + return this; + } + + @Override + public int lookupId(@Nullable String name) + { + return NullHandling.isNullOrEquivalent(name) ? 0 : -1; + } + + @Nullable + @Override + public Object getObject() + { + return null; + } + + @Override + public Class classOfObject() + { + return Object.class; + } + + @Override + public void inspectRuntimeShape(RuntimeShapeInspector inspector) + { + // nothing to inspect + } + } + } } diff --git a/processing/src/main/java/org/apache/druid/segment/DimensionSelectorUtils.java b/processing/src/main/java/org/apache/druid/segment/DimensionSelectorUtils.java index a8a6ed2..224b016 100644 --- a/processing/src/main/java/org/apache/druid/segment/DimensionSelectorUtils.java +++ b/processing/src/main/java/org/apache/druid/segment/DimensionSelectorUtils.java @@ -21,9 +21,7 @@ package org.apache.druid.segment; import com.google.common.base.Predicate; import com.google.common.base.Predicates; -import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.IAE; -import org.apache.druid.query.extraction.ExtractionFn; import org.apache.druid.query.filter.ValueMatcher; import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector; import org.apache.druid.segment.data.IndexedInts; @@ -265,32 +263,4 @@ public final class DimensionSelectorUtils } return valueIds; } - - public static DimensionSelector constantSelector(@Nullable final String value) - { - if (NullHandling.isNullOrEquivalent(value)) { - return NullDimensionSelector.instance(); - } else { - return new ConstantDimensionSelector(value); - } - } - - public static DimensionSelector constantSelector( - @Nullable final String value, - @Nullable final ExtractionFn extractionFn - ) - { - if (extractionFn == null) { - return constantSelector(value); - } else { - return constantSelector(extractionFn.apply(value)); - } - } - - public static boolean isNilSelector(final DimensionSelector selector) - { - return selector.nameLookupPossibleInAdvance() - && selector.getValueCardinality() == 1 - && selector.lookupName(0) == null; - } } diff --git a/processing/src/main/java/org/apache/druid/segment/NullDimensionSelector.java b/processing/src/main/java/org/apache/druid/segment/NullDimensionSelector.java deleted file mode 100644 index 6403dce..0000000 --- a/processing/src/main/java/org/apache/druid/segment/NullDimensionSelector.java +++ /dev/null @@ -1,128 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF 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 org.apache.druid.segment; - -import com.google.common.base.Predicate; -import org.apache.druid.common.config.NullHandling; -import org.apache.druid.query.filter.ValueMatcher; -import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector; -import org.apache.druid.segment.data.IndexedInts; -import org.apache.druid.segment.data.ZeroIndexedInts; -import org.apache.druid.segment.filter.BooleanValueMatcher; -import org.apache.druid.segment.historical.SingleValueHistoricalDimensionSelector; - -import javax.annotation.Nullable; - -public class NullDimensionSelector implements SingleValueHistoricalDimensionSelector, IdLookup -{ - private static final NullDimensionSelector INSTANCE = new NullDimensionSelector(); - - private NullDimensionSelector() - { - // Singleton. - } - - public static NullDimensionSelector instance() - { - return INSTANCE; - } - - @Override - public IndexedInts getRow() - { - return ZeroIndexedInts.instance(); - } - - @Override - public int getRowValue(int offset) - { - return 0; - } - - @Override - public IndexedInts getRow(int offset) - { - return getRow(); - } - - @Override - public ValueMatcher makeValueMatcher(@Nullable String value) - { - return BooleanValueMatcher.of(value == null); - } - - @Override - public ValueMatcher makeValueMatcher(Predicate<String> predicate) - { - return BooleanValueMatcher.of(predicate.apply(null)); - } - - @Override - public int getValueCardinality() - { - return 1; - } - - @Override - @Nullable - public String lookupName(int id) - { - assert id == 0 : "id = " + id; - return null; - } - - @Override - public boolean nameLookupPossibleInAdvance() - { - return true; - } - - @Nullable - @Override - public IdLookup idLookup() - { - return this; - } - - @Override - public int lookupId(@Nullable String name) - { - return NullHandling.isNullOrEquivalent(name) ? 0 : -1; - } - - @Nullable - @Override - public Object getObject() - { - return null; - } - - @Override - public Class classOfObject() - { - return Object.class; - } - - @Override - public void inspectRuntimeShape(RuntimeShapeInspector inspector) - { - // nothing to inspect - } -} diff --git a/processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java b/processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java index 2beca4a..c0b5ee5 100644 --- a/processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java +++ b/processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java @@ -81,7 +81,7 @@ class QueryableIndexColumnSelectorFactory implements ColumnSelectorFactory final ColumnHolder columnHolder = index.getColumnHolder(dimension); if (columnHolder == null) { - return DimensionSelectorUtils.constantSelector(null, extractionFn); + return DimensionSelector.constant(null, extractionFn); } if (dimension.equals(ColumnHolder.TIME_COLUMN_NAME)) { @@ -108,7 +108,7 @@ class QueryableIndexColumnSelectorFactory implements ColumnSelectorFactory //noinspection unchecked return ((DictionaryEncodedColumn<String>) column).makeDimensionSelector(offset, extractionFn); } else { - return DimensionSelectorUtils.constantSelector(null, extractionFn); + return DimensionSelector.constant(null, extractionFn); } } diff --git a/processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndexColumnSelectorFactory.java b/processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndexColumnSelectorFactory.java index 970ccfd..e1b04c3 100644 --- a/processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndexColumnSelectorFactory.java +++ b/processing/src/main/java/org/apache/druid/segment/incremental/IncrementalIndexColumnSelectorFactory.java @@ -25,7 +25,6 @@ import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.ColumnValueSelector; import org.apache.druid.segment.DimensionIndexer; import org.apache.druid.segment.DimensionSelector; -import org.apache.druid.segment.DimensionSelectorUtils; import org.apache.druid.segment.SingleScanTimeDimensionSelector; import org.apache.druid.segment.VirtualColumns; import org.apache.druid.segment.column.ColumnCapabilities; @@ -81,7 +80,7 @@ class IncrementalIndexColumnSelectorFactory implements ColumnSelectorFactory // not a dimension, column may be a metric ColumnCapabilities capabilities = getColumnCapabilities(dimension); if (capabilities == null) { - return DimensionSelectorUtils.constantSelector(null, extractionFn); + return DimensionSelector.constant(null, extractionFn); } if (capabilities.getType().isNumeric()) { return capabilities.getType().makeNumericWrappingDimensionSelector( @@ -91,7 +90,7 @@ class IncrementalIndexColumnSelectorFactory implements ColumnSelectorFactory } // if we can't wrap the base column, just return a column of all nulls - return DimensionSelectorUtils.constantSelector(null, extractionFn); + return DimensionSelector.constant(null, extractionFn); } else { final DimensionIndexer indexer = dimensionDesc.getIndexer(); return indexer.makeDimensionSelector(dimensionSpec, rowHolder, dimensionDesc); diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java index cc1ba97..1442903 100644 --- a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java +++ b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java @@ -37,9 +37,7 @@ import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.ColumnValueSelector; import org.apache.druid.segment.ConstantExprEvalSelector; import org.apache.druid.segment.DimensionSelector; -import org.apache.druid.segment.DimensionSelectorUtils; import org.apache.druid.segment.NilColumnValueSelector; -import org.apache.druid.segment.NullDimensionSelector; import org.apache.druid.segment.column.ColumnCapabilities; import org.apache.druid.segment.column.ColumnHolder; import org.apache.druid.segment.column.ValueType; @@ -195,10 +193,10 @@ public class ExpressionSelectors if (baseSelector instanceof ConstantExprEvalSelector) { // Optimization for dimension selectors on constants. - return DimensionSelectorUtils.constantSelector(baseSelector.getObject().asString(), extractionFn); + return DimensionSelector.constant(baseSelector.getObject().asString(), extractionFn); } else if (baseSelector instanceof NilColumnValueSelector) { // Optimization for null dimension selector. - return NullDimensionSelector.instance(); + return DimensionSelector.constant(null); } else if (extractionFn == null) { class DefaultExpressionDimensionSelector extends BaseSingleValueDimensionSelector { diff --git a/processing/src/test/java/org/apache/druid/segment/ConstantDimensionSelectorTest.java b/processing/src/test/java/org/apache/druid/segment/ConstantDimensionSelectorTest.java index d397496..de305f0 100644 --- a/processing/src/test/java/org/apache/druid/segment/ConstantDimensionSelectorTest.java +++ b/processing/src/test/java/org/apache/druid/segment/ConstantDimensionSelectorTest.java @@ -28,13 +28,13 @@ import org.junit.Test; public class ConstantDimensionSelectorTest { - private final DimensionSelector NULL_SELECTOR = DimensionSelectorUtils.constantSelector(null); - private final DimensionSelector CONST_SELECTOR = DimensionSelectorUtils.constantSelector("billy"); - private final DimensionSelector NULL_EXTRACTION_SELECTOR = DimensionSelectorUtils.constantSelector( + private final DimensionSelector NULL_SELECTOR = DimensionSelector.constant(null); + private final DimensionSelector CONST_SELECTOR = DimensionSelector.constant("billy"); + private final DimensionSelector NULL_EXTRACTION_SELECTOR = DimensionSelector.constant( null, new StringFormatExtractionFn("billy") ); - private final DimensionSelector CONST_EXTRACTION_SELECTOR = DimensionSelectorUtils.constantSelector( + private final DimensionSelector CONST_EXTRACTION_SELECTOR = DimensionSelector.constant( "billybilly", new SubstringDimExtractionFn(0, 5) ); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org