Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16152 )
Change subject: IMPALA-9924: handle single subquery in or predicate ...................................................................... Patch Set 12: (5 comments) http://gerrit.cloudera.org:8080/#/c/16152/12/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java File fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java: http://gerrit.cloudera.org:8080/#/c/16152/12/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@620 PS12, Line 620: // TODO: IMPALA-9932: we need to recurse over 'expr' and systematically pull out Is this referencing the right JIRA #? should this be 9945 ? http://gerrit.cloudera.org:8080/#/c/16152/12/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@627 PS12, Line 627: parent instanceof BinaryPredicate || parent instanceof LikePredicate, parent); For LIKE predicates, is there limitations in terms of wildcard usage or combinations with other string functions such as concat() ? (since those are most commonly associated with Like preds). http://gerrit.cloudera.org:8080/#/c/16152/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java: http://gerrit.cloudera.org:8080/#/c/16152/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java@892 PS12, Line 892: // OR predicates nested inside a different different expr types in select list nit: extra 'different' http://gerrit.cloudera.org:8080/#/c/16152/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java@897 PS12, Line 897: // IsNullPredicate rejects nested subqueries. For these unsupported cases of nested subqueries , is there an umbrella JIRA that should be mentioned here ? Also, just to be clear, these limitations already exist for ANDed predicates also, not just for OR predicates (I replaced the or with and and it failed with the same error). I think it might be slightly confusing given the comment is mainly about OR and all the tests are focused on that. http://gerrit.cloudera.org:8080/#/c/16152/12/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test File testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test: http://gerrit.cloudera.org:8080/#/c/16152/12/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test@4177 PS12, Line 4177: | other predicates: CASE WHEN id % 2 = 0 THEN id IS NOT NULL END OR id = 2 nit: here the first and last id refer to alltypes table while the middle id is coming from alltypesagg table..it is a little confusing to read at first glance. I think it is fine to leave it as-is. In future tests, perhaps you could choose a different column in the subquery. -- To view, visit http://gerrit.cloudera.org:8080/16152 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I64588992901afd7cd885419a0b7f949b0b174976 Gerrit-Change-Number: 16152 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: David Rorke <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Shant Hovsepian <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 10 Jul 2020 19:42:25 +0000 Gerrit-HasComments: Yes
