Amogh Margoor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17706 )
Change subject: IMPALA-3430: Runtime filter : Extend runtime filter to support Min/Max values for HDFS scans ...................................................................... Patch Set 29: (4 comments) Looked at the FE changes and changes look quite good. Only major comment I have added are for few more test scenarios to be included. Other comment which can be addressed in separate patch is extension of this to handle (its good to have JIRAs for them): 1. non-aggregate, non-correlated scalar subqueries. 2. Runtime filters for nested loop join in general, not just for NC scalar subqueries. http://gerrit.cloudera.org:8080/#/c/17706/29/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java File fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java: http://gerrit.cloudera.org:8080/#/c/17706/29/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@84 PS29, Line 84: public boolean isSingleRange() { I think isRelationalOperator() (from PL terminology) or isComparisionOperator() is a better name. Predicates are evaluated to boolean, so they specifying range can be confusing. http://gerrit.cloudera.org:8080/#/c/17706/29/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java: http://gerrit.cloudera.org:8080/#/c/17706/29/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@392 PS29, Line 392: if (!(child1 instanceof AggregationNode) Any reason we are limiting this to just Aggregate ? It appears we can extend it to even non-aggregate scalar subqueries. http://gerrit.cloudera.org:8080/#/c/17706/29/testdata/workloads/functional-query/queries/QueryTest/overlap_min_max_filters.test File testdata/workloads/functional-query/queries/QueryTest/overlap_min_max_filters.test: http://gerrit.cloudera.org:8080/#/c/17706/29/testdata/workloads/functional-query/queries/QueryTest/overlap_min_max_filters.test@334 PS29, Line 334: ---- QUERY Can we add test for more than 1 NC scalar subqueries in a query ? http://gerrit.cloudera.org:8080/#/c/17706/29/testdata/workloads/functional-query/queries/QueryTest/overlap_min_max_filters.test@437 PS29, Line 437: # Negative tests to check out the explain output involving a non-correlated one-row Can we add negative test for: 1. non-aggregate, non-correlated scalar subquery 2. correlated scalar subquery -- To view, visit http://gerrit.cloudera.org:8080/17706 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c2bb5baad622051d1002c9c162c672d428e5446 Gerrit-Change-Number: 17706 Gerrit-PatchSet: 29 Gerrit-Owner: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Amogh Margoor <amarg...@gmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Mon, 16 Aug 2021 17:01:25 +0000 Gerrit-HasComments: Yes
