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]

Reply via email to