andygrove opened a new pull request, #4032:
URL: https://github.com/apache/datafusion-comet/pull/4032

   ## Which issue does this PR close?
   
   Closes #4031.
   
   ## Rationale for this change
   
   Two tests in `CometExecSuite` (`SparkToColumnar eliminate redundant in AQE` 
and `SparkToColumnar override node name for row input`) were skipped on Spark 4 
via `assume(!isSpark40Plus)`. When the guards are removed they both fail with 
`List() had length 0 instead of expected length 1`: the tests look for exactly 
one `CometSparkToColumnarExec` in the final AQE plan and find zero.
   
   The `CometSparkToColumnarExec` insertion is actually working correctly. 
Spark 4 introduced `ResultQueryStageExec`, which extends `LeafExecNode` and now 
wraps the final plan exposed by `AdaptiveSparkPlanExec.executedPlan`. The tests 
used `SparkPlan.collect`, which treats a `LeafExecNode` as opaque and does not 
descend into `QueryStageExec.plan`, so the node was invisible to the assertion.
   
   ## What changes are included in this PR?
   
   - Switch the two affected assertions to use `collect` from 
`AdaptiveSparkPlanHelper` (already mixed into `CometTestBase`). The helper's 
`collect` descends through `AdaptiveSparkPlanExec` and `QueryStageExec.plan` 
via `allChildren`, so it works identically on Spark 3.4 / 3.5 (where the 
top-level plan is not wrapped in a `ResultQueryStageExec`) and Spark 4.
   - Remove the `assume(!isSpark40Plus)` guards that were disabling the tests 
on Spark 4.
   
   ## How are these changes tested?
   
   The two previously skipped tests now run and pass on Spark 4. Verified with:
   
   ```
   ./mvnw test -Pspark-4.0 -Dtest=none 
-Dsuites='org.apache.comet.exec.CometExecSuite SparkToColumnar'
   ```
   
   All 9 `SparkToColumnar*` tests pass. Also verified on the default Spark 3.5 
profile; all 9 still pass.


-- 
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