Skip to content

Proper null handling in equality, inequality and membership operators for all SV column data types #9173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jackie-Jiang merged 11 commits into from
Aug 6, 2022
Merged

Proper null handling in equality, inequality and membership operators for all SV column data types #9173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b21ec37
[Draft] proper null handling in operators
nizarhejazi Aug 2, 2022
432f7ae
Merge branch 'master' into null_op
nizarhejazi Aug 4, 2022
27f6271
Updating null removal in SVSScanDocIdIterator
nizarhejazi Aug 5, 2022
a01ff56
Merge branch 'master' into null_op
nizarhejazi Aug 5, 2022
fc0bff9
add more tests
nizarhejazi Aug 5, 2022
7043c20
Fix a bug in retrieving dictionary id set of the values in the given …
nizarhejazi Aug 5, 2022
10dad22
address comments
nizarhejazi Aug 5, 2022
474575f
address comments
nizarhejazi Aug 5, 2022
d006525
address one more comment
nizarhejazi Aug 5, 2022
d007c58
address one more comment
nizarhejazi Aug 5, 2022
741a790
address a comment
nizarhejazi Aug 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
259 changes: 248 additions & 11 deletions ...core/src/main/java/org/apache/pinot/core/operator/dociditerators/SVScanDocIdIterator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
*/
package org.apache.pinot.core.operator.dociditerators;

import javax.annotation.Nullable;
import org.apache.pinot.core.operator.filter.predicate.PredicateEvaluator;
import org.apache.pinot.segment.spi.Constants;
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;
Expand All @@ -48,12 +50,17 @@ public final class SVScanDocIdIterator implements ScanBasedDocIdIterator {
private int _nextDocId = 0;
private long _numEntriesScanned = 0L;

public SVScanDocIdIterator(PredicateEvaluator predicateEvaluator, ForwardIndexReader reader, int numDocs) {
public SVScanDocIdIterator(PredicateEvaluator predicateEvaluator, ForwardIndexReader reader, int numDocs,
@Nullable NullValueVectorReader nullValueReader) {
_predicateEvaluator = predicateEvaluator;
_reader = reader;
_readerContext = reader.createContext();
_numDocs = numDocs;
_valueMatcher = getValueMatcher();
ImmutableRoaringBitmap nullBitmap = nullValueReader != null ? nullValueReader.getNullBitmap() : null;
if (nullBitmap != null && nullBitmap.isEmpty()) {
nullBitmap = null;
}
_valueMatcher = getValueMatcher(nullBitmap);
}

@Override
Expand Down Expand Up @@ -122,25 +129,25 @@ public long getNumEntriesScanned() {
return _numEntriesScanned;
}

private ValueMatcher getValueMatcher() {
private ValueMatcher getValueMatcher(@Nullable ImmutableRoaringBitmap nullBitmap) {
if (_reader.isDictionaryEncoded()) {
return new DictIdMatcher();
return nullBitmap == null ? new DictIdMatcher() : new DictIdMatcherAndNullHandler(nullBitmap);
} else {
switch (_reader.getStoredType()) {
case INT:
return new IntMatcher();
return nullBitmap == null ? new IntMatcher() : new IntMatcherAndNullHandler(nullBitmap);
case LONG:
return new LongMatcher();
return nullBitmap == null ? new LongMatcher() : new LongMatcherAndNullHandler(nullBitmap);
case FLOAT:
return new FloatMatcher();
return nullBitmap == null ? new FloatMatcher() : new FloatMatcherAndNullHandler(nullBitmap);
case DOUBLE:
return new DoubleMatcher();
return nullBitmap == null ? new DoubleMatcher() : new DoubleMatcherAndNullHandler(nullBitmap);
case BIG_DECIMAL:
return new BigDecimalMatcher();
return nullBitmap == null ? new BigDecimalMatcher() : new BigDecimalMatcherAndNullHandler(nullBitmap);
case STRING:
return new StringMatcher();
return nullBitmap == null ? new StringMatcher() : new StringMatcherAndNullHandler(nullBitmap);
case BYTES:
return new BytesMatcher();
return nullBitmap == null ? new BytesMatcher() : new BytesMatcherAndNullHandler(nullBitmap);
default:
throw new UnsupportedOperationException();
}
Expand Down Expand Up @@ -172,6 +179,57 @@ default int matchValues(int limit, int[] docIds) {
}
}

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[OPTIMAL_ITERATOR_BATCH_SIZE];
Expand All @@ -188,6 +246,34 @@ public int matchValues(int limit, int[] docIds) {
}
}

private class DictIdMatcherAndNullHandler implements ValueMatcher {

private final int[] _buffer = new int[OPTIMAL_ITERATOR_BATCH_SIZE];
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[OPTIMAL_ITERATOR_BATCH_SIZE];
Expand All @@ -204,6 +290,31 @@ public int matchValues(int limit, int[] docIds) {
}
}

private class IntMatcherAndNullHandler implements ValueMatcher {

private final ImmutableRoaringBitmap _nullBitmap;
private final int[] _buffer = new int[OPTIMAL_ITERATOR_BATCH_SIZE];

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[OPTIMAL_ITERATOR_BATCH_SIZE];
Expand All @@ -220,6 +331,31 @@ public int matchValues(int limit, int[] docIds) {
}
}

private class LongMatcherAndNullHandler implements ValueMatcher {

private final ImmutableRoaringBitmap _nullBitmap;
private final long[] _buffer = new long[OPTIMAL_ITERATOR_BATCH_SIZE];

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[OPTIMAL_ITERATOR_BATCH_SIZE];
Expand All @@ -236,6 +372,31 @@ public int matchValues(int limit, int[] docIds) {
}
}

private class FloatMatcherAndNullHandler implements ValueMatcher {

private final ImmutableRoaringBitmap _nullBitmap;
private final float[] _buffer = new float[OPTIMAL_ITERATOR_BATCH_SIZE];

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[OPTIMAL_ITERATOR_BATCH_SIZE];
Expand All @@ -252,6 +413,31 @@ public int matchValues(int limit, int[] docIds) {
}
}

private class DoubleMatcherAndNullHandler implements ValueMatcher {

private final ImmutableRoaringBitmap _nullBitmap;
private final double[] _buffer = new double[OPTIMAL_ITERATOR_BATCH_SIZE];

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
Expand All @@ -260,6 +446,23 @@ public boolean doesValueMatch(int docId) {
}
}

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
Expand All @@ -268,11 +471,45 @@ public boolean doesValueMatch(int docId) {
}
}

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
public boolean doesValueMatch(int docId) {
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));
}
}
}
10 changes: 7 additions & 3 deletions pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/SVScanDocIdSet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,18 @@

import org.apache.pinot.core.operator.dociditerators.SVScanDocIdIterator;
import org.apache.pinot.core.operator.filter.predicate.PredicateEvaluator;
import org.apache.pinot.segment.spi.index.reader.ForwardIndexReader;
import org.apache.pinot.segment.spi.datasource.DataSource;
import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;


public final class SVScanDocIdSet implements FilterBlockDocIdSet {
private final SVScanDocIdIterator _docIdIterator;

public SVScanDocIdSet(PredicateEvaluator predicateEvaluator, ForwardIndexReader<?> reader, int numDocs) {
_docIdIterator = new SVScanDocIdIterator(predicateEvaluator, reader, numDocs);
public SVScanDocIdSet(PredicateEvaluator predicateEvaluator, DataSource dataSource, int numDocs,
boolean nullHandlingEnabled) {
NullValueVectorReader nullValueVector = nullHandlingEnabled ? dataSource.getNullValueVector() : null;
_docIdIterator = new SVScanDocIdIterator(
predicateEvaluator, dataSource.getForwardIndex(), numDocs, nullValueVector);
}

@Override
Expand Down
Loading