Marcel Kornacker has posted comments on this change. Change subject: IMPALA-3063: Separate join inversion from join ordering. ......................................................................
Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/3846/2//COMMIT_MSG Commit Message: Line 7: IMPALA-3063: Separate join inversion from join ordering. > Correct. Let me explain why it is not only a separation of functionality. a) please add this explanation to the commit msg b) follow-on work: a join b and b join a should really be symmetric/the fk/pk detection heuristic should work in both directions (having it be symmetric means that the inversion logic really can be separated from ordering, aside from doing that just for code structuring purposes). http://gerrit.cloudera.org:8080/#/c/3846/2/testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test File testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test: Line 814: | |--07:HASH JOIN [RIGHT OUTER JOIN] > Good catch. The cardinality estimates are clear: Join 08 should indeed be i i think we want to improve the existing behavior, no? :) http://gerrit.cloudera.org:8080/#/c/3846/2/testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test File testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test: Line 1088: | hash predicates: l_suppkey = s_suppkey, l_partkey = ps_partkey > We do order them based on cost, but we don't have a tie breaker to get an a yes, i didn't mean as part of this patch. and this sounds like a ramp-up task for someone else. Line 1777: | hash predicates: p_partkey = ps_partkey > Another good catch. The change was only coincidentally caused by this patch so is that fixed now? -- To view, visit http://gerrit.cloudera.org:8080/3846 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If86db7753fc585bb4c69612745ec0103278888a4 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
