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

   ## Which issue does this PR close?
   
   Closes #5335. Alternate approach to #5345, based on review discussion there.
   
   Heads up: I used an LLM to help draft this. The design is mine, but the code 
and prose have been shaped with LLM assistance, so review with that in mind.
   
   ## Rationale for this change
   
   Users evaluating Comet on a workload need a way to estimate how much of it 
Comet would accelerate without actually changing execution. Turning Comet on 
and comparing runs carries real risk. This adds a mode that builds the Comet 
plan we would have executed, logs it, and then lets Spark run the query 
unchanged.
   
   ## What changes are included in this PR?
   
   - New config `spark.comet.explain.planOnly.enabled`. Default off.
   - When set, `CometScanRule` and `CometExecRule` short-circuit and return the 
plan untouched.
   - `CometExecRule` builds a preview by rerunning both rules against a 
scan-wrapped copy of the plan (using thread-local bypass flags to force normal 
behavior on the nested call), logs the resulting Comet plan and coverage 
summary at WARN level, then returns the original Spark plan.
   - The WARN is deduped per SQL execution ID so under AQE a query gets one 
report, not one per stage.
   - Doc section in `understanding-comet-plans.md`.
   
   The estimate is Scala-side only. The native plan is never handed to 
DataFusion, so anything that would have failed inside DataFusion still counts 
as accelerated. That is called out in the config docstring and the user guide.
   
   ## How are these changes tested?
   
   New tests in `CometExecRuleSuite` cover:
   
   - V1 and V2 Parquet scans, each with AQE on and off.
   - Scalar subquery, so the recursive preview builds through the subquery plan 
without leaving any Comet operators behind in the executed plan.
   - Same query with the config off, asserting Comet operators do appear 
(sanity check that we haven't accidentally disabled Comet).
   
   Each test asserts the executed plan has zero `CometPlan` operators and the 
query results match a plain Spark run.


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