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 <s...@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-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org