impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Kornacker (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-3063: Separate join inversion from join ordering.
Date Mon, 15 Aug 2016 18:20:50 GMT
Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-3063: Separate join inversion from join ordering.

Patch Set 2:

File fe/src/main/java/com/cloudera/impala/analysis/

Line 125:   private boolean isStraightJoin_ = false;
nice clean-up
File fe/src/main/java/com/cloudera/impala/planner/

Line 81
do we still have these checks somewhere?

Line 95
what about this?
File fe/src/main/java/com/cloudera/impala/planner/

Line 478:     for (BinaryPredicate p: eqJoinConjuncts_) Collections.swap(p.getChildren(),
0, 1);
add BinaryPredicate.reverse()?
File fe/src/main/java/com/cloudera/impala/planner/

Line 345:    * 1. The right-hand side is a SingularRowSrcNode. We place such a node on the
the first sentence makes it sound like it inverts *if* the rhs is already a singularrowsrc,
in other words, it moves it to the lhs.

Line 355:   private void invertJoins(PlanNode root, boolean isSingleNodeExec) {
explain 2nd param

Line 360:       for (PlanNode child: root.getChildren()) {
single line

Line 372:       //    we cannot execute it efficiently.
the same is true for right-anything if it's a broadcast. do we know we're not going to run
into that?

why does a repartitioning null-aware right anti join not work?

Line 373:       if (joinNode.isStraightJoin() || joinOp.isNullAwareLeftAntiJoin()) {
what does it mean to deal with 'straight join' here and not for the entire block?

Line 380:         // Always place a singular row src on the build side because it has
has produces

Line 385:         // There is no backend support for distributed non-equi right outer/semi
this seems to contradict the condition, which triggers if !issinglenodeexec (maybe phrase
that flag as isDistrExec?)

PS2, Line 388: build-side hash table
"the materialized rhs", no? (this still applies to nlj?)

Line 398:             !(joinOp.isInnerJoin() && !joinNode.getEqJoinConjuncts().isEmpty()))
why can't this be applied to inner equi joins?

Line 404:     // Re-compute tuple ids since their order must correspond to the order of children.

Line 414:   private PlanNode useNljForSmallBuilds(PlanNode root, Analyzer analyzer)
the name doesn't really reflect the (much narrower) semantics, find a better one.

Line 423:     if (joinNode.getJoinOp().isNullAwareLeftAntiJoin()) return root;
also check that this is really a hash join

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: If86db7753fc585bb4c69612745ec0103278888a4
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-HasComments: Yes

View raw message