comphead commented on code in PR #6852:
URL: https://github.com/apache/datafusion-comet/pull/6852#discussion_r4379457445
##########
spark/src/test/scala/org/apache/comet/rules/RevertNativeForTransitionHeavyStagesSuite.scala:
##########
@@ -803,6 +842,104 @@ class RevertNativeForTransitionHeavyStagesSuite extends
CometTestBase {
}
}
+ // https://github.com/apache/datafusion-comet/issues/6687
+ test("transition-heavy revert restores columnar output for a cached AQE
result stage") {
Review Comment:
The new `outputColumnar = !plan.supportsRowBased` branch for a columnar-only
root is only exercised by this test, which is skipped before Spark 4.0, and the
suite has no assertion on `RowToColumnarExec` (the only mentions are in a doc
comment). The branch itself is version independent, so the 3.4 and 3.5 profiles
never run it. Could a small rule-level case cover it, like the
`BothFormatsExec` test already does for the dual-format root? For example,
apply the rule to
`CometSparkToColumnarExec(CometNativeColumnarToRowExec(cometFilter(leaf)))`,
which is columnar-only with one transition, and assert a `RowToColumnarExec`
over a reverted root with no Comet execs left, plus the matching row case where
a `ColumnarToRowExec` root stays row output. I did not run the suite.
--
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]