Copilot commented on code in PR #12766:
URL: https://github.com/apache/gluten/pull/12766#discussion_r3781108501
##########
gluten-substrait/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##########
@@ -935,6 +937,16 @@ object GlutenConfig extends ConfigRegistry {
.booleanConf
.createWithDefault(true)
+ val COLUMNAR_EMPTY_RELATION_ENABLED =
+ buildConf("spark.gluten.sql.columnar.emptyRelation")
+ .doc(
+ "Enable or disable native columnar execution of EmptyRelationExec
(Spark 4.0+). When " +
+ "true, Gluten replaces EmptyRelationExec (a leaf node AQE creates
when it proves a " +
+ "subtree produces no output) with a columnar transformer, avoiding
unnecessary " +
+ "ColumnarToRow / RowToColumnar transitions around the empty
relation.")
Review Comment:
The config doc string says "native columnar execution", but the transformer
currently just returns an empty RDD[ColumnarBatch] on the JVM (no native engine
execution). To avoid user confusion, consider describing this as "columnar
execution" / "columnar transformer" rather than "native".
##########
gluten-substrait/src/main/scala/org/apache/spark/sql/execution/EmptyRelationExecTransformer.scala:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.spark.sql.execution
+
+import org.apache.gluten.backendsapi.BackendsApiManager
+import org.apache.gluten.execution.{ValidatablePlan, ValidationResult}
+import org.apache.gluten.extension.columnar.transition.Convention
+
+import org.apache.spark.rdd.RDD
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.expressions.Attribute
+import org.apache.spark.sql.vectorized.ColumnarBatch
+
+/**
+ * Columnar-aware replacement for Spark's EmptyRelationExec (Spark 4.0+). It
produces an empty
+ * RDD[ColumnarBatch] so that surrounding columnar operators do not need to be
wrapped in
+ * unnecessary ColumnarToRow / RowToColumnar transitions when AQE propagates
an empty relation
+ * through the plan.
+ */
+case class EmptyRelationExecTransformer(output: Seq[Attribute]) extends
ValidatablePlan {
+
+ override def rowType(): Convention.RowType = Convention.RowType.None
+
+ override def batchType(): Convention.BatchType =
BackendsApiManager.getSettings.primaryBatchType
+
+ override protected def doValidateInternal(): ValidationResult =
ValidationResult.succeeded
+
+ override protected def doExecute(): RDD[InternalRow] =
+ throw new UnsupportedOperationException(
+ "EmptyRelationExecTransformer does not support row execution.")
+
+ override protected def doExecuteColumnar(): RDD[ColumnarBatch] =
+ sparkContext.emptyRDD[ColumnarBatch]
+
+ override def children: Seq[SparkPlan] = Seq.empty
+
+ override protected def withNewChildrenInternal(
+ newChildren: IndexedSeq[SparkPlan]): SparkPlan = this
+}
+
+object EmptyRelationExecTransformer {
+
+ /**
+ * Whether the backend supports offloading the given empty-relation plan to
native. The plan is
+ * typed as [[SparkPlan]] because EmptyRelationExec only exists on Spark
4.0+; callers must first
+ * confirm the type through `SparkShims.isEmptyRelationExec`.
+ */
Review Comment:
The helper Scaladoc says "offloading ... to native", but
EmptyRelationExecTransformer is currently a JVM-side columnar leaf (empty
RDD[ColumnarBatch]). Consider rewording to "offloading ... to a columnar
transformer" to match behavior.
##########
gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala:
##########
@@ -655,6 +655,17 @@ trait SparkPlanExecApi {
def getRDDScanTransform(plan: RDDScanExec): RDDScanTransformer =
throw new GlutenNotSupportException("RDDScanExec is not supported")
+ /**
+ * Whether the backend supports offloading the given empty-relation plan to
native. Typed as
+ * [[SparkPlan]] because EmptyRelationExec only exists on Spark 4.0+;
callers must first confirm
+ * the type through `SparkShims.isEmptyRelationExec`.
+ */
Review Comment:
The API Scaladoc says "offloading ... to native", but the current
EmptyRelationExecTransformer implementation is a JVM-side columnar leaf that
returns an empty RDD[ColumnarBatch]. Consider clarifying the wording to
"offloading to a columnar transformer" (backend-gated) to avoid implying native
execution.
##########
docs/Configuration.md:
##########
@@ -55,6 +55,7 @@ nav_order: 15
| spark.gluten.sql.columnar.coalesce | 🔄
Dynamic | true | Enable or disable columnar coalesce.
|
| spark.gluten.sql.columnar.collectLimit | 🔄
Dynamic | true | Enable or disable columnar collectLimit.
|
| spark.gluten.sql.columnar.collectTail | 🔄
Dynamic | true | Enable or disable columnar collectTail.
|
+| spark.gluten.sql.columnar.emptyRelation | 🔄
Dynamic | true | Enable or disable native columnar execution of
EmptyRelationExec (Spark 4.0+). When true, Gluten replaces EmptyRelationExec
with a columnar transformer, avoiding unnecessary ColumnarToRow / RowToColumnar
transitions around the empty relation.
|
Review Comment:
This configuration table entry says "native columnar execution", but
EmptyRelationExecTransformer currently produces an empty ColumnarBatch RDD on
the JVM (no native engine execution). Consider updating the wording to
"columnar execution" to match behavior.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
