DRILL-6323: Lateral Join - Refactor Join PopConfigs

apache · Apr 18, 2018 · 8051c24 · 8051c24
1 parent 62f1469
commit 8051c24
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 175 deletions.
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractJoinPop.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractJoinPop.java
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.physical.base;
+
+import com.google.common.collect.Iterators;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.drill.common.expression.LogicalExpression;
+import org.apache.drill.common.logical.data.JoinCondition;
+
+import java.util.Iterator;
+import java.util.List;
+
+public abstract class AbstractJoinPop extends AbstractBase {
+
+  protected final PhysicalOperator left;
+
+  protected final PhysicalOperator right;
+
+  protected final JoinRelType joinType;
+
+  protected final LogicalExpression condition;
+
+  protected final List<JoinCondition> conditions;
+
+  public AbstractJoinPop(PhysicalOperator leftOp, PhysicalOperator rightOp,
+                         JoinRelType joinType, LogicalExpression joinCondition,
+                         List<JoinCondition> joinConditions) {
+    left = leftOp;
+    right = rightOp;
+    this.joinType = joinType;
+    condition = joinCondition;
+    conditions = joinConditions;
+  }
+
+  @Override
+  public <T, X, E extends Throwable> T accept(PhysicalVisitor<T, X, E> physicalVisitor, X value) throws E {
+    return physicalVisitor.visitOp(this, value);
+  }
+
+  @Override
+  public Iterator<PhysicalOperator> iterator() {
+    return Iterators.forArray(left, right);
+  }
+
+  public PhysicalOperator getLeft() {
+    return left;
+  }
+
+  public PhysicalOperator getRight() {
+    return right;
+  }
+
+  public JoinRelType getJoinType() {
+    return joinType;
+  }
+
+  public LogicalExpression getCondition() { return condition; }
+
+  public List<JoinCondition> getConditions() {
+    return conditions;
+  }
+
+}
diff --git a/.../java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractPhysicalVisitor.java b/.../java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractPhysicalVisitor.java
@@ -137,21 +137,6 @@ public T visitChildren(PhysicalOperator op, X value) throws E{
     return null;
   }
 
-  @Override
-  public T visitMergeJoin(MergeJoinPOP join, X value) throws E {
-    return visitOp(join, value);
-  }
-
-  @Override
-  public T visitHashJoin(HashJoinPOP join, X value) throws E {
-    return visitOp(join, value);
-  }
-
-  @Override
-  public T visitNestedLoopJoin(NestedLoopJoinPOP join, X value) throws E {
-    return visitOp(join, value);
-  }
-
   @Override
   public T visitHashPartitionSender(HashPartitionSender op, X value) throws E {
     return visitSender(op, value);

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/PhysicalVisitor.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/PhysicalVisitor.java
@@ -62,9 +62,6 @@ public interface PhysicalVisitor<RETURN, EXTRA, EXCEP extends Throwable> {
   public RETURN visitSort(Sort sort, EXTRA value) throws EXCEP;
   public RETURN visitLimit(Limit limit, EXTRA value) throws EXCEP;
   public RETURN visitFlatten(FlattenPOP flatten, EXTRA value) throws EXCEP;
-  public RETURN visitMergeJoin(MergeJoinPOP join, EXTRA value) throws EXCEP;
-  public RETURN visitHashJoin(HashJoinPOP join, EXTRA value) throws EXCEP;
-  public RETURN visitNestedLoopJoin(NestedLoopJoinPOP join, EXTRA value) throws EXCEP;
   public RETURN visitSender(Sender sender, EXTRA value) throws EXCEP;
   public RETURN visitReceiver(Receiver receiver, EXTRA value) throws EXCEP;
   public RETURN visitStreamingAggregate(StreamingAggregate agg, EXTRA value) throws EXCEP;

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
@@ -17,50 +17,29 @@
  */
 package org.apache.drill.exec.physical.config;
 
-import java.util.Iterator;
-import java.util.List;
-
-import org.apache.drill.common.logical.data.JoinCondition;
-import org.apache.drill.exec.physical.base.AbstractBase;
-import org.apache.drill.exec.physical.base.PhysicalOperator;
-import org.apache.drill.exec.physical.base.PhysicalVisitor;
-import org.apache.drill.exec.proto.UserBitShared.CoreOperatorType;
-import org.apache.calcite.rel.core.JoinRelType;
-
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.annotation.JsonTypeName;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.Iterators;
 import com.google.common.collect.Lists;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.drill.common.logical.data.JoinCondition;
+import org.apache.drill.exec.physical.base.AbstractJoinPop;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.proto.UserBitShared.CoreOperatorType;
+
+import java.util.List;
 
 @JsonTypeName("hash-join")
-public class HashJoinPOP extends AbstractBase {
+public class HashJoinPOP extends AbstractJoinPop {
     static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(HashJoinPOP.class);
 
-
-    private final PhysicalOperator left;
-    private final PhysicalOperator right;
-    private final List<JoinCondition> conditions;
-    private final JoinRelType joinType;
-
     @JsonCreator
-    public HashJoinPOP(
-            @JsonProperty("left") PhysicalOperator left,
-            @JsonProperty("right") PhysicalOperator right,
-            @JsonProperty("conditions") List<JoinCondition> conditions,
-            @JsonProperty("joinType") JoinRelType joinType
-    ) {
-        this.left = left;
-        this.right = right;
-        this.conditions = conditions;
-        Preconditions.checkArgument(joinType != null, "Join type is missing!");
-        this.joinType = joinType;
-    }
-
-    @Override
-    public <T, X, E extends Throwable> T accept(PhysicalVisitor<T, X, E> physicalVisitor, X value) throws E {
-        return physicalVisitor.visitHashJoin(this, value);
+    public HashJoinPOP(@JsonProperty("left") PhysicalOperator left, @JsonProperty("right") PhysicalOperator right,
+                       @JsonProperty("conditions") List<JoinCondition> conditions,
+                       @JsonProperty("joinType") JoinRelType joinType) {
+        super(left, right, joinType, null, conditions);
+        Preconditions.checkArgument(joinType != null, "Join type is missing for HashJoin Pop");
     }
 
     @Override
@@ -69,41 +48,20 @@ public PhysicalOperator getNewWithChildren(List<PhysicalOperator> children) {
         return new HashJoinPOP(children.get(0), children.get(1), conditions, joinType);
     }
 
-    @Override
-    public Iterator<PhysicalOperator> iterator() {
-        return Iterators.forArray(left, right);
-    }
-
-    public PhysicalOperator getLeft() {
-        return left;
-    }
-
-    public PhysicalOperator getRight() {
-        return right;
-    }
-
-    public JoinRelType getJoinType() {
-        return joinType;
-    }
-
-    public List<JoinCondition> getConditions() {
-        return conditions;
-    }
-
-    public HashJoinPOP flipIfRight(){
-        if(joinType == JoinRelType.RIGHT){
+    public HashJoinPOP flipIfRight() {
+        if (joinType == JoinRelType.RIGHT) {
             List<JoinCondition> flippedConditions = Lists.newArrayList();
-            for(JoinCondition c : conditions){
+            for (JoinCondition c : conditions) {
                 flippedConditions.add(c.flip());
             }
             return new HashJoinPOP(right, left, flippedConditions, JoinRelType.LEFT);
-        }else{
+        } else {
             return this;
         }
     }
 
     @Override
     public int getOperatorType() {
-      return CoreOperatorType.HASH_JOIN_VALUE;
+        return CoreOperatorType.HASH_JOIN_VALUE;
     }
 }
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
@@ -17,51 +17,34 @@
  */
 package org.apache.drill.exec.physical.config;
 
-import java.util.Iterator;
-import java.util.List;
-
-import org.apache.drill.common.logical.data.JoinCondition;
-import org.apache.drill.exec.physical.base.AbstractBase;
-import org.apache.drill.exec.physical.base.PhysicalOperator;
-import org.apache.drill.exec.physical.base.PhysicalVisitor;
-import org.apache.drill.exec.proto.UserBitShared.CoreOperatorType;
-import org.apache.calcite.rel.core.JoinRelType;
-
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.annotation.JsonTypeName;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.Iterators;
 import com.google.common.collect.Lists;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.drill.common.logical.data.JoinCondition;
+import org.apache.drill.exec.physical.base.AbstractJoinPop;
+import org.apache.drill.exec.physical.base.PhysicalOperator;
+import org.apache.drill.exec.proto.UserBitShared.CoreOperatorType;
+
+import java.util.List;
 
 @JsonTypeName("merge-join")
-public class MergeJoinPOP extends AbstractBase{
+public class MergeJoinPOP extends AbstractJoinPop{
   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;
-  private final JoinRelType joinType;
-
   @JsonCreator
   public MergeJoinPOP(
       @JsonProperty("left") PhysicalOperator left,
       @JsonProperty("right") PhysicalOperator right,
       @JsonProperty("conditions") List<JoinCondition> conditions,
       @JsonProperty("joinType") JoinRelType joinType
   ) {
-    this.left = left;
-    this.right = right;
-    this.conditions = conditions;
+    super(left, right, joinType, null, 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 <T, X, E extends Throwable> T accept(PhysicalVisitor<T, X, E> physicalVisitor, X value) throws E {
-    return physicalVisitor.visitMergeJoin(this, value);
+    Preconditions.checkArgument(joinType != JoinRelType.FULL,
+      "Full outer join not currently supported with Merge Join");
   }
 
   @Override
@@ -70,27 +53,6 @@ public PhysicalOperator getNewWithChildren(List<PhysicalOperator> children) {
     return new MergeJoinPOP(children.get(0), children.get(1), conditions, joinType);
   }
 
-  @Override
-  public Iterator<PhysicalOperator> iterator() {
-    return Iterators.forArray(left, right);
-  }
-
-  public PhysicalOperator getLeft() {
-    return left;
-  }
-
-  public PhysicalOperator getRight() {
-    return right;
-  }
-
-  public JoinRelType getJoinType() {
-    return joinType;
-  }
-
-  public List<JoinCondition> getConditions() {
-    return conditions;
-  }
-
   public MergeJoinPOP flipIfRight(){
     if(joinType == JoinRelType.RIGHT){
       List<JoinCondition> flippedConditions = Lists.newArrayList();
@@ -101,7 +63,6 @@ public MergeJoinPOP flipIfRight(){
     }else{
       return this;
     }
-
   }
 
   @Override