sunchao commented on code in PR #5394:
URL: https://github.com/apache/datafusion-comet/pull/5394#discussion_r3815482134
##########
spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala:
##########
@@ -573,7 +652,31 @@ case class CometExecRule(session: SparkSession)
newPlan
}
- private def _apply(plan: SparkPlan): SparkPlan = {
+ /**
+ * Build the Comet plan we would have executed and log it. Called from
`_apply` in plan-only
+ * mode; the built plan is discarded. Passes `forPreview = true` through the
nested calls so
+ * both rules run their normal transforms instead of short-circuiting.
+ *
+ * Conversion is only the first half of Comet planning. Normally Spark then
inserts the columnar
+ * transitions and runs Comet's post-columnar rules (see
+ * `CometSparkSessionExtensions.CometExecColumnar.postColumnarTransitions`),
which can revert
+ * whole stages back to Spark and drop redundant transitions. Those steps
run here too, so the
+ * report describes the plan that would really have executed and counts the
transitions that
+ * would really have been there, rather than the pre-transition conversion
result.
+ *
+ * `RevertNativeForTransitionHeavyStages` is applied with `applyToAllStages`
because the preview
+ * holds the whole plan at once, whereas under AQE Spark hands that rule one
stage at a time.
+ */
+ private def reportPlanOnlyCoverage(plan: SparkPlan): Unit = {
+ val converted = _apply(CometScanRule(session)._apply(plan), forPreview =
true)
Review Comment:
[P2] Include converted scalar subqueries in the outer coverage report
Even with AQE disabled, an acceleratable scalar subquery is counted as Spark
in the outer report. Its independently converted preview has already been
discarded, and these two conversion passes walk ordinary plan children, leaving
the original `ScalarSubquery.plan` in the outer preview. `ExtendedExplainInfo`
then traverses those expression-owned plans and includes their operators in the
percentage. For the new scalar-subquery test query, the separate subquery
warning can therefore report acceleration that is missing from the outer
query's coverage. A constructed-plan probe using the exact-head
formatter/serializer and real Comet project nodes reports 1/5 with the
untouched subquery versus 2/5 after replacing only its plan. Please carry the
converted subquery previews into the outer preview, or exclude separately
reported subqueries from that report's counts, and compare its coverage with
normal Comet planning.
--
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]
