andygrove commented on code in PR #5696:
URL: https://github.com/apache/datafusion-comet/pull/5696#discussion_r3941229931
##########
spark/src/test/scala/org/apache/comet/CometIcebergWriteActionSuite.scala:
##########
@@ -1688,6 +1741,31 @@ class CometIcebergWriteActionSuite
assume(icebergAvailable, "Iceberg not available in classpath")
}
+ /**
+ * Every row-consuming operator must receive row-based input. Spark
guarantees that by inserting
+ * `ColumnarToRow` transitions in `ApplyColumnarRulesAndInsertTransitions`;
an operator that
+ * Comet rewrote in a way that skips the insertion pass shows up here as a
row-based node with a
+ * columnar-only child, and would fail at runtime with a `ColumnarBatch
cannot be cast to
+ * InternalRow` `ClassCastException` rather than at planning time.
+ *
+ * `CometIcebergWriteExec` is the one legitimate exception: it is row-based
on the outside but
+ * pulls Arrow batches from its Comet-native child over FFI (see the class
docstring), so the
+ * transition below it is deliberately stripped again by
`EliminateRedundantTransitions`.
+ */
+ private def assertColumnarContract(plan: SparkPlan): Unit = {
Review Comment:
Done in bc6429f. `captureWrite` now runs `assertColumnarContract` over every
captured plan, and the explicit call in the CoW DELETE test is gone as
redundant.
Also added the other two CoW shapes with AQE off. One thing that came out of
it: the partitioned MERGE does engage natively, unlike the unpartitioned one
the existing test pins. Partitioning puts a `CometColumnarExchange`
(`REBALANCE_PARTITIONS_BY_COL`) between `MergeRowsExec` and the write, so the
write's own child is Comet-native and `requiresNativeChildren` is satisfied
even though `MergeRowsExec` stays JVM. Its plan is the strongest of the three
for this issue, since the subtree needs transitions in three separate places:
```
CometIcebergWrite
+- CometColumnarExchange hashpartitioning(region, 10),
REBALANCE_PARTITIONS_BY_COL
+- MergeRowsExec
+- *(4) CometColumnarToRow
+- CometSortMergeJoin FullOuter
:- CometSort
: +- CometColumnarExchange
: +- *(1) Project
: +- *(1) ColumnarToRow
: +- BatchScan IcebergCopyOnWriteScan
...
```
The test asserts native engagement rather than fallback as a result.
--
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]
