Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
parthchandra committed Apr 11, 2024
1 parent dae33d9 commit bdf6b6d
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,6 @@ class CometSparkSessionExtensions
CometTakeOrderedAndProjectExec
.isSupported(s)
._1 =>
// TODO: support offset for Spark 3.4
val (newOp, info) = QueryPlanSerde.operator2Proto(s)
newOp match {
case Some(nativeOp) =>
Expand Down Expand Up @@ -691,9 +690,7 @@ class CometSparkSessionExtensions
case op =>
// An operator that is not supported by Comet
op match {
case b: CometExec => b
case b: CometBroadcastExchangeExec => b
case b: CometShuffleExchangeExec => b
case _: CometExec | _: CometBroadcastExchangeExec | _: CometShuffleExchangeExec => op
case o =>
opWithInfo(o, CometExplainInfo(s"${o.nodeName} is not supported"))
}
Expand Down
18 changes: 7 additions & 11 deletions spark/src/main/scala/org/apache/comet/ExtendedExplainInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ class ExtendedExplainInfo extends ExtendedExplainGenerator {
private def extensionInfo(plan: SparkPlan): mutable.Seq[String] = {
var info = mutable.Seq[String]()
val sorted = sortup(plan)
sorted.foreach(p => {
sorted.foreach { p =>
val s =
getActualPlan(p).getTagValue(CometExplainInfo.EXTENSION_INFO).map(t => t).getOrElse("")
getActualPlan(p).getTagValue(CometExplainInfo.EXTENSION_INFO).getOrElse("")
if (s.nonEmpty) {
info = info :+ s
}
})
}
info
}

Expand All @@ -65,18 +65,14 @@ class ExtendedExplainInfo extends ExtendedExplainGenerator {
val s = traversed.dequeue()
ordered += s
if (s.innerChildren.nonEmpty) {
s.innerChildren.foreach(c => {
c match {
case _: SparkPlan => traversed.enqueue(getActualPlan(c.asInstanceOf[SparkPlan]))
case _ =>
}
()
})
s.innerChildren.foreach {
case c@(_: SparkPlan) => traversed.enqueue(getActualPlan(c))
case _ =>
}
}
if (s.children.nonEmpty) {
s.children.foreach(c => {
traversed.enqueue(getActualPlan(c))
()
})
}
}
Expand Down

0 comments on commit bdf6b6d

Please sign in to comment.