[CORE][VL] Add LocalTableScanExec offload support to Velox backend - #12631
Conversation
Offload LocalTableScanExec (a driver-side local collection) to native columnar execution on the Velox backend by converting its rows into columnar batches via the existing RowToVeloxColumnar JNI path. - Add gluten-substrait LocalTableScanTransformer base and SparkPlanExecApi hooks (isSupportLocalTableScanExec / getLocalTableScanTransform), wired into OffloadOthers. - Add VeloxLocalTableScanTransformer with schema/Arrow-compatibility validation (falls back for Map/Interval and other unsupported types). - Gate offload behind spark.gluten.sql.columnar.localTableScan (default true, consistent with other columnar operator toggles). - Skip offload for deserialized plans whose @transient rows became null (avoids an NPE when an AQE sub-plan is shipped across an RPC boundary), and for streaming sources via a new SparkShims.getLocalTableScanStream accessor (None on Spark 3.x, plan.stream on Spark 4.0+). - Add unit/integration tests and document the new config. Generated-by: Copilot claude-opus-4.8 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 677ac585-63d4-4657-9fef-89195c8751e8
|
Run Gluten Clickhouse CI on x86 |
There was a problem hiding this comment.
Pull request overview
This PR adds Velox-backend offload support for Spark’s LocalTableScanExec by introducing a transformer that converts local InternalRow collections into native columnar batches (via the existing Velox row-to-columnar JNI path), wiring it into the single-node offload rule, and gating it behind a new SQL config.
Changes:
- Introduce a new
LocalTableScanTransformerbase + backend API hooks to enable/construct backend-specific replacements forLocalTableScanExec. - Add Velox implementation (
VeloxLocalTableScanTransformer) with schema/Arrow-compatibility validation and config gating, including shims to detect Spark 4.x streaming sources. - Add Velox test suite coverage and document the new
spark.gluten.sql.columnar.localTableScanconfig.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| shims/spark41/src/main/scala/org/apache/gluten/sql/shims/spark41/Spark41Shims.scala | Exposes LocalTableScanExec.stream (Spark 4.1) via a shim accessor. |
| shims/spark40/src/main/scala/org/apache/gluten/sql/shims/spark40/Spark40Shims.scala | Exposes LocalTableScanExec.stream (Spark 4.0) via a shim accessor. |
| shims/common/src/main/scala/org/apache/gluten/sql/shims/SparkShims.scala | Adds cross-version shim hook for LocalTableScan streaming-source detection (defaults to None). |
| gluten-substrait/src/main/scala/org/apache/spark/sql/execution/LocalTableScanTransformer.scala | Adds a transformer base + companion helpers to route backend support/creation. |
| gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/offload/OffloadSingleNodeRules.scala | Wires LocalTableScanExec into the “others” offload rule when backend reports support. |
| gluten-substrait/src/main/scala/org/apache/gluten/config/GlutenConfig.scala | Adds spark.gluten.sql.columnar.localTableScan config and accessor. |
| gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala | Adds backend API hooks (isSupportLocalTableScanExec, getLocalTableScanTransform). |
| docs/Configuration.md | Documents the new spark.gluten.sql.columnar.localTableScan toggle. |
| backends-velox/src/test/scala/org/apache/gluten/execution/VeloxLocalTableScanSuite.scala | Adds Velox-side test coverage for offload and fallback scenarios. |
| backends-velox/src/main/scala/org/apache/gluten/execution/VeloxLocalTableScanTransformer.scala | Implements Velox local-table-scan offload via driver materialization + JNI row-to-columnar conversion. |
| backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala | Enables Velox support checks + transformer creation, including streaming-source and null-transient-row guards. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (rows.isEmpty) { | ||
| sparkContext.emptyRDD[ColumnarBatch] | ||
| } else { | ||
| // Materialize rows as UnsafeRow on the driver, then parallelize |
There was a problem hiding this comment.
Good catch — fixed in b2323f7. doExecuteColumnar() now guards rows before use: if it's null (the deserialized @transient case) it throws an IllegalStateException with a clear message explaining the plan should never have been offloaded, instead of a bare NPE. Since offload is already blocked at planning time in isSupportLocalTableScanExec (which returns false for null rows), this is a defensive execution-time backstop; IllegalStateException matches the invariant-violation convention used elsewhere in this package (e.g. GenerateExecTransformer, VeloxSerializedBroadcastRDD).
| } | ||
| } | ||
|
|
||
| logInfo( |
There was a problem hiding this comment.
Done in b2323f7 — downgraded the per-node validation-success log from logInfo to logDebug. doValidateInternal() runs per plan node per query, so INFO was too noisy for production; DEBUG keeps it available for diagnostics without the spam.
| // A streaming source (Spark 4.0+ only) must keep vanilla execution. | ||
| if (SparkShimLoader.getSparkShims.getLocalTableScanStream(plan).isDefined) { | ||
| logDebug("LocalTableScan offload skipped: streaming source detected") | ||
| return false | ||
| } |
There was a problem hiding this comment.
Addressed in b2323f7 by adding explicit test coverage for the streaming path:
- Spark 4.x streaming-detected path — new
Spark40LocalTableScanStreamSuiteandSpark41LocalTableScanStreamSuiteconstruct aLocalTableScanExeccarrying a stream and assertgetLocalTableScanStream(plan).isDefined(plusNonefor a batch plan). These live in theshims/spark40andshims/spark41modules because thestreamconstructor parameter is Spark 4.x-only and cannot compile in the shared cross-versionVeloxLocalTableScanSuite. - Version-agnostic guard — added a case to
VeloxLocalTableScanSuiteasserting a batchLocalTableScanExecis classified as non-streaming (None) and is therefore not falsely skipped by the streaming guard, on every supported Spark version.
Both shim suites pass locally (2/2 each) under -Pspark-4.0 and -Pspark-4.1.
|
Run Gluten Clickhouse CI on x86 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
backends-velox/src/main/scala/org/apache/gluten/execution/VeloxLocalTableScanTransformer.scala:70
- This logs a validation-success message at INFO for every LocalTableScan offload, which can be very noisy in production (local relations can appear frequently, e.g. in AQE/broadcast-related plans). Consider lowering this to DEBUG to avoid inflating driver logs without losing troubleshooting capability.
logInfo(
s"local_table_scan native validation succeeded: " +
s"schema=${schema.fields.map(_.dataType.simpleString).mkString(",")}, " +
s"appId=${sparkContext.applicationId}")
backends-velox/src/main/scala/org/apache/gluten/execution/VeloxLocalTableScanTransformer.scala:108
rows.map(...).toArraymaterializes and copies the full local relation as UnsafeRows on the driver before parallelizing. This duplicates memory (rows already exist on the driver) and can significantly increase driver heap pressure for large local relations. It’s safer to parallelize the original rows first and project/copy per-partition on executors (and also guard againstrows == nullin case the transformer is ever serialized).
if (rows.isEmpty) {
sparkContext.emptyRDD[ColumnarBatch]
} else {
// Materialize rows as UnsafeRow on the driver, then parallelize
val proj = UnsafeProjection.create(outputAttributes, outputAttributes)
val unsafeRows = rows.map(r => proj(r).copy()).toArray
- Guard against null @transient rows in VeloxLocalTableScanTransformer .doExecuteColumnar with a clear IllegalStateException instead of a bare NPE. - Lower per-node validation-success log from INFO to DEBUG to avoid log spam. - Add a version-agnostic test asserting a batch LocalTableScanExec is not classified as a streaming source (not skipped by the streaming guard). - Add Spark 4.0/4.1 shim suites covering the streaming-source detection path (getLocalTableScanStream returns the stream), which cannot live in the shared cross-version suite because the `stream` ctor param is Spark 4.x-only. Generated-by: GitHub Copilot CLI claude-opus-4.8 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a4ac0059-8498-4138-bdeb-0c66249bde12
|
Run Gluten Clickhouse CI on x86 |
LocalTableScanExec gained a required `stream` parameter on Spark 4.0, so
the direct 2-arg constructor calls in the suite failed to compile under
the Spark 4.0 profile ("Unspecified value parameter stream").
Build the plan through the physical planner via a version-agnostic
`newBatchLocalTableScan()` helper instead of calling the constructor,
keeping the suite compilable on all supported Spark versions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a4ac0059-8498-4138-bdeb-0c66249bde12
|
Run Gluten Clickhouse CI on x86 |
The generated docs/Configuration.md row for spark.gluten.sql.columnar.localTableScan carried two extra trailing padding spaces, causing the "Check gluten configs" test to fail (generated output vs committed file mismatch on this line). Trim the row to the correct column width so it matches gen-all-config-docs.sh output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a4ac0059-8498-4138-bdeb-0c66249bde12
|
Run Gluten Clickhouse CI on x86 |
The LocalTableScanExec offload defaulted to enabled, which changed plan shapes across the whole test suite and caused deterministic CI failures: - MiscOperatorSuite "RowToVeloxColumnar preferredBatchBytes": with offload on, the local scan produces columnar batches itself, so no RowToVeloxColumnarExec node is inserted and the assertion on its count fails. Pin the offload off in that test so it keeps exercising the RowToVeloxColumnarExec batching path it targets. - VeloxParquetWriteForHiveSuite hive VALUES write (Spark 3.3): when the scan is offloaded but the parent projection falls back to vanilla row execution (AnsiCast is not mappable to Substrait on 3.3), the inserted VeloxColumnarToRowExec (not CodegenSupport) sits under FileFormatWriter whole-stage codegen with no InputAdapter, throwing a ClassCastException. Make the feature opt-in (default false) so existing behavior is preserved, and document the write-path codegen limitation. The feature stays fully covered by VeloxLocalTableScanSuite, which enables the config explicitly. The codegen-safe write path is left as a follow-up before flipping the default back on. Generated-by: Copilot claude-opus-4.8
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (3)
backends-velox/src/main/scala/org/apache/gluten/execution/VeloxLocalTableScanTransformer.scala:118
- This always runs an
UnsafeProjectionfor every row, even if a row is already anUnsafeRow. SinceLocalTableScanExecrows can already be unsafe, you can avoid unnecessary projection work by copyingUnsafeRowinputs directly (similar toRowToVeloxColumnarExec).
// Materialize rows as UnsafeRow on the driver, then parallelize
val proj = UnsafeProjection.create(outputAttributes, outputAttributes)
val unsafeRows = rows.map(r => proj(r).copy()).toArray
gluten-substrait/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:930
- The PR description says
spark.gluten.sql.columnar.localTableScandefaults totrue, but this config is created with a default offalse(and the docs entry also showsfalse). Please reconcile the PR description vs. the shipped default (either update the description or flip the default once the codegen-safe transition issue is resolved).
"attempts to replace LocalTableScanExec (a driver-side local collection) with a " +
backends-velox/src/test/scala/org/apache/gluten/execution/VeloxLocalTableScanSuite.scala:101
- The empty-collection case only asserts correctness via Spark fallback (
checkAnswer), but doesn’t assert that the new LocalTableScan offload path is actually exercised. Adding anassertHasVeloxLocalTableScan(df)here would cover therows.isEmptyexecution branch inVeloxLocalTableScanTransformer.doExecuteColumnar()and guard against regressions where empty local scans silently stop offloading.
test("LocalTableScan with empty collection") {
val schema = StructType(Seq(StructField("id", IntegerType), StructField("name", StringType)))
val df = createDF(Seq.empty, schema)
checkAnswer(df, Seq.empty[Row])
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.
Suppressed comments (1)
gluten-substrait/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:934
- The PR description says
spark.gluten.sql.columnar.localTableScandefaults totrue, but this config entry is created withcreateWithDefault(false)(and docs also showfalse). Please align the PR description with the actual default (or change the default here iftrueis intended).
val COLUMNAR_LOCAL_TABLE_SCAN_ENABLED =
// NOTE: Disabled by default. When an offloaded local scan feeds an operator that falls back
// to vanilla row execution under the write path, the inserted columnar-to-row transition is
// not yet codegen-safe (VeloxColumnarToRowExec is not CodegenSupport), which can fail
// FileFormatWriter codegen. Flip the default to true once that path is handled.
buildConf("spark.gluten.sql.columnar.localTableScan")
.doc(
"Enable or disable native columnar execution of LocalTableScanExec. When true, Gluten " +
"attempts to replace LocalTableScanExec (a driver-side local collection) with a " +
"backend transformer that converts the rows into columnar batches natively.")
.booleanConf
.createWithDefault(false)
|
Note on the default value ( For this PR the config defaults to The feature itself is complete and fully covered by tests (which enable the config explicitly). This limitation will be addressed in a follow-up PR that makes the transition codegen-safe — reverting a no-op offload that is immediately topped by a columnar-to-row transition back to a vanilla Keeping the default off here lets this PR land safely while the codegen-safe handling is reviewed separately. |
What changes were proposed in this pull request?
Offload
LocalTableScanExec(a driver-side local collection, e.g. fromSeq(...).toDF,VALUES, or a smallLocalRelation) to native columnar execution on the Velox backend by converting its rows into columnar batches via the existingRowToVeloxColumnarnative row-to-columnar path.LocalTableScanTransformerbase andSparkPlanExecApihooks (isSupportLocalTableScanExec/getLocalTableScanTransform), wired intoOffloadOthers.VeloxLocalTableScanTransformerwith schema/Arrow-compatibility validation (falls back for Map/Interval and other unsupported types).spark.gluten.sql.columnar.localTableScan.@transientrows became null (avoids an NPE when an AQE sub-plan is shipped across an RPC boundary), and for streaming sources via a newSparkShims.getLocalTableScanStreamaccessor (Noneon Spark 3.x,plan.streamon Spark 4.0+).Default is
falsefor now (known write-path limitation)The config currently defaults to
false. When an offloaded local scan feeds an operator that falls back to vanilla row execution under the write path, the inserted columnar-to-row transition is not yet codegen-safe:VeloxColumnarToRowExecdoes not implementCodegenSupport, soFileFormatWriter's whole-stage codegen can cast it toCodegenSupportand fail with aClassCastException(reproducible on Spark 3.3 viaINSERT ... VALUESwhose casts become anAnsiCastthat falls back).The feature itself is complete and fully covered by tests (which enable the config explicitly). A follow-up will make the transition codegen-safe — reverting a no-op offload that is immediately topped by a columnar-to-row transition back to a vanilla
LocalTableScanExec— and flip the default totrue.How was this patch tested?
VeloxLocalTableScanSuitecovering: successful offload, fallback when disabled, fallback for unsupported (Map/Interval) types, the deserialized-null-rows NPE guard, and streaming-source skip. The suite setsspark.gluten.sql.columnar.localTableScan=trueexplicitly, so it is independent of the default.MiscOperatorSuite'sRowToVeloxColumnar preferredBatchBytestest pins the config off so its plan expectations are unaffected by this operator.