Xianqing He has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16266 )

Change subject: IMPALA-5022: Outer join simplification
......................................................................


Patch Set 13:

(5 comments)

I had add new query tests 
testdata/workloads/functional-query/queries/QueryTest/outer-to-inner-joins.test 
to test the correctness

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@3286
PS12, Line 3286: ntains a U
> nit: contains
Done


http://gerrit.cloudera.org:8080/#/c/16266/12/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3286
PS12, Line 3286: a c
> nit: Reword to 'if a'
Done


http://gerrit.cloudera.org:8080/#/c/16266/12/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3313
PS12, Line 3313:  is
> nit: extra space. need to put 'the' before null-rejecting predicate.
Done


http://gerrit.cloudera.org:8080/#/c/16266/12/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@3482
PS12, Line 3482:  case INNER_JOIN: {
               :         if (tableRef.getLeftTblRef() != null) {
               :           boolean ret = 
simplifyOuterJoinsByIjOnClause(processedTblRefs, tableRef);
               :           isSimplified = isSimplified ? true : ret;
               :         }
               :         break;
> I wonder if we need this case here.
This case optimizes the pattern likes A left join B on A.id=B.id inner join C 
on B.id=C.id


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
> We may need to add some more to the list, such as 'case'. See https://impal
In Analyzer#isNullableConjunct will check case expr.
Other conditional functions like 'nvl2' rewriting to 'if', 'decode' rewriting 
to CaseExpr



--
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: 13
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: Wed, 26 Aug 2020 08:15:38 +0000
Gerrit-HasComments: Yes

Reply via email to