richardstartin commented on code in PR #10043:
URL: https://github.com/apache/pinot/pull/10043#discussion_r1063374698
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/RangeIndexBasedFilterOperator.java:
##########
@@ -124,38 +137,102 @@ public List<Operator> getChildOperators() {
@Override
public String toExplainString() {
- return EXPLAIN_NAME + "(indexLookUp:range_index"
- + ",operator:" + _rangePredicateEvaluator.getPredicateType()
- + ",predicate:" + _rangePredicateEvaluator.getPredicate().toString()
- + ')';
+ return EXPLAIN_NAME + "(indexLookUp:range_index" + ",operator:" +
_rangePredicateEvaluator.getPredicateType()
+ + ",predicate:" + _rangePredicateEvaluator.getPredicate().toString() +
')';
}
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 = (predicateEvaluator.isDictionaryBased() ||
SUPPORTED_RAW_DATA_TYPES.contains(
+ predicateEvaluator.getDataType()));
+ switch (predicateEvaluator.getPredicateType()) {
+ case EQ:
+ return datatypeSupported && dataSource.getRangeIndex().isExact()
&& dataSource.getRangeIndex().isExact()
+ && (predicateEvaluator instanceof
DictionaryBasedEqPredicateEvaluator
+ || predicateEvaluator instanceof
IntRawValueBasedEqPredicateEvaluator
+ || predicateEvaluator instanceof
LongRawValueBasedEqPredicateEvaluator
+ || predicateEvaluator instanceof
FloatRawValueBasedEqPredicateEvaluator
+ || predicateEvaluator instanceof
DoubleRawValueBasedEqPredicateEvaluator);
+ case RANGE:
+ return datatypeSupported && (predicateEvaluator instanceof
SortedDictionaryBasedRangePredicateEvaluator
+ || predicateEvaluator instanceof
IntRawValueBasedRangePredicateEvaluator
+ || predicateEvaluator instanceof
LongRawValueBasedRangePredicateEvaluator
+ || predicateEvaluator instanceof
FloatRawValueBasedRangePredicateEvaluator
+ || predicateEvaluator instanceof
DoubleRawValueBasedRangePredicateEvaluator);
Review Comment:
To me, this level of knowledge of `PredicateEvaluator` implementation
details is kind of unacceptable (I actually had to refactor some of the classes
to make the class names short enough for ergonomic use within this class) and I
actually just need to know "can this thing give me an int for equality queries
and a pair of ints for range queries?"
I have a proposal in mind to make this mapping more loosely defined, so that
we don't get this level of coupling, and potentially the mapping could even be
soft coded, which would allow some users to just opt out of the change (use the
range index to solve equality queries) or even to reprioritise it, so I'd like
to accept the state of things for now and come back to the problem in general
later.
--
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]
