Skip to content

Commit 722d02c

Browse files
zhztheplayeryaooqinn
authored andcommitted
[SPARK-52484][SQL] Skip child.supportsColumnar assertion from driver side in ColumnarToRowExec
https://issues.apache.org/jira/browse/SPARK-52484 ### What changes were proposed in this pull request? The PR removes the unnecessary assertion in `ColumnarToRowExec` introduced by #25264 to guarantee some flexibilities for 3rd Spark plugins. Especially in Apache Gluten, the assertion blocks some of our effort in query optimization because we needed an intermediate state of the query plan which Spark may see as illegal. Moreover, some typical reasons this intermediate state is needed in Gluten are: 1. Gluten has a cost evaluator API to evaluate the cost of a `transition rule` (which adds a unary node on top of an input plan). In the case Gluten will need a fake leaf to let the rule apply on it for cost evaluation. This leaf node has to be made a columnar one to bypass this assertion, which is a bit hacky. 2. Gluten has a cascades-style query optimizer (RAS) which could set a leaf, dummy, row-based plan node to hide up a child-tree of a brach query plan node, during which this leaf is to represent a so-called cascades 'group'. Although this pattern (C2R on a row-based plan) is illegal, it could still be used as the input of an optimizer rule to potentially be matched on and then to be converted into a valid query plan. This PR is to remove the assertion to ensure some flexibilities to the 3rd plugins. This should be no harm for the upstream Apache Spark, because the query execution will still be failed by [this error](https://github.com/apache/spark/blob/5d0b2f41794bf4dd25b3ce19bc4f634082b40876/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkPlan.scala#L343-L351) without this assertion on an illegal query plan. Some workarounds used by Gluten for bypassing this assertion: 1. https://github.com/apache/incubator-gluten/blob/0a1b5c28678653242ab0fd7b28ebba1dca43ccb1/gluten-core/src/main/scala/org/apache/gluten/extension/columnar/transition/package.scala#L83 2. https://github.com/apache/incubator-gluten/blob/0a1b5c28678653242ab0fd7b28ebba1dca43ccb1/gluten-core/src/main/scala/org/apache/gluten/extension/columnar/enumerated/planner/plan/GlutenPlanModel.scala#L51-L55 Once the assertion is removed, Gluten will be able to remove these workarounds to simply code. ### Does this PR introduce _any_ user-facing change? Basically no. An assertion error in plan-building time will be replaced by an exception in execution time (still from the driver side) when an illegal query plan is generated. ### How was this patch tested? Existing UTs. Closes #51183 from zhztheplayer/wip-rm-c2r-check. Authored-by: Hongze Zhang <[email protected]> Signed-off-by: Kent Yao <[email protected]>
1 parent fa7b6b5 commit 722d02c

File tree

1 file changed

+0
-4
lines changed

1 file changed

+0
-4
lines changed

sql/core/src/main/scala/org/apache/spark/sql/execution/Columnar.scala

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import org.apache.spark.sql.execution.metric.{SQLMetric, SQLMetrics}
3232
import org.apache.spark.sql.execution.vectorized.WritableColumnVector
3333
import org.apache.spark.sql.types._
3434
import org.apache.spark.sql.vectorized.{ColumnarBatch, ColumnVector}
35-
import org.apache.spark.util.Utils
3635

3736
/**
3837
* Holds a user defined rule that can be used to inject columnar implementations of various
@@ -66,9 +65,6 @@ trait ColumnarToRowTransition extends UnaryExecNode
6665
* [[MapPartitionsInRWithArrowExec]]. Eventually this should replace those implementations.
6766
*/
6867
case class ColumnarToRowExec(child: SparkPlan) extends ColumnarToRowTransition with CodegenSupport {
69-
// supportsColumnar requires to be only called on driver side, see also SPARK-37779.
70-
assert(Utils.isInRunningSparkTask || child.supportsColumnar)
71-
7268
override def output: Seq[Attribute] = child.output
7369

7470
override def outputPartitioning: Partitioning = child.outputPartitioning

0 commit comments

Comments
 (0)