You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2023/11/15 18:46:05 UTC

(pinot) branch master updated: Fix the misuse of star-tree when all predicates are always false under OR (#12003)

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 6d1b0609c5 Fix the misuse of star-tree when all predicates are always false under OR (#12003)
6d1b0609c5 is described below

commit 6d1b0609c508c254d808fe39335d6449e4744bc5
Author: Xiaotian (Jackie) Jiang <17...@users.noreply.github.com>
AuthorDate: Wed Nov 15 10:45:59 2023 -0800

    Fix the misuse of star-tree when all predicates are always false under OR (#12003)
---
 .../pinot/core/plan/AggregationPlanNode.java       | 47 +++++++++++-----------
 .../apache/pinot/core/plan/GroupByPlanNode.java    |  4 +-
 .../apache/pinot/core/startree/StarTreeUtils.java  |  9 +++--
 .../pinot/core/startree/v2/BaseStarTreeV2Test.java |  5 +++
 4 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java b/pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java
index 95a61c10be..2cf067864e 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java
@@ -101,12 +101,12 @@ public class AggregationPlanNode implements PlanNode {
     FilterPlanNode filterPlanNode = new FilterPlanNode(_indexSegment, _queryContext);
     BaseFilterOperator filterOperator = filterPlanNode.run();
 
-    if (canOptimizeFilteredCount(filterOperator, aggregationFunctions) && !_queryContext.isNullHandlingEnabled()) {
-      return new FastFilteredCountOperator(_queryContext, filterOperator, _indexSegment.getSegmentMetadata());
-    }
+    if (!_queryContext.isNullHandlingEnabled()) {
+      if (canOptimizeFilteredCount(filterOperator, aggregationFunctions)) {
+        return new FastFilteredCountOperator(_queryContext, filterOperator, _indexSegment.getSegmentMetadata());
+      }
 
-    if (filterOperator.isResultMatchingAll() && !_queryContext.isNullHandlingEnabled()) {
-      if (isFitForNonScanBasedPlan(aggregationFunctions, _indexSegment)) {
+      if (filterOperator.isResultMatchingAll() && isFitForNonScanBasedPlan(aggregationFunctions, _indexSegment)) {
         DataSource[] dataSources = new DataSource[aggregationFunctions.length];
         for (int i = 0; i < aggregationFunctions.length; i++) {
           List<?> inputExpressions = aggregationFunctions[i].getInputExpressions();
@@ -117,31 +117,32 @@ public class AggregationPlanNode implements PlanNode {
         }
         return new NonScanBasedAggregationOperator(_queryContext, dataSources, numTotalDocs);
       }
-    }
 
-    // Use star-tree to solve the query if possible
-    List<StarTreeV2> starTrees = _indexSegment.getStarTrees();
-    if (starTrees != null && !_queryContext.isSkipStarTree() && !_queryContext.isNullHandlingEnabled()) {
-      AggregationFunctionColumnPair[] aggregationFunctionColumnPairs =
-          StarTreeUtils.extractAggregationFunctionPairs(aggregationFunctions);
-      if (aggregationFunctionColumnPairs != null) {
-        Map<String, List<CompositePredicateEvaluator>> predicateEvaluatorsMap =
-            StarTreeUtils.extractPredicateEvaluatorsMap(_indexSegment, _queryContext.getFilter(),
-                filterPlanNode.getPredicateEvaluators());
-        if (predicateEvaluatorsMap != null) {
-          for (StarTreeV2 starTreeV2 : starTrees) {
-            if (StarTreeUtils.isFitForStarTree(starTreeV2.getMetadata(), aggregationFunctionColumnPairs, null,
-                predicateEvaluatorsMap.keySet())) {
-              BaseProjectOperator<?> projectOperator =
-                  new StarTreeProjectPlanNode(_queryContext, starTreeV2, aggregationFunctionColumnPairs, null,
-                      predicateEvaluatorsMap).run();
-              return new AggregationOperator(_queryContext, projectOperator, numTotalDocs, true);
+      // Use star-tree to solve the query if possible
+      List<StarTreeV2> starTrees = _indexSegment.getStarTrees();
+      if (!filterOperator.isResultEmpty() && starTrees != null && !_queryContext.isSkipStarTree()) {
+        AggregationFunctionColumnPair[] aggregationFunctionColumnPairs =
+            StarTreeUtils.extractAggregationFunctionPairs(aggregationFunctions);
+        if (aggregationFunctionColumnPairs != null) {
+          Map<String, List<CompositePredicateEvaluator>> predicateEvaluatorsMap =
+              StarTreeUtils.extractPredicateEvaluatorsMap(_indexSegment, _queryContext.getFilter(),
+                  filterPlanNode.getPredicateEvaluators());
+          if (predicateEvaluatorsMap != null) {
+            for (StarTreeV2 starTreeV2 : starTrees) {
+              if (StarTreeUtils.isFitForStarTree(starTreeV2.getMetadata(), aggregationFunctionColumnPairs, null,
+                  predicateEvaluatorsMap.keySet())) {
+                BaseProjectOperator<?> projectOperator =
+                    new StarTreeProjectPlanNode(_queryContext, starTreeV2, aggregationFunctionColumnPairs, null,
+                        predicateEvaluatorsMap).run();
+                return new AggregationOperator(_queryContext, projectOperator, numTotalDocs, true);
+              }
             }
           }
         }
       }
     }
 
+    // TODO: Do not create ProjectOperator when filter result is empty
     Set<ExpressionContext> expressionsToTransform =
         AggregationFunctionUtils.collectExpressionsToTransform(aggregationFunctions, null);
     BaseProjectOperator<?> projectOperator =
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/plan/GroupByPlanNode.java b/pinot-core/src/main/java/org/apache/pinot/core/plan/GroupByPlanNode.java
index 4348dbda13..d5324e073e 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/plan/GroupByPlanNode.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/plan/GroupByPlanNode.java
@@ -78,7 +78,8 @@ public class GroupByPlanNode implements PlanNode {
 
     // Use star-tree to solve the query if possible
     List<StarTreeV2> starTrees = _indexSegment.getStarTrees();
-    if (starTrees != null && !_queryContext.isSkipStarTree()) {
+    if (!_queryContext.isNullHandlingEnabled() && !filterOperator.isResultEmpty() && starTrees != null
+        && !_queryContext.isSkipStarTree()) {
       AggregationFunctionColumnPair[] aggregationFunctionColumnPairs =
           StarTreeUtils.extractAggregationFunctionPairs(aggregationFunctions);
       if (aggregationFunctionColumnPairs != null) {
@@ -99,6 +100,7 @@ public class GroupByPlanNode implements PlanNode {
       }
     }
 
+    // TODO: Do not create ProjectOperator when filter result is empty
     Set<ExpressionContext> expressionsToTransform =
         AggregationFunctionUtils.collectExpressionsToTransform(aggregationFunctions, groupByExpressionsList);
     BaseProjectOperator<?> projectOperator =
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/startree/StarTreeUtils.java b/pinot-core/src/main/java/org/apache/pinot/core/startree/StarTreeUtils.java
index 5892e6312b..c8de00208f 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/startree/StarTreeUtils.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/startree/StarTreeUtils.java
@@ -117,8 +117,8 @@ public class StarTreeUtils {
           Predicate predicate = filterNode.getPredicate();
           PredicateEvaluator predicateEvaluator = getPredicateEvaluator(indexSegment, predicate,
               predicateEvaluatorMapping);
-          if (predicateEvaluator == null) {
-            // The predicate cannot be solved with star-tree
+          // Do not use star-tree when the predicate cannot be solved with star-tree or is always false
+          if (predicateEvaluator == null || predicateEvaluator.isAlwaysFalse()) {
             return null;
           }
           if (!predicateEvaluator.isAlwaysTrue()) {
@@ -210,6 +210,10 @@ public class StarTreeUtils {
         predicateEvaluators.add(predicateEvaluator);
       }
     }
+    // When all predicates are always false, do not use star-tree
+    if (predicateEvaluators.isEmpty()) {
+      return null;
+    }
     return Pair.of(identifier, predicateEvaluators);
   }
 
@@ -230,7 +234,6 @@ public class StarTreeUtils {
           if (!extractOrClausePredicates(child, predicates)) {
             return false;
           }
-          predicates.add(child.getPredicate());
           break;
         case PREDICATE:
           predicates.add(child.getPredicate());
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/BaseStarTreeV2Test.java b/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/BaseStarTreeV2Test.java
index 8b650d08ee..3787363d17 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/BaseStarTreeV2Test.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/startree/v2/BaseStarTreeV2Test.java
@@ -111,6 +111,9 @@ abstract class BaseStarTreeV2Test<R, A> {
   private static final String QUERY_FILTER_OR_MULTIPLE_DIMENSIONS = " WHERE d1 > 10 OR d2 < 50";
   private static final String QUERY_FILTER_OR_ON_AND = " WHERE (d1 > 10 AND d1 < 50) OR d1 < 50";
   private static final String QUERY_FILTER_OR_ON_NOT = " WHERE (NOT d1 > 10) OR d1 < 50";
+  // Always false filters
+  private static final String QUERY_FILTER_ALWAYS_FALSE = " WHERE d1 > 100";
+  private static final String QUERY_FILTER_OR_ALWAYS_FALSE = " WHERE d1 > 100 OR d1 < 0";
 
   private static final String QUERY_GROUP_BY = " GROUP BY d2";
   private static final String FILTER_AGG_CLAUSE = " FILTER(WHERE d1 > 10)";
@@ -188,6 +191,8 @@ abstract class BaseStarTreeV2Test<R, A> {
     testUnsupportedFilter(query + QUERY_FILTER_OR_MULTIPLE_DIMENSIONS);
     testUnsupportedFilter(query + QUERY_FILTER_OR_ON_AND);
     testUnsupportedFilter(query + QUERY_FILTER_OR_ON_NOT);
+    testUnsupportedFilter(query + QUERY_FILTER_ALWAYS_FALSE);
+    testUnsupportedFilter(query + QUERY_FILTER_OR_ALWAYS_FALSE);
   }
 
   @Test


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