andygrove opened a new pull request, #5240:
URL: https://github.com/apache/datafusion-comet/pull/5240
## Which issue does this PR close?
Docs follow-up to review feedback on #5206, which was deferred to keep that
PR's CI
scope small.
## Rationale for this change
#5206 added a section to `understanding-comet-plans.md` listing the nodes
excluded from
the operator counts in the extended explain summary. Review raised two gaps
in that
list.
## What changes are included in this PR?
1. **Both rendered names of `CometSparkToColumnarExec` are listed.**
`nodeName` returns
`CometSparkColumnarToColumnar` when `child.supportsColumnar` is true and
`CometSparkRowToColumnar` otherwise, but the transition-nodes bullet only
named the
latter. Both are already excluded today, because `generateTreeString`
matches on the
Scala type rather than the rendered name, so a reader who sees
`CometSparkColumnarToColumnar` in their own explain output could not tell
from the
list that it was covered. The bullet now names both and says they are one
operator
under two names. This matches the Columnar/Row Transitions table further
down, which
already documents both names.
2. **`ReusedExchange` is now covered, with its caveat.** It is matched in
the same
ignore arm as `AdaptiveSparkPlanExec`, `InputAdapter`,
`WholeStageCodegenExec`,
`QueryStageExec`, and `AQEShuffleReadExec`, but was not mentioned
anywhere in the
section. It also does not behave like those wrappers: `getActualPlan`
unwraps it to
its child, so the reused subtree is counted once per reference rather
than once for
the whole plan. Since the section exists to explain what is excluded and
why, the new
bullet states that the node itself is skipped, spells out the
per-reference
double-counting with a concrete example, and points at item 3 of #5203 as
the
tracking issue.
No prose was removed, and no non-docs files are touched.
## How are these changes tested?
Docs-only. `npx prettier
"docs/source/user-guide/latest/understanding-comet-plans.md"
--check` passes. No new in-page anchor links were added: MyST slugifies the
`Columnar/Row Transitions` heading with the slash dropped, so a
natural-looking
`#columnar-row-transitions` link would have been broken, and the two node
names are
explained inline in the bullet instead.
--
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]
