[Impala-ASF-CR] IMPALA-8361: Propagate predicates of outer-joined InlineView

["Quanlong Huang (Code Review)" <ge...@cloudera.org>]

Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15047 )

Change subject: IMPALA-8361: Propagate predicates of outer-joined InlineView
......................................................................


Patch Set 9:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/15047/9//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15047/9//COMMIT_MSG@24
PS9, Line 24: ,
nit: need space


http://gerrit.cloudera.org:8080/#/c/15047/9//COMMIT_MSG@24
PS9, Line 24: the predicates that
            : must be evaluted at a join node but can also be safely evaluted by the
            : outer-joined inline view.
I think you mean "some predicates that must be evaluted at a join node can also be safely evaluted by the outer-joined inline view".


http://gerrit.cloudera.org:8080/#/c/15047/9//COMMIT_MSG@30
PS9, Line 30: .
nit: need space


http://gerrit.cloudera.org:8080/#/c/15047/9/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:

http://gerrit.cloudera.org:8080/#/c/15047/9/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@985
PS9, Line 985: picked up by getBoundPredicates()
I think this comment is stale after merging this patch. Could you update it to "picked up by getBoundPredicates() and migrateConjunctsToInlineView()"?


http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:

http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2368
PS9, Line 2368: predicates: rand() = 12
This looks strange to me. rand() returns a random value between 0 and 1 so "rand() = 12" will always be false. All rows should be rejected by the WHERE clause. If "rand() = 12" is evaluated in only one side, the other side can still produce rows. So the outer join will still have results.

However, looks like the original planner has the same plan. Could you create a JIRA for this? I think it's a bug. It's worth to mention it in the above comments.


http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2402
PS9, Line 2402:  ,
nit: put the space after the comma


http://gerrit.cloudera.org:8080/#/c/15047/9/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2442
PS9, Line 2442: upper(b.string_col)
Should we move the function inside the view? This can be propagated without this patch. Maybe change it to

 SELECT * FROM functional.alltypestiny a
LEFT JOIN
(SELECT upper(b.string_col) as string_col, b.id FROM functional.alltypestiny a LEFT JOIN
functional.alltypestiny b ON a.id=b.id) b ON a.id=b.id
WHERE b.string_col='1';



-- 
To view, visit http://gerrit.cloudera.org:8080/15047
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c23a45aeb5dd1aa06a95c9aa8628ecbe37ef2c1
Gerrit-Change-Number: 15047
Gerrit-PatchSet: 9
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Xianqing He <he...@126.com>
Gerrit-Comment-Date: Tue, 04 Feb 2020 03:49:33 +0000
Gerrit-HasComments: Yes