Fix wrong join type passed to substrait/velox

apache · Sep 9, 2024 · d87bd99 · d87bd99
1 parent 23cb69a
commit d87bd99
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 1 deletion.
diff --git a/cpp/velox/substrait/SubstraitToVeloxPlan.cc b/cpp/velox/substrait/SubstraitToVeloxPlan.cc
@@ -391,6 +391,9 @@ core::PlanNodePtr SubstraitToVeloxPlanConverter::toVeloxPlan(const ::substrait::
     case ::substrait::CrossRel_JoinType::CrossRel_JoinType_JOIN_TYPE_LEFT:
       joinType = core::JoinType::kLeft;
       break;
+    case ::substrait::CrossRel_JoinType::CrossRel_JoinType_JOIN_TYPE_RIGHT:
+      joinType = core::JoinType::kRight;
+      break;
     default:
       VELOX_NYI("Unsupported Join type: {}", std::to_string(crossRel.type()));
   }

diff --git a/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc b/cpp/velox/substrait/SubstraitToVeloxPlanValidator.cc
@@ -973,6 +973,7 @@ bool SubstraitToVeloxPlanValidator::validate(const ::substrait::CrossRel& crossR
   switch (crossRel.type()) {
     case ::substrait::CrossRel_JoinType_JOIN_TYPE_INNER:
     case ::substrait::CrossRel_JoinType_JOIN_TYPE_LEFT:
+    case ::substrait::CrossRel_JoinType_JOIN_TYPE_RIGHT:
       break;
     default:
       LOG_VALIDATION_MSG("Unsupported Join type in CrossRel");

diff --git a/...t/src/main/scala/org/apache/gluten/execution/BroadcastNestedLoopJoinExecTransformer.scala b/...t/src/main/scala/org/apache/gluten/execution/BroadcastNestedLoopJoinExecTransformer.scala
@@ -49,7 +49,7 @@ abstract class BroadcastNestedLoopJoinExecTransformer(
   override def rightKeys: Seq[Expression] = Nil
 
   private lazy val substraitJoinType: CrossRel.JoinType =
-    SubstraitUtil.toCrossRelSubstrait(joinType)
+    SubstraitUtil.toCrossRelSubstrait(joinType, buildSide)
 
   // Unique ID for builded table
   lazy val buildBroadcastTableId: String = "BuiltBNLJBroadcastTable-" + buildPlan.id

diff --git a/gluten-substrait/src/main/scala/org/apache/gluten/utils/SubstraitUtil.scala b/gluten-substrait/src/main/scala/org/apache/gluten/utils/SubstraitUtil.scala
@@ -23,6 +23,7 @@ import org.apache.gluten.substrait.SubstraitContext
 import org.apache.gluten.substrait.expression.ExpressionNode
 
 import org.apache.spark.sql.catalyst.expressions.{Attribute, Expression}
+import org.apache.spark.sql.catalyst.optimizer.{BuildLeft, BuildRight, BuildSide}
 import org.apache.spark.sql.catalyst.plans.{FullOuter, InnerLike, JoinType, LeftAnti, LeftOuter, LeftSemi, RightOuter}
 
 import io.substrait.proto.{CrossRel, JoinRel}
@@ -49,6 +50,31 @@ object SubstraitUtil {
       JoinRel.JoinType.UNRECOGNIZED
   }
 
+  def toCrossRelSubstrait(sparkJoin: JoinType, buildSide: BuildSide): CrossRel.JoinType =
+    sparkJoin match {
+      case _: InnerLike =>
+        CrossRel.JoinType.JOIN_TYPE_INNER
+      case LeftOuter =>
+        // since we always assume build right side in substrait,
+        // the left and right relations are exchanged and the
+        // join type is reverted.
+        buildSide match {
+          case BuildLeft => CrossRel.JoinType.JOIN_TYPE_RIGHT
+          case BuildRight => CrossRel.JoinType.JOIN_TYPE_LEFT
+        }
+      case RightOuter =>
+        buildSide match {
+          case BuildRight => CrossRel.JoinType.JOIN_TYPE_RIGHT
+          case BuildLeft => CrossRel.JoinType.JOIN_TYPE_LEFT
+        }
+      case LeftSemi =>
+        CrossRel.JoinType.JOIN_TYPE_LEFT_SEMI
+      case FullOuter =>
+        CrossRel.JoinType.JOIN_TYPE_OUTER
+      case _ =>
+        CrossRel.JoinType.UNRECOGNIZED
+    }
+
   def toCrossRelSubstrait(sparkJoin: JoinType): CrossRel.JoinType = sparkJoin match {
     case _: InnerLike =>
       CrossRel.JoinType.JOIN_TYPE_INNER