[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4350: For RANGE and REGEXP operators, if there is single matching dictionary id, use inverted index

[GitBox <gi...@apache.org>]

Jackie-Jiang commented on a change in pull request #4350: For RANGE and REGEXP operators, if there is single matching dictionary id, use inverted index
URL: https://github.com/apache/incubator-pinot/pull/4350#discussion_r296069788
 
 

 ##########
 File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/FilterOperatorUtils.java
 ##########
 @@ -53,18 +53,25 @@ public static BaseFilterOperator getLeafFilterOperator(PredicateEvaluator predic
     // TODO: make it exclusive
     int endDocId = numDocs - 1;
 
-    // Use inverted index if the predicate type is not RANGE or REGEXP_LIKE for efficiency
+    // Use scan-based operator if inverted index does not exist or the predicate type is RANGE or REGEXP_LIKE with more
+    // than 1 matching dictionary ids
+    // NOTE: allow RANGE with single matching dictionary id to use inverted index is very useful for time column with
+    //       DAYS granularity and time values across two days
+    // TODO: whether to use inverted index should be based on the number of matching dictionary ids and cardinality of
+    //       the column instead of the predicate type
+    // TODO: if column is sorted, should always use sorted index for RANGE predicate
     DataSourceMetadata dataSourceMetadata = dataSource.getDataSourceMetadata();
     Predicate.Type predicateType = predicateEvaluator.getPredicateType();
-    if (dataSourceMetadata.hasInvertedIndex() && (predicateType != Predicate.Type.RANGE) && (predicateType
-        != Predicate.Type.REGEXP_LIKE)) {
+    if (!dataSourceMetadata.hasInvertedIndex() || (
+        (predicateType == Predicate.Type.RANGE || predicateType == Predicate.Type.REGEXP_LIKE)
+            && predicateEvaluator.getNumMatchingDictIds() > 1)) {
 
 Review comment:
   Agree, but I want to keep this pr small and only solve the time column issue for now. I added TODOs and want to revisit this part later. The current way of handling IN can also backfire. The filter should be much smarter. Created #4351 to track this.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org