Marcel Kornacker has posted comments on this change. Change subject: IMPALA-3126: Conservative assignment of inner-join On-clause predicates. ......................................................................
Patch Set 3: Code-Review+2 (4 comments) http://gerrit.cloudera.org:8080/#/c/4982/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: Line 1340: * join of the corresponding On-clause and the top-most full-outer join below "must be assigned between": i don't find that particularly useful, because this function isn't aware of the whole tree, just one node in it. Line 1348: * lowest node that materializes the required tuple ids, unless they reference i think you can remove the 'lowest node' reference here, since this is only about deciding whether a particular tree node can evaluate an expr. the logic to ensure it's the lowest one isn't evident in this function. Line 1366: // evaluate it the join that the On-clause belongs to. "evaluate it at ..."? http://gerrit.cloudera.org:8080/#/c/4982/2/testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test File testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test: Line 900: on c.id = d.id > It has 4 joins and 5 tables and is exactly the same query as reported in th i think we should stick to a minimal repro, ie, 2 joins. -- To view, visit http://gerrit.cloudera.org:8080/4982 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf45323ed9102ffb45c9d94a130ea3692286f215 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-HasComments: Yes