andygrove commented on PR #5394: URL: https://github.com/apache/datafusion-comet/pull/5394#issuecomment-5345576046
Both fixed in 2f94707. On the first one — I couldn't find a way to identify the root plan at rule-application time. Subqueries are prepared before the outer plan with nothing to distinguish them, and matching the incoming plan against the root QueryExecution's output breaks down for commands and write plans, which would then get no report at all. So instead of one report owned by the root, each independently planned plan gets one: the outer query plus one per separately prepared subquery. Dedupe is now keyed on execution ID and plan, and AQE's per-stage and post-re-optimization applications are skipped as re-plans of something already reported, so the stage spam the execution-ID cache was there to prevent is still gone. Does that seem like a reasonable trade to you, or would you rather see the root identified even if some plan shapes drop out of reporting? The second one was as you described. The preview now inserts transitions and runs both post-columnar rules. RevertNativeForTransitionHeavyStages needed a new entry point for this: its `apply` takes the AQE branch when AQE is on, which only judges the topmost stage, whereas the preview holds a plan that hasn't been split into stages, so it needs every shuffle boundary visited. The new test builds your regression configuration, reads the coverage out of the captured report, and compares it against `CometCoverageStats` for the plan Comet really executes — before the fix the report claimed 4/4 with no transitions against a real 3/4 with one. What the preview still can't match is AQE re-planning: it describes the pre-adaptive plan and applies the post-columnar rules to it in one pass rather than per stage. That's called out in the user guide alongside the native-planning caveat. -- 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]
