You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2023/07/28 07:31:13 UTC
[pinot] branch master updated: Delete filtering NULL support dead code paths. (#11198)
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 00502337f6 Delete filtering NULL support dead code paths. (#11198)
00502337f6 is described below
commit 00502337f667933ff7832e2a4ed4635b132d3a52
Author: Shen Yu <sh...@startree.ai>
AuthorDate: Fri Jul 28 00:31:06 2023 -0700
Delete filtering NULL support dead code paths. (#11198)
---
.../dociditerators/SVScanDocIdIterator.java | 269 +--------------------
.../core/operator/docidsets/SVScanDocIdSet.java | 7 +-
.../operator/filter/ScanBasedFilterOperator.java | 2 +-
.../pinot/perf/BenchmarkScanDocIdIterators.java | 2 +-
4 files changed, 17 insertions(+), 263 deletions(-)
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/SVScanDocIdIterator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/SVScanDocIdIterator.java
index baf07c16aa..2050347ae6 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/SVScanDocIdIterator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/SVScanDocIdIterator.java
@@ -19,17 +19,14 @@
package org.apache.pinot.core.operator.dociditerators;
import java.util.OptionalInt;
-import javax.annotation.Nullable;
import org.apache.pinot.core.common.BlockDocIdIterator;
import org.apache.pinot.core.operator.filter.predicate.PredicateEvaluator;
import org.apache.pinot.segment.spi.Constants;
import org.apache.pinot.segment.spi.datasource.DataSource;
import org.apache.pinot.segment.spi.index.reader.ForwardIndexReader;
import org.apache.pinot.segment.spi.index.reader.ForwardIndexReaderContext;
-import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
import org.roaringbitmap.BatchIterator;
import org.roaringbitmap.RoaringBitmapWriter;
-import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
import org.roaringbitmap.buffer.MutableRoaringBitmap;
@@ -54,34 +51,24 @@ public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
private int _nextDocId = 0;
private long _numEntriesScanned = 0L;
- public SVScanDocIdIterator(PredicateEvaluator predicateEvaluator, DataSource dataSource, int numDocs,
- @Nullable NullValueVectorReader nullValueReader, int batchSize) {
+ public SVScanDocIdIterator(PredicateEvaluator predicateEvaluator, DataSource dataSource, int numDocs, int batchSize) {
_batch = new int[batchSize];
_predicateEvaluator = predicateEvaluator;
_reader = dataSource.getForwardIndex();
_readerContext = _reader.createContext();
_numDocs = numDocs;
- ImmutableRoaringBitmap nullBitmap = nullValueReader != null ? nullValueReader.getNullBitmap() : null;
- if (nullBitmap != null && nullBitmap.isEmpty()) {
- nullBitmap = null;
- }
- _valueMatcher = getValueMatcher(nullBitmap);
+ _valueMatcher = getValueMatcher();
_cardinality = dataSource.getDataSourceMetadata().getCardinality();
}
// for testing
- public SVScanDocIdIterator(PredicateEvaluator predicateEvaluator, ForwardIndexReader reader, int numDocs,
- @Nullable NullValueVectorReader nullValueReader) {
+ public SVScanDocIdIterator(PredicateEvaluator predicateEvaluator, ForwardIndexReader reader, int numDocs) {
_batch = new int[BlockDocIdIterator.OPTIMAL_ITERATOR_BATCH_SIZE];
_predicateEvaluator = predicateEvaluator;
_reader = reader;
_readerContext = reader.createContext();
_numDocs = numDocs;
- ImmutableRoaringBitmap nullBitmap = nullValueReader != null ? nullValueReader.getNullBitmap() : null;
- if (nullBitmap != null && nullBitmap.isEmpty()) {
- nullBitmap = null;
- }
- _valueMatcher = getValueMatcher(nullBitmap);
+ _valueMatcher = getValueMatcher();
_cardinality = -1;
}
@@ -173,25 +160,25 @@ public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
return ((float) _cardinality) / numMatchingItems;
}
- private ValueMatcher getValueMatcher(@Nullable ImmutableRoaringBitmap nullBitmap) {
+ private ValueMatcher getValueMatcher() {
if (_reader.isDictionaryEncoded()) {
- return nullBitmap == null ? new DictIdMatcher() : new DictIdMatcherAndNullHandler(nullBitmap);
+ return new DictIdMatcher();
} else {
switch (_reader.getStoredType()) {
case INT:
- return nullBitmap == null ? new IntMatcher() : new IntMatcherAndNullHandler(nullBitmap);
+ return new IntMatcher();
case LONG:
- return nullBitmap == null ? new LongMatcher() : new LongMatcherAndNullHandler(nullBitmap);
+ return new LongMatcher();
case FLOAT:
- return nullBitmap == null ? new FloatMatcher() : new FloatMatcherAndNullHandler(nullBitmap);
+ return new FloatMatcher();
case DOUBLE:
- return nullBitmap == null ? new DoubleMatcher() : new DoubleMatcherAndNullHandler(nullBitmap);
+ return new DoubleMatcher();
case BIG_DECIMAL:
- return nullBitmap == null ? new BigDecimalMatcher() : new BigDecimalMatcherAndNullHandler(nullBitmap);
+ return new BigDecimalMatcher();
case STRING:
- return nullBitmap == null ? new StringMatcher() : new StringMatcherAndNullHandler(nullBitmap);
+ return new StringMatcher();
case BYTES:
- return nullBitmap == null ? new BytesMatcher() : new BytesMatcherAndNullHandler(nullBitmap);
+ return new BytesMatcher();
default:
throw new UnsupportedOperationException();
}
@@ -223,57 +210,6 @@ public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
}
}
- private static class MatcherUtils {
- public static int removeNullDocs(int[] docIds, int[] values, int limit, ImmutableRoaringBitmap nullBitmap) {
- assert !nullBitmap.isEmpty();
- int copyToIdx = 0;
- for (int i = 0; i < limit; i++) {
- if (!nullBitmap.contains(docIds[i])) {
- // Compact non-null entries into the prefix of the docIds and values arrays.
- docIds[copyToIdx] = docIds[i];
- values[copyToIdx++] = values[i];
- }
- }
- return copyToIdx;
- }
-
- public static int removeNullDocs(int[] docIds, long[] values, int limit, ImmutableRoaringBitmap nullBitmap) {
- assert !nullBitmap.isEmpty();
- int copyToIdx = 0;
- for (int i = 0; i < limit; i++) {
- if (!nullBitmap.contains(docIds[i])) {
- docIds[copyToIdx] = docIds[i];
- values[copyToIdx++] = values[i];
- }
- }
- return copyToIdx;
- }
-
- public static int removeNullDocs(int[] docIds, float[] values, int limit, ImmutableRoaringBitmap nullBitmap) {
- assert !nullBitmap.isEmpty();
- int copyToIdx = 0;
- for (int i = 0; i < limit; i++) {
- if (!nullBitmap.contains(docIds[i])) {
- docIds[copyToIdx] = docIds[i];
- values[copyToIdx++] = values[i];
- }
- }
- return copyToIdx;
- }
-
- public static int removeNullDocs(int[] docIds, double[] values, int limit, ImmutableRoaringBitmap nullBitmap) {
- assert !nullBitmap.isEmpty();
- int copyToIdx = 0;
- for (int i = 0; i < limit; i++) {
- if (!nullBitmap.contains(docIds[i])) {
- docIds[copyToIdx] = docIds[i];
- values[copyToIdx++] = values[i];
- }
- }
- return copyToIdx;
- }
- }
-
private class DictIdMatcher implements ValueMatcher {
private final int[] _buffer = new int[_batch.length];
@@ -290,34 +226,6 @@ public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
}
}
- private class DictIdMatcherAndNullHandler implements ValueMatcher {
-
- private final int[] _buffer = new int[_batch.length];
- private final ImmutableRoaringBitmap _nullBitmap;
-
- public DictIdMatcherAndNullHandler(ImmutableRoaringBitmap nullBitmap) {
- _nullBitmap = nullBitmap;
- }
-
- @Override
- public boolean doesValueMatch(int docId) {
- // Any comparison (equality, inequality, or membership) with null results in false (similar to Presto) even if
- // the compared with value is null, and comparison is equality.
- // To consider nulls, use: IS NULL, or IS NOT NULL operators.
- if (_nullBitmap.contains(docId)) {
- return false;
- }
- return _predicateEvaluator.applySV(_reader.getDictId(docId, _readerContext));
- }
-
- @Override
- public int matchValues(int limit, int[] docIds) {
- _reader.readDictIds(docIds, limit, _buffer, _readerContext);
- int newLimit = MatcherUtils.removeNullDocs(docIds, _buffer, limit, _nullBitmap);
- return _predicateEvaluator.applySV(newLimit, docIds, _buffer);
- }
- }
-
private class IntMatcher implements ValueMatcher {
private final int[] _buffer = new int[_batch.length];
@@ -334,31 +242,6 @@ public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
}
}
- private class IntMatcherAndNullHandler implements ValueMatcher {
-
- private final ImmutableRoaringBitmap _nullBitmap;
- private final int[] _buffer = new int[_batch.length];
-
- public IntMatcherAndNullHandler(ImmutableRoaringBitmap nullBitmap) {
- _nullBitmap = nullBitmap;
- }
-
- @Override
- public boolean doesValueMatch(int docId) {
- if (_nullBitmap.contains(docId)) {
- return false;
- }
- return _predicateEvaluator.applySV(_reader.getInt(docId, _readerContext));
- }
-
- @Override
- public int matchValues(int limit, int[] docIds) {
- _reader.readValuesSV(docIds, limit, _buffer, _readerContext);
- int newLimit = MatcherUtils.removeNullDocs(docIds, _buffer, limit, _nullBitmap);
- return _predicateEvaluator.applySV(newLimit, docIds, _buffer);
- }
- }
-
private class LongMatcher implements ValueMatcher {
private final long[] _buffer = new long[_batch.length];
@@ -375,31 +258,6 @@ public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
}
}
- private class LongMatcherAndNullHandler implements ValueMatcher {
-
- private final ImmutableRoaringBitmap _nullBitmap;
- private final long[] _buffer = new long[_batch.length];
-
- public LongMatcherAndNullHandler(ImmutableRoaringBitmap nullBitmap) {
- _nullBitmap = nullBitmap;
- }
-
- @Override
- public boolean doesValueMatch(int docId) {
- if (_nullBitmap.contains(docId)) {
- return false;
- }
- return _predicateEvaluator.applySV(_reader.getLong(docId, _readerContext));
- }
-
- @Override
- public int matchValues(int limit, int[] docIds) {
- _reader.readValuesSV(docIds, limit, _buffer, _readerContext);
- int newLimit = MatcherUtils.removeNullDocs(docIds, _buffer, limit, _nullBitmap);
- return _predicateEvaluator.applySV(newLimit, docIds, _buffer);
- }
- }
-
private class FloatMatcher implements ValueMatcher {
private final float[] _buffer = new float[_batch.length];
@@ -416,31 +274,6 @@ public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
}
}
- private class FloatMatcherAndNullHandler implements ValueMatcher {
-
- private final ImmutableRoaringBitmap _nullBitmap;
- private final float[] _buffer = new float[_batch.length];
-
- public FloatMatcherAndNullHandler(ImmutableRoaringBitmap nullBitmap) {
- _nullBitmap = nullBitmap;
- }
-
- @Override
- public boolean doesValueMatch(int docId) {
- if (_nullBitmap.contains(docId)) {
- return false;
- }
- return _predicateEvaluator.applySV(_reader.getFloat(docId, _readerContext));
- }
-
- @Override
- public int matchValues(int limit, int[] docIds) {
- _reader.readValuesSV(docIds, limit, _buffer, _readerContext);
- int newLimit = MatcherUtils.removeNullDocs(docIds, _buffer, limit, _nullBitmap);
- return _predicateEvaluator.applySV(newLimit, docIds, _buffer);
- }
- }
-
private class DoubleMatcher implements ValueMatcher {
private final double[] _buffer = new double[_batch.length];
@@ -457,31 +290,6 @@ public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
}
}
- private class DoubleMatcherAndNullHandler implements ValueMatcher {
-
- private final ImmutableRoaringBitmap _nullBitmap;
- private final double[] _buffer = new double[_batch.length];
-
- public DoubleMatcherAndNullHandler(ImmutableRoaringBitmap nullBitmap) {
- _nullBitmap = nullBitmap;
- }
-
- @Override
- public boolean doesValueMatch(int docId) {
- if (_nullBitmap.contains(docId)) {
- return false;
- }
- return _predicateEvaluator.applySV(_reader.getDouble(docId, _readerContext));
- }
-
- @Override
- public int matchValues(int limit, int[] docIds) {
- _reader.readValuesSV(docIds, limit, _buffer, _readerContext);
- int newLimit = MatcherUtils.removeNullDocs(docIds, _buffer, limit, _nullBitmap);
- return _predicateEvaluator.applySV(newLimit, docIds, _buffer);
- }
- }
-
private class BigDecimalMatcher implements ValueMatcher {
@Override
@@ -490,23 +298,6 @@ public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
}
}
- private class BigDecimalMatcherAndNullHandler implements ValueMatcher {
-
- private final ImmutableRoaringBitmap _nullBitmap;
-
- public BigDecimalMatcherAndNullHandler(ImmutableRoaringBitmap nullBitmap) {
- _nullBitmap = nullBitmap;
- }
-
- @Override
- public boolean doesValueMatch(int docId) {
- if (_nullBitmap.contains(docId)) {
- return false;
- }
- return _predicateEvaluator.applySV(_reader.getBigDecimal(docId, _readerContext));
- }
- }
-
private class StringMatcher implements ValueMatcher {
@Override
@@ -515,23 +306,6 @@ public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
}
}
- private class StringMatcherAndNullHandler implements ValueMatcher {
-
- private final ImmutableRoaringBitmap _nullBitmap;
-
- public StringMatcherAndNullHandler(ImmutableRoaringBitmap nullBitmap) {
- _nullBitmap = nullBitmap;
- }
-
- @Override
- public boolean doesValueMatch(int docId) {
- if (_nullBitmap.contains(docId)) {
- return false;
- }
- return _predicateEvaluator.applySV(_reader.getString(docId, _readerContext));
- }
- }
-
private class BytesMatcher implements ValueMatcher {
@Override
@@ -539,21 +313,4 @@ public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
return _predicateEvaluator.applySV(_reader.getBytes(docId, _readerContext));
}
}
-
- private class BytesMatcherAndNullHandler implements ValueMatcher {
-
- private final ImmutableRoaringBitmap _nullBitmap;
-
- public BytesMatcherAndNullHandler(ImmutableRoaringBitmap nullBitmap) {
- _nullBitmap = nullBitmap;
- }
-
- @Override
- public boolean doesValueMatch(int docId) {
- if (_nullBitmap.contains(docId)) {
- return false;
- }
- return _predicateEvaluator.applySV(_reader.getBytes(docId, _readerContext));
- }
- }
}
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/SVScanDocIdSet.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/SVScanDocIdSet.java
index 40fc00a621..2e0e5b8810 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/SVScanDocIdSet.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/SVScanDocIdSet.java
@@ -22,16 +22,13 @@ import org.apache.pinot.core.common.BlockDocIdSet;
import org.apache.pinot.core.operator.dociditerators.SVScanDocIdIterator;
import org.apache.pinot.core.operator.filter.predicate.PredicateEvaluator;
import org.apache.pinot.segment.spi.datasource.DataSource;
-import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
public final class SVScanDocIdSet implements BlockDocIdSet {
private final SVScanDocIdIterator _docIdIterator;
- public SVScanDocIdSet(PredicateEvaluator predicateEvaluator, DataSource dataSource, int numDocs,
- boolean nullHandlingEnabled, int batchSize) {
- NullValueVectorReader nullValueVector = nullHandlingEnabled ? dataSource.getNullValueVector() : null;
- _docIdIterator = new SVScanDocIdIterator(predicateEvaluator, dataSource, numDocs, nullValueVector, batchSize);
+ public SVScanDocIdSet(PredicateEvaluator predicateEvaluator, DataSource dataSource, int numDocs, int batchSize) {
+ _docIdIterator = new SVScanDocIdIterator(predicateEvaluator, dataSource, numDocs, batchSize);
}
@Override
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/ScanBasedFilterOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/ScanBasedFilterOperator.java
index 34262cd417..938e304256 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/ScanBasedFilterOperator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/ScanBasedFilterOperator.java
@@ -57,7 +57,7 @@ public class ScanBasedFilterOperator extends BaseColumnFilterOperator {
protected BlockDocIdSet getNextBlockWithoutNullHandling() {
DataSourceMetadata dataSourceMetadata = _dataSource.getDataSourceMetadata();
if (dataSourceMetadata.isSingleValue()) {
- return new SVScanDocIdSet(_predicateEvaluator, _dataSource, _numDocs, false, _batchSize);
+ return new SVScanDocIdSet(_predicateEvaluator, _dataSource, _numDocs, _batchSize);
} else {
return new MVScanDocIdSet(_predicateEvaluator, _dataSource, _numDocs);
}
diff --git a/pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkScanDocIdIterators.java b/pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkScanDocIdIterators.java
index 3e061c0d2d..718d99e9ba 100644
--- a/pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkScanDocIdIterators.java
+++ b/pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkScanDocIdIterators.java
@@ -123,7 +123,7 @@ public class BenchmarkScanDocIdIterators {
@Benchmark
public MutableRoaringBitmap benchmarkSVLong() {
- return new SVScanDocIdIterator(_predicateEvaluator, _readerV2, _numDocs, null).applyAnd(_bitmap);
+ return new SVScanDocIdIterator(_predicateEvaluator, _readerV2, _numDocs).applyAnd(_bitmap);
}
public static class DummyPredicateEvaluator implements PredicateEvaluator {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org