Jackie-Jiang commented on code in PR #10043:
URL: https://github.com/apache/pinot/pull/10043#discussion_r1062985665
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/RangeIndexBasedFilterOperator.java:
##########
@@ -131,27 +139,75 @@ public String toExplainString() {
}
interface RangeEvaluator {
+
+ Set<FieldSpec.DataType> SUPPORTED_RAW_DATA_TYPES =
EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT,
FieldSpec.DataType.DOUBLE);
+
+ static boolean canEvaluate(PredicateEvaluator predicateEvaluator,
DataSource dataSource) {
+ if (dataSource.getRangeIndex() != null) {
Review Comment:
This part can be simplified. We should be able to apply the range index when:
1. Range index exist and is exact
2. Column is dictionary encoded or has supported data types
3. Predicate is EQ or RANGE (RANGE won't hit this method though)
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/BitSlicedRangeIndexReader.java:
##########
@@ -94,6 +94,32 @@ public int getNumMatchingDocs(double min, double max) {
return queryRangeBitmapCardinality(FPOrdering.ordinalOf(min),
FPOrdering.ordinalOf(max), 0xFFFFFFFFFFFFFFFFL);
}
+ @Override
+ public int getNumMatchingDocs(int value) {
+ if (value < _min) {
+ return 0;
+ }
+ return queryRangeBitmapCardinality(Math.max(value, _min) - _min, _max -
_min);
Review Comment:
Can be simplified, same for long type
```suggestion
return queryRangeBitmapCardinality(value - _min, _max - _min);
```
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/RangeIndexBasedFilterOperator.java:
##########
@@ -131,27 +139,75 @@ public String toExplainString() {
}
interface RangeEvaluator {
+
+ Set<FieldSpec.DataType> SUPPORTED_RAW_DATA_TYPES =
EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT,
FieldSpec.DataType.DOUBLE);
+
+ static boolean canEvaluate(PredicateEvaluator predicateEvaluator,
DataSource dataSource) {
+ if (dataSource.getRangeIndex() != null) {
+ boolean datatypeSupported = dataSource.getRangeIndex().isExact()
+ && (predicateEvaluator.isDictionaryBased()
+ ||
SUPPORTED_RAW_DATA_TYPES.contains(predicateEvaluator.getDataType()));
+ switch (predicateEvaluator.getPredicateType()) {
+ case EQ:
+ return datatypeSupported && dataSource.getRangeIndex().isExact()
+ && predicateEvaluator instanceof
EqualsPredicateEvaluatorFactory.EqualsPredicateEvaluator;
+ case RANGE:
+ return datatypeSupported && (predicateEvaluator instanceof
SortedDictionaryBasedRangePredicateEvaluator
+ || predicateEvaluator instanceof
IntRawValueBasedRangePredicateEvaluator
+ || predicateEvaluator instanceof
LongRawValueBasedRangePredicateEvaluator
+ || predicateEvaluator instanceof
FloatRawValueBasedRangePredicateEvaluator
+ || predicateEvaluator instanceof
DoubleRawValueBasedRangePredicateEvaluator);
+ default:
+ }
+ }
+ return false;
+ }
+
static RangeEvaluator of(RangeIndexReader<ImmutableRoaringBitmap>
rangeIndexReader,
PredicateEvaluator predicateEvaluator) {
- if (predicateEvaluator instanceof
SortedDictionaryBasedRangePredicateEvaluator) {
- return new IntRangeEvaluator(rangeIndexReader,
- (SortedDictionaryBasedRangePredicateEvaluator) predicateEvaluator);
+ if (predicateEvaluator.isDictionaryBased()) {
+ if (predicateEvaluator instanceof
EqualsPredicateEvaluatorFactory.EqualsPredicateEvaluator) {
+ return new IntPointEvaluator(rangeIndexReader,
+ (EqualsPredicateEvaluatorFactory.EqualsPredicateEvaluator)
predicateEvaluator);
Review Comment:
Can we follow the same way as how we extract the predicate value from the
range predicate evaluator: make the child class public, and add a method
getValue() or getDictId(). I don't see much value added by this new interface,
and tend to keep the handling consistent
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/RangeIndexBasedFilterOperator.java:
##########
@@ -131,27 +139,75 @@ public String toExplainString() {
}
interface RangeEvaluator {
+
+ Set<FieldSpec.DataType> SUPPORTED_RAW_DATA_TYPES =
EnumSet.of(FieldSpec.DataType.INT,
+ FieldSpec.DataType.LONG, FieldSpec.DataType.FLOAT,
FieldSpec.DataType.DOUBLE);
+
+ static boolean canEvaluate(PredicateEvaluator predicateEvaluator,
DataSource dataSource) {
+ if (dataSource.getRangeIndex() != null) {
+ boolean datatypeSupported = dataSource.getRangeIndex().isExact()
+ && (predicateEvaluator.isDictionaryBased()
+ ||
SUPPORTED_RAW_DATA_TYPES.contains(predicateEvaluator.getDataType()));
+ switch (predicateEvaluator.getPredicateType()) {
+ case EQ:
+ return datatypeSupported && dataSource.getRangeIndex().isExact()
+ && predicateEvaluator instanceof
EqualsPredicateEvaluatorFactory.EqualsPredicateEvaluator;
+ case RANGE:
+ return datatypeSupported && (predicateEvaluator instanceof
SortedDictionaryBasedRangePredicateEvaluator
+ || predicateEvaluator instanceof
IntRawValueBasedRangePredicateEvaluator
+ || predicateEvaluator instanceof
LongRawValueBasedRangePredicateEvaluator
+ || predicateEvaluator instanceof
FloatRawValueBasedRangePredicateEvaluator
+ || predicateEvaluator instanceof
DoubleRawValueBasedRangePredicateEvaluator);
+ default:
+ }
+ }
+ return false;
+ }
+
static RangeEvaluator of(RangeIndexReader<ImmutableRoaringBitmap>
rangeIndexReader,
PredicateEvaluator predicateEvaluator) {
- if (predicateEvaluator instanceof
SortedDictionaryBasedRangePredicateEvaluator) {
- return new IntRangeEvaluator(rangeIndexReader,
- (SortedDictionaryBasedRangePredicateEvaluator) predicateEvaluator);
+ if (predicateEvaluator.isDictionaryBased()) {
Review Comment:
Suggest branching on the predicate type first for readability. Basically add
an if check on `predicateEvaluator.getPredicateType()` first, then do the switch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
