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 <jni@maprtech.com>
Authored: Thu May 15 16:11:04 2014 -0700
Committer: Jacques Nadeau <jacques@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",
|