andygrove opened a new pull request, #5696:
URL: https://github.com/apache/datafusion-comet/pull/5696
## Which issue does this PR close?
Closes #5689.
## Rationale for this change
`CometIcebergWriteExec` is tagged `with ColumnarToRowTransition` so that
Spark does not wedge a
`ColumnarToRow` between the write and its Comet-native child. That trait
does more than suppress
one insertion. In Spark's `ApplyColumnarRulesAndInsertTransitions`:
```scala
private def ensureOutputsRowBased(plan: SparkPlan): SparkPlan = {
if (plan.supportsColumnar && !plan.supportsRowBased) {
ColumnarToRowExec(ensureOutputsColumnar(plan))
} else if (!plan.isInstanceOf[ColumnarToRowTransition]) {
plan.withNewChildren(plan.children.map(insertTransitions(_,
outputsColumnar)))
} else {
plan // returned untouched -- the subtree is never visited
}
}
```
`CometIcebergWriteExec` is row-based (`supportsColumnar = false`), so it
lands in the third branch
and the *entire subtree below the write* skips the transition-insertion pass.
The Iceberg copy-on-write rewrite plan feeds a Spark-columnar `BatchScan
(IcebergCopyOnWriteScan)`
into row-based joins and filters, so the `ColumnarToRow` that plan needs is
never inserted and
every CoW DELETE / UPDATE / MERGE fails at runtime with:
```
java.lang.ClassCastException: class
org.apache.spark.sql.vectorized.ColumnarBatch
cannot be cast to class org.apache.spark.sql.catalyst.InternalRow
```
With AQE enabled the failure disappears, because each stage gets its own
insertion pass when it
materialises. Every existing Comet Iceberg suite runs with AQE on, which is
why this was never
caught here; it is the dominant failure in Iceberg's own `spark-extensions`
suites (see #5649),
whose `ExtensionsTestBase` randomises AQE per session.
## What changes are included in this PR?
- `CometIcebergWriteExec`: drop the `ColumnarToRowTransition` trait so Spark
walks the write's
subtree normally and inserts the transitions it needs. The comment
explaining why the node is
not a transition is kept and expanded.
- `EliminateRedundantTransitions`: strip the columnar-to-row transition
Spark now inserts *below*
the write, so `doExecuteColumnar` still sees the columnar child directly.
This mirrors the
existing `ColumnarToRowExec(nativeWrite: CometNativeWriteExec)` arm. The
new
`stripColumnarToRow` helper handles all three variants
(`ColumnarToRowExec` and the two Comet
ones), because `transformUp` has usually already rewritten the plain node
by the time the
parent arm sees it.
The strip is unconditional: `CometIcebergNativeWrite.requiresNativeChildren
= true` already
guarantees the write's child was a `CometNativeExec` at conversion time.
(Guarding it on
`child.isInstanceOf[CometPlan]` is wrong -- under AQE the child is a
`ShuffleQueryStageExec`, a
Spark node wrapping the Comet exchange.)
## How are these changes tested?
New test `native acceleration: ReplaceData (CoW DELETE) with AQE disabled` in
`CometIcebergWriteActionSuite`, plus an `assertColumnarContract` helper that
walks the executed
plan and flags any row-based operator consuming a columnar-only child (the
shape that produces the
`ClassCastException` at runtime rather than a planning error).
- Without the fix, the new test fails with exactly the `ClassCastException`
from the issue, while
the other 53 tests in the suite pass -- so it reproduces the bug rather
than being vacuous.
- With the fix, 275 tests pass across `CometIcebergWriteActionSuite` (54),
`CometIcebergWriteDetectionSuite` (46), `CometIcebergRewriteActionSuite`
(5),
`CometIcebergSystemFunctionSuite` (11), `CometExecSuite` (144) and
`RevertNativeForTransitionHeavyStagesSuite` (15), on the default Spark 4.1
/ Iceberg 1.11.0
profile.
--
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]
