Fix bug in MergeJoin when there are repeating values across left batches. Ensure join type is specified. Add join type to some physical plan in unit test case.
Re-enable a testcase for MergeJoin. + Project: http://git-wip-us.apache.org/repos/asf/incubator-drill/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-drill/commit/cc99b97d Tree: http://git-wip-us.apache.org/repos/asf/incubator-drill/tree/cc99b97d Diff: http://git-wip-us.apache.org/repos/asf/incubator-drill/diff/cc99b97d Branch: refs/heads/master Commit: cc99b97db1d2de47fb012c6157de5b96b4f2a5b9 Parents: 74c1d1b Author: Jinfeng Ni <j...@maprtech.com> Authored: Thu May 15 16:11:04 2014 -0700 Committer: Jacques Nadeau <jacq...@apache.org> Committed: Thu May 15 16:59:39 2014 -0700 ---------------------------------------------------------------------- .../org/apache/drill/exec/physical/config/HashJoinPOP.java | 1 + .../org/apache/drill/exec/physical/config/MergeJoinPOP.java | 9 +++++---- .../apache/drill/exec/physical/impl/join/JoinTemplate.java | 3 ++- .../exec/physical/impl/join/TestMergeJoinMulCondition.java | 2 +- exec/java-exec/src/test/resources/join/hash_join.json | 3 ++- .../src/test/resources/join/hash_join_multi_batch.json | 3 ++- .../src/test/resources/join/hj_multi_condition_join.json | 3 ++- exec/java-exec/src/test/resources/join/join_batchsize.json | 3 ++- 8 files changed, 17 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/cc99b97d/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashJoinPOP.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashJoinPOP.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashJoinPOP.java index c3e74cf..4ae27b8 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashJoinPOP.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/HashJoinPOP.java @@ -67,6 +67,7 @@ public class HashJoinPOP extends AbstractBase { this.left = left; this.right = right; this.conditions = conditions; + Preconditions.checkArgument(joinType != null, "Join type is missing!"); this.joinType = joinType; } http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/cc99b97d/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/MergeJoinPOP.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/MergeJoinPOP.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/MergeJoinPOP.java index 5bb378a..264ee94 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/MergeJoinPOP.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/MergeJoinPOP.java @@ -40,7 +40,7 @@ import com.google.common.collect.Iterators; public class MergeJoinPOP extends AbstractBase{ static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(MergeJoinPOP.class); - + private final PhysicalOperator left; private final PhysicalOperator right; private final List<JoinCondition> conditions; @@ -53,7 +53,7 @@ public class MergeJoinPOP extends AbstractBase{ @JsonCreator public MergeJoinPOP( - @JsonProperty("left") PhysicalOperator left, + @JsonProperty("left") PhysicalOperator left, @JsonProperty("right") PhysicalOperator right, @JsonProperty("conditions") List<JoinCondition> conditions, @JsonProperty("joinType") JoinRelType joinType @@ -61,10 +61,11 @@ public class MergeJoinPOP extends AbstractBase{ this.left = left; this.right = right; this.conditions = conditions; + Preconditions.checkArgument(joinType != null, "Join type is missing!"); this.joinType = joinType; Preconditions.checkArgument(joinType != JoinRelType.FULL, "Full outer join not currently supported"); } - + @Override public Size getSize() { return left.getSize().add(right.getSize()); @@ -101,7 +102,7 @@ public class MergeJoinPOP extends AbstractBase{ public List<JoinCondition> getConditions() { return conditions; } - + public MergeJoinPOP flipIfRight(){ if(joinType == JoinRelType.RIGHT){ List<JoinCondition> flippedConditions = Lists.newArrayList(conditions.size()); http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/cc99b97d/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java index af0d378..0bfef5b 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinTemplate.java @@ -159,7 +159,8 @@ public abstract class JoinTemplate implements JoinWorker { } while (status.isRightPositionInCurrentBatch() && doCompare(status.getLeftPosition(), status.getRightPosition()) == 0); - if (status.getRightPosition() > initialRightPosition && status.isLeftRepeating()) + if (status.getRightPosition() > initialRightPosition && + (status.isLeftRepeating() || ! status.isNextLeftPositionInCurrentBatch())) // more than one matching result from right table; reset position in case of subsequent left match status.setRightPosition(initialRightPosition); status.advanceLeft(); http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/cc99b97d/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestMergeJoinMulCondition.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestMergeJoinMulCondition.java b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestMergeJoinMulCondition.java index 5167b2e..1994987 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestMergeJoinMulCondition.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestMergeJoinMulCondition.java @@ -37,7 +37,7 @@ import org.junit.rules.TestRule; import com.google.common.base.Charsets; import com.google.common.io.Files; -@Ignore("Currently returns wrong result. Stopped working when incoming became more than one result set.") +//@Ignore("Currently returns wrong result. Stopped working when incoming became more than one result set.") public class TestMergeJoinMulCondition extends PopUnitTestBase { static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestMergeJoinMulCondition.class); http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/cc99b97d/exec/java-exec/src/test/resources/join/hash_join.json ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/resources/join/hash_join.json b/exec/java-exec/src/test/resources/join/hash_join.json index e71d0bc..41a983b 100644 --- a/exec/java-exec/src/test/resources/join/hash_join.json +++ b/exec/java-exec/src/test/resources/join/hash_join.json @@ -52,7 +52,8 @@ right: 3, left: 4, pop: "hash-join", - conditions: [ {relationship: "==", left: "B", right: "A"} ] + conditions: [ {relationship: "==", left: "B", right: "A"} ], + joinType : "INNER" }, { @id: 6, http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/cc99b97d/exec/java-exec/src/test/resources/join/hash_join_multi_batch.json ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/resources/join/hash_join_multi_batch.json b/exec/java-exec/src/test/resources/join/hash_join_multi_batch.json index 16a1f2e..c71914b 100644 --- a/exec/java-exec/src/test/resources/join/hash_join_multi_batch.json +++ b/exec/java-exec/src/test/resources/join/hash_join_multi_batch.json @@ -36,7 +36,8 @@ right: 1, left: 2, pop: "hash-join", - conditions: [ {relationship: "==", left: "blue1", right: "blue"} ] + conditions: [ {relationship: "==", left: "blue1", right: "blue"} ], + joinType : "INNER" }, { @id: 4, http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/cc99b97d/exec/java-exec/src/test/resources/join/hj_multi_condition_join.json ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/resources/join/hj_multi_condition_join.json b/exec/java-exec/src/test/resources/join/hj_multi_condition_join.json index 23818ed..0f1c32b 100644 --- a/exec/java-exec/src/test/resources/join/hj_multi_condition_join.json +++ b/exec/java-exec/src/test/resources/join/hj_multi_condition_join.json @@ -55,7 +55,8 @@ conditions: [ {relationship: "==", left: "B", right: "A"}, {relationship: "==", left: "DCOL", right: "CCOL"} - ] + ], + joinType : "INNER" }, { @id: 6, http://git-wip-us.apache.org/repos/asf/incubator-drill/blob/cc99b97d/exec/java-exec/src/test/resources/join/join_batchsize.json ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/resources/join/join_batchsize.json b/exec/java-exec/src/test/resources/join/join_batchsize.json index dfbfe20..969ff0d 100644 --- a/exec/java-exec/src/test/resources/join/join_batchsize.json +++ b/exec/java-exec/src/test/resources/join/join_batchsize.json @@ -66,7 +66,8 @@ right: 6, left: 3, pop: "merge-join", - conditions: [ {relationship: "==", left: "blue", right: "blue1"} ] + conditions: [ {relationship: "==", left: "blue", right: "blue1"} ], + joinType : "INNER" }, { pop : "limit",
