You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by ji...@apache.org on 2018/11/02 11:38:28 UTC
[incubator-druid] branch master updated: Hide NullDimensionSelector
from public (#6480)
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 <le...@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-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org