You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/07/21 09:58:35 UTC
[GitHub] [incubator-pinot] atris opened a new pull request #7184: Introduce OR Predicate Execution On Star Tree Index
atris opened a new pull request #7184:
URL: https://github.com/apache/incubator-pinot/pull/7184
This commit allows OR predicates to be executed on Star Tree indices. We only allow a single dimension to
be present in the OR predicate in order to be qualified for execution on Star Tree node. This is to ensure
that we do not double count pre aggregated documents while traversing the tree for two dimensions. A follow
up PR will be done for multi dimensional OR predicates.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #7184: Introduce OR Predicate Execution On Star Tree Index
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7184:
URL: https://github.com/apache/incubator-pinot/pull/7184#discussion_r674389500
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/startree/StarTreeUtils.java
##########
@@ -179,4 +183,51 @@ public static boolean isFitForStarTree(StarTreeV2Metadata starTreeV2Metadata,
// Check predicate columns
return starTreeDimensions.containsAll(predicateColumns);
}
+
+ /**
+ * Evaluates a given filter to ascertain if the OR clause is valid for StarTree processing.
+ *
+ * StarTree supports OR predicates on a single dimension only (d1 < 10 OR d1 > 50).
+ */
+ private static boolean isOrClauseValidForStarTree(FilterContext filterContext) {
+ assert filterContext != null;
+
+ Set<String> seenLiterals = new HashSet<>();
+
+ isOrClauseValidForStarTreeInternal(filterContext, seenLiterals);
+
+ return seenLiterals.size() == 1;
+ }
+
+ /** Internal processor for the above evaluator */
+ private static void isOrClauseValidForStarTreeInternal(FilterContext filterContext, Set<String> seenLiterals) {
+ assert filterContext != null;
+
+ if (filterContext.getType() == FilterContext.Type.OR) {
+ List<FilterContext> childFilterContexts = filterContext.getChildren();
+
+ for (FilterContext childFilterContext : childFilterContexts) {
+ isOrClauseValidForStarTreeInternal(childFilterContext, seenLiterals);
+ }
+ } else if (filterContext.getType() == FilterContext.Type.PREDICATE) {
+ String literalValue = validateExpressionAndExtractLiteral(filterContext.getPredicate());
+
+ if (literalValue != null) {
Review comment:
When the lhs is not an identifier, we should fail the check because the OR clause is not valid
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/startree/StarTreeUtils.java
##########
@@ -102,8 +102,12 @@ public static boolean isStarTreeDisabled(QueryContext queryContext) {
queue.addAll(filterNode.getChildren());
break;
case OR:
- // Star-tree does not support OR filter
- return null;
+ if (isOrClauseValidForStarTree(filter)) {
Review comment:
We cannot mix the predicates under OR with predicates under AND. One example filter would be `a < 10 AND (b < 0 OR b > 10)`, where we need to preserve the tree structure
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/startree/StarTreeUtils.java
##########
@@ -179,4 +183,51 @@ public static boolean isFitForStarTree(StarTreeV2Metadata starTreeV2Metadata,
// Check predicate columns
return starTreeDimensions.containsAll(predicateColumns);
}
+
+ /**
+ * Evaluates a given filter to ascertain if the OR clause is valid for StarTree processing.
+ *
+ * StarTree supports OR predicates on a single dimension only (d1 < 10 OR d1 > 50).
+ */
+ private static boolean isOrClauseValidForStarTree(FilterContext filterContext) {
+ assert filterContext != null;
+
+ Set<String> seenLiterals = new HashSet<>();
+
+ isOrClauseValidForStarTreeInternal(filterContext, seenLiterals);
+
+ return seenLiterals.size() == 1;
+ }
+
+ /** Internal processor for the above evaluator */
+ private static void isOrClauseValidForStarTreeInternal(FilterContext filterContext, Set<String> seenLiterals) {
+ assert filterContext != null;
+
+ if (filterContext.getType() == FilterContext.Type.OR) {
+ List<FilterContext> childFilterContexts = filterContext.getChildren();
+
+ for (FilterContext childFilterContext : childFilterContexts) {
+ isOrClauseValidForStarTreeInternal(childFilterContext, seenLiterals);
+ }
+ } else if (filterContext.getType() == FilterContext.Type.PREDICATE) {
+ String literalValue = validateExpressionAndExtractLiteral(filterContext.getPredicate());
+
+ if (literalValue != null) {
+ seenLiterals.add(literalValue);
+ }
+ }
+ }
+
+ /** Checks if the given predicate has an expression which is of type identifier and returns its literal */
+ private static String validateExpressionAndExtractLiteral(Predicate predicate) {
Review comment:
The method name is confusing. Here we want to extract the identifier name, instead of literal.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] atris commented on pull request #7184: Introduce OR Predicate Execution On Star Tree Index
Posted by GitBox <gi...@apache.org>.
atris commented on pull request #7184:
URL: https://github.com/apache/incubator-pinot/pull/7184#issuecomment-885721925
The integration test is failing due to a Kafka issue -- unrelated IMO to this change
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org