andygrove commented on code in PR #6082:
URL: https://github.com/apache/datafusion-comet/pull/6082#discussion_r4065647528
##########
spark/src/test/scala/org/apache/comet/rules/CometExecRuleSuite.scala:
##########
@@ -1086,4 +1086,30 @@ class CometExecRuleSuite extends CometTestBase {
}
}
+ test("operator conversion alone converts nothing without scan conversion") {
Review Comment:
Good catch, the test was leaning on the default and never touched the class
the PR adds. Rewrote it along the lines you suggested.
One wrinkle worth recording: your version reuses a single plan for both
assertions, and that doesn't work. Fallback reasons are tags on the Spark
nodes, and `CometNativeScan.isSupported` returns false immediately when the
scan already carries one, so the `CometExecRule`-alone pass poisons the
`FileSourceScanExec` and `CometRule` on the same tree then declines the scan
too — the second assertion gets 0. Each direction now gets its own plan from
`createSparkPlan`.
I also checked it isn't vacuous by mutating `CometRule`: it fails when the
scan phase is dropped, and it fails when the two phases are swapped.
--
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]
