Xianqing He has posted comments on this change. ( http://gerrit.cloudera.org:8080/16266 )
Change subject: IMPALA-5022 part 1/2: Outer join simplification ...................................................................... Patch Set 14: (12 comments) http://gerrit.cloudera.org:8080/#/c/16266/12/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/16266/12/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3483 PS12, Line 3483: : * As a general rule, an outer join can be converted to an inner join if there is a : * condition on the null-filling table that filte > Can this IF block to be invoked only when isSimplified is false? For the sql like A full join B on A.id = B.id join C on B.id = C.id where A.name = 'name1' Fist step, we transform A full join B to A left join B by predicate A.name = 'name1' and isSimlified is true. Second step, we transform A left join B to A join B by the inner join on clause B.id = C.id If IF block to be invoked only when isSimplified is false, we can't do the second step http://gerrit.cloudera.org:8080/#/c/16266/13/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/16266/13/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3231 PS13, Line 3231: > Please add a comment for the new method. Done http://gerrit.cloudera.org:8080/#/c/16266/13/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3249 PS13, Line 3249: > It probably is better to call this method collectTupleIdsForConjuncts(Expr Done http://gerrit.cloudera.org:8080/#/c/16266/13/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3260 PS13, Line 3260: HashSet<TupleId>( > Please refer to the comment for line 3268, which may discard this method an This method also uses in simplifyOuterJoins http://gerrit.cloudera.org:8080/#/c/16266/13/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3268 PS13, Line 3268: ivate boolean tupleIdIntersect(List<TupleId> tids1, List<TupleId> tids2) { > Sounds like this comment paragraph should be moved above line 3267 to act a Done http://gerrit.cloudera.org:8080/#/c/16266/13/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3277 PS13, Line 3277: */ : private boolean isNullableConjunct(Expr e, List<TupleId> tupleIds) { : // A clause like "t1.v1 IS NOT NULL OR t2.v2 IS NOT NULL" and t1 in 'tupleIds' does : // not prove that t1.v1 can't be NULL, because when t2.v2 IS NOT NULL, t1.v1 can be : // null. But a clause like "t1.v1 IS NOT NULL OR t1.v2 IS NOT NULL" proves that the : // t1 row as a whole can't be all-NULL. : Lis > Please refer to comment at line 3249. We need check each of the disjunctive conjunct's children which is not disjunctive http://gerrit.cloudera.org:8080/#/c/16266/13/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3352 PS13, Line 3352: > nit: reword to "When transforming an outer join to an inner join is feasibl Done http://gerrit.cloudera.org:8080/#/c/16266/13/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3365 PS13, Line 3365: > nit: reword to "When transforming an outer join to a left/right/inner join Done http://gerrit.cloudera.org:8080/#/c/16266/13/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3383 PS13, Line 3383: TableRef ref = globalState_.fullOuterJo > nit: reword to "When transforming an outer join to an inner join is feasibl Done http://gerrit.cloudera.org:8080/#/c/16266/12/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java: http://gerrit.cloudera.org:8080/#/c/16266/12/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@327 PS12, Line 327: f > The function may be used elsewhere in future. I get the conditional functions list from common/function-registry/impala_functions.py http://gerrit.cloudera.org:8080/#/c/16266/13/testdata/workloads/functional-query/queries/QueryTest/outer-to-inner-joins.test File testdata/workloads/functional-query/queries/QueryTest/outer-to-inner-joins.test: http://gerrit.cloudera.org:8080/#/c/16266/13/testdata/workloads/functional-query/queries/QueryTest/outer-to-inner-joins.test@1 PS13, Line 1: == > Can we add some more tests here? The 5) is not currently supported. I will do it in part 2 http://gerrit.cloudera.org:8080/#/c/16266/13/tests/query_test/test_join_queries.py File tests/query_test/test_join_queries.py: http://gerrit.cloudera.org:8080/#/c/16266/13/tests/query_test/test_join_queries.py@96 PS13, Line 96: new_vector.get_value('exec_option')['mt_dop'] = vector.get_value('mt_dop') : self.run_test_case('QueryTest/outer-joins', new_vector) > Nice. Done -- To view, visit http://gerrit.cloudera.org:8080/16266 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa7804033fac68e93f33c387dc68ef67f803e93e Gerrit-Change-Number: 16266 Gerrit-PatchSet: 14 Gerrit-Owner: Xianqing He <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Shant Hovsepian <[email protected]> Gerrit-Reviewer: Xianqing He <[email protected]> Gerrit-Comment-Date: Thu, 27 Aug 2020 16:29:23 +0000 Gerrit-HasComments: Yes
