You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2018/11/02 11:38:23 UTC
[GitHub] jihoonson closed pull request #6480: Hide NullDimensionSelector
from public
jihoonson closed pull request #6480: Hide NullDimensionSelector from public
URL: https://github.com/apache/incubator-druid/pull/6480
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 8168fddf5d3..0b63c319357 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.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 Aggregator factorize(final ColumnSelectorFactory metricFactory)
// 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 BufferAggregator factorizeBuffered(final ColumnSelectorFactory metricFact
// 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 3eae4b15332..17e2f3e1802 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.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;
@@ -126,7 +125,7 @@ public void updateSearchResultSet(
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 a50afc3b1e1..758aad82824 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 @@
{
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 d4ffe6c280c..1c9bf9765f2 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 @@ default Object defaultGetObject()
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 a8a6ed2712f..224b01636e8 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 @@
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 static BitSet makePredicateMatchingSet(DimensionSelector selector, Predic
}
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 6403dceeae0..00000000000
--- 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 2beca4a0d27..c0b5ee52f4d 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 @@ private DimensionSelector makeDimensionSelectorUndecorated(DimensionSpec dimensi
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 @@ private DimensionSelector makeDimensionSelectorUndecorated(DimensionSpec dimensi
//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 970ccfd179e..e1b04c39f58 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.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 @@ private DimensionSelector makeDimensionSelectorUndecorated(DimensionSpec dimensi
// 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 @@ private DimensionSelector makeDimensionSelectorUndecorated(DimensionSpec dimensi
}
// 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 c82be3bbb90..76279700b7d 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
@@ -38,9 +38,7 @@
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 static DimensionSelector makeDimensionSelector(
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 d397496a0db..de305f0a076 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 @@
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)
);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org