maropu commented on a change in pull request #31736:
URL: https://github.com/apache/spark/pull/31736#discussion_r587461811
##########
File path:
sql/core/src/test/scala/org/apache/spark/sql/execution/WholeStageCodegenSuite.scala
##########
@@ -92,6 +92,39 @@ class WholeStageCodegenSuite extends QueryTest with
SharedSparkSession
Seq(Row(0, 0, 0), Row(1, 1, 1), Row(2, 2, 2), Row(3, 3, 3), Row(4, 4,
4)))
}
+ test("Inner/Cross BroadcastNestedLoopJoinExec should be included in
WholeStageCodegen") {
+ val df1 = spark.range(4).select($"id".as("k1"))
+ val df2 = spark.range(3).select($"id".as("k2"))
+ val df3 = spark.range(2).select($"id".as("k3"))
+
+ // test broadcast nested loop join without condition
+ val oneJoinDF = df1.join(df2)
+ assert(oneJoinDF.queryExecution.executedPlan.collect {
+ case WholeStageCodegenExec(_ : BroadcastNestedLoopJoinExec) => true
+ }.size === 1)
+ checkAnswer(oneJoinDF,
Review comment:
For better test coverage, could you run tests with codegen
enabled/disabled?
##########
File path:
sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/JoinBenchmark.scala
##########
@@ -166,6 +166,19 @@ object JoinBenchmark extends SqlBasedBenchmark {
}
}
+ def broadcastNestedLoopJoin(): Unit = {
+ val N = 20 << 20
+ val M = 1 << 4
+
+ val dim = broadcast(spark.range(M).selectExpr("id as k", "cast(id as
string) as v"))
+ codegenBenchmark("broadcast nested loop join", N) {
Review comment:
Could you update `benchmarks/JoinBenchmark-results.txt`, too?
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/joins/BroadcastNestedLoopJoinExec.scala
##########
@@ -393,4 +394,65 @@ case class BroadcastNestedLoopJoinExec(
}
}
}
+
+ override def supportCodegen: Boolean = {
+ joinType.isInstanceOf[InnerLike]
+ }
+
+ override def inputRDDs(): Seq[RDD[InternalRow]] = {
+ streamed.asInstanceOf[CodegenSupport].inputRDDs()
+ }
+
+ override def needCopyResult: Boolean = true
+
+ override def doProduce(ctx: CodegenContext): String = {
+ streamed.asInstanceOf[CodegenSupport].produce(ctx, this)
+ }
+
+ override def doConsume(ctx: CodegenContext, input: Seq[ExprCode], row:
ExprCode): String = {
+ joinType match {
+ case _: InnerLike => codegenInner(ctx, input)
+ case x =>
+ throw new IllegalArgumentException(
+ s"BroadcastNestedLoopJoin code-gen should not take $x as the
JoinType")
+ }
+ }
+
+ /**
+ * Returns the variable name for [[Broadcast]] side.
+ */
+ private def prepareBroadcast(ctx: CodegenContext): String = {
+ // Create a name for broadcast side
+ val broadcastArray = broadcast.executeBroadcast[Array[InternalRow]]()
+ val broadcastTerm = ctx.addReferenceObj("broadcastTerm", broadcastArray)
+
+ // Inline mutable state since not many join operations in a task
+ ctx.addMutableState("InternalRow[]", "broadcastArray",
+ v => s"$v = (InternalRow[]) $broadcastTerm.value();", forceInline = true)
+ }
+
+ private def codegenInner(ctx: CodegenContext, input: Seq[ExprCode]): String
= {
+ val arrayTerm = prepareBroadcast(ctx)
Review comment:
nit: `arrayTerm` -> `buildRowArrayTerm`, `buildRowsTerm`, ...?
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/joins/BroadcastNestedLoopJoinExec.scala
##########
@@ -393,4 +394,65 @@ case class BroadcastNestedLoopJoinExec(
}
}
}
+
+ override def supportCodegen: Boolean = {
+ joinType.isInstanceOf[InnerLike]
+ }
+
+ override def inputRDDs(): Seq[RDD[InternalRow]] = {
+ streamed.asInstanceOf[CodegenSupport].inputRDDs()
+ }
+
+ override def needCopyResult: Boolean = true
+
+ override def doProduce(ctx: CodegenContext): String = {
+ streamed.asInstanceOf[CodegenSupport].produce(ctx, this)
+ }
+
+ override def doConsume(ctx: CodegenContext, input: Seq[ExprCode], row:
ExprCode): String = {
+ joinType match {
+ case _: InnerLike => codegenInner(ctx, input)
+ case x =>
+ throw new IllegalArgumentException(
+ s"BroadcastNestedLoopJoin code-gen should not take $x as the
JoinType")
+ }
+ }
+
+ /**
+ * Returns the variable name for [[Broadcast]] side.
+ */
+ private def prepareBroadcast(ctx: CodegenContext): String = {
+ // Create a name for broadcast side
+ val broadcastArray = broadcast.executeBroadcast[Array[InternalRow]]()
+ val broadcastTerm = ctx.addReferenceObj("broadcastTerm", broadcastArray)
+
+ // Inline mutable state since not many join operations in a task
+ ctx.addMutableState("InternalRow[]", "broadcastArray",
Review comment:
nit: `broadcastArray` -> `buildRows`, `buidlRowArray`, ...?
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/joins/BroadcastNestedLoopJoinExec.scala
##########
@@ -393,4 +394,65 @@ case class BroadcastNestedLoopJoinExec(
}
}
}
+
+ override def supportCodegen: Boolean = {
+ joinType.isInstanceOf[InnerLike]
+ }
+
+ override def inputRDDs(): Seq[RDD[InternalRow]] = {
+ streamed.asInstanceOf[CodegenSupport].inputRDDs()
+ }
+
+ override def needCopyResult: Boolean = true
Review comment:
always true? Is there a trade-off between the overheads of uniqueness
checks and result copys?
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/joins/BroadcastNestedLoopJoinExec.scala
##########
@@ -393,4 +394,65 @@ case class BroadcastNestedLoopJoinExec(
}
}
}
+
+ override def supportCodegen: Boolean = {
+ joinType.isInstanceOf[InnerLike]
+ }
+
+ override def inputRDDs(): Seq[RDD[InternalRow]] = {
+ streamed.asInstanceOf[CodegenSupport].inputRDDs()
+ }
+
+ override def needCopyResult: Boolean = true
+
+ override def doProduce(ctx: CodegenContext): String = {
+ streamed.asInstanceOf[CodegenSupport].produce(ctx, this)
+ }
+
+ override def doConsume(ctx: CodegenContext, input: Seq[ExprCode], row:
ExprCode): String = {
+ joinType match {
+ case _: InnerLike => codegenInner(ctx, input)
+ case x =>
+ throw new IllegalArgumentException(
+ s"BroadcastNestedLoopJoin code-gen should not take $x as the
JoinType")
Review comment:
nit:
```
case _ =>
throw new IllegalArgumentException(
s"BroadcastNestedLoopJoin code-gen should not take $joinType as
the JoinType")
```
?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
