drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jacq...@apache.org
Subject [6/7] git commit: 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.
Date Fri, 16 May 2014 01:00:16 GMT
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",


Mime
View raw message