andygrove opened a new issue, #6081:
URL: https://github.com/apache/datafusion-comet/issues/6081
`CometScanRule` and `CometExecRule` are registered as two independent rules,
adjacently, in both the columnar path and the query-stage-prep path:
```scala
extensions.injectColumnar { session => CometScanColumnar(session) }
extensions.injectColumnar { session => CometExecColumnar(session) }
...
extensions.injectQueryStagePrepRule { session => CometScanRule(session) }
extensions.injectQueryStagePrepRule { session => CometExecRule(session) }
```
They are really one pass split in two, held together by registration order:
- Nothing ever runs between them. Both registrations happen back to back
inside one `apply`, so no other extension can interleave.
- Neither is useful on its own. `CometExecRule.transform` seeds its native
chain only from nodes `CometScanRule` produces (`CometScanExec`,
`CometBatchScanExec`, `CometContribScanMarker`), so operator conversion over
unconverted Spark scans converts nothing. Scan conversion is not independently
useful either, since the V1 `CometScanExec` path is itself gated on
`spark.comet.exec.enabled`.
The split leaks into anything that needs the whole conversion rather than
half of it. It is what forces #5394 to widen both rules' internals to package
visibility and thread a `forPreview` flag through `CometExecRule._apply`, so
that one rule can reach into the other and rebuild the conversion for a report.
Proposal: add a `CometRule` that applies scan conversion followed by
operator conversion, and register that instead of the two. Both rules keep
their own classes, files and tests; this only changes how they are sequenced,
making the ordering an invariant of the code rather than of the registration
order.
The two rules predate the file split in #1695, which only moved them into
their own files, so there is no recorded rationale for keeping them separate.
--
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]
