Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/16266 )
Change subject: IMPALA-5022 part 1/2: Outer join simplification ...................................................................... Patch Set 15: (6 comments) Nice! Thanks for the rework. These two items are my only concerns. 1. The member function checks whether a built-in function is conditional. 2. Whether Java's Set intersection method(s) can be used in the analyzer to keep compilation time low. http://gerrit.cloudera.org:8080/#/c/16266/15/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: http://gerrit.cloudera.org:8080/#/c/16266/15/common/thrift/ImpalaInternalService.thrift@449 PS15, Line 449: optional bool enable_outer_join_to_inner_transformation = true Nice! Thanks a lot for adding the control. 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 > For the sql like A full join B on A.id = B.id join C on B.id = C.id where A Done 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@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 > We need check each of the disjunctive conjunct's children which is not disj Yes and I agree. My comment was about collecting all of them once into a set and use Java's Set data structure to do the intersection more efficiently. For complex queries, the saving in compile time can be substantial. 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 > I get the conditional functions list from common/function-registry/impala_f It seems to me the list in impala_functions.py is not complete. From https://impala.apache.org/docs/build/html/topics/impala_conditional_functions.html#conditional_functions__decode, the following conditional functions are listed. CASE CASE2 COALESCE DECODE IF IFNULL ISFALSE ISNOTFALSE ISNOTTRUE ISNULL ISTRUE NONNULLVALUE NULLIF NULLIFZERO NULLVALUE NVL NVL2 ZEROIFNULL I tested two of these in a way and they work. Note the use of null in them. 1). select case int_col when 1 then 1 when 2 then 2 else null end from functional_parquet.alltypes; 2). SELECT decode(int_col, 1, "Monday", 2, "Tuesday", 3, "Wednesday", 4, "Thursday", 5, "Friday", 6, "Saturday", 7, "Sunday", null) FROM functional_parquet.alltypes; 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: == > The 5) is not currently supported. I will do it in part 2 OK. http://gerrit.cloudera.org:8080/#/c/16266/14/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/14/testdata/workloads/functional-query/queries/QueryTest/outer-to-inner-joins.test@168 PS14, Line 168: n Extra spaces. -- 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: 15 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: Fri, 28 Aug 2020 13:41:38 +0000 Gerrit-HasComments: Yes
