alamb opened a new pull request, #22415: URL: https://github.com/apache/datafusion/pull/22415
## Which issue does this PR close? - Addresses feedback from https://github.com/apache/datafusion/issues/21080#issuecomment-4507775224 ## Rationale for this change As @milenkovicm noted after upgrading Ballista, the upgrade guide for the new required `ExecutionPlan::apply_expressions` method explains *what* the method does but not *why* downstream implementors need to implement it. Without that motivation, it's not obvious whether a no-op `Ok(TreeNodeRecursion::Continue)` is safe, or what the consequences of skipping expressions are. This PR tightens the section and adds a brief explanation of the purpose of the method (visiting every `PhysicalExpr` owned by the node so optimizer passes/analyzers — e.g. dynamic filter discovery — can see them). ## What changes are included in this PR? `docs/source/library-user-guide/upgrading/54.0.0.md`: - Replaces the opening two-sentence restatement of "required method" with a single, more direct paragraph that also names the purpose (analyze/rewrite expressions, e.g. dynamic filter discovery). - Drops "Add `apply_expressions` to your implementation" from the migration intro since the section header already says it is required. - Simplifies the heading of the `output_ordering()` example. ## Are these changes tested? No tests — documentation-only change. ## Are there any user-facing changes? Yes — clarifies the upgrade guide for users implementing custom `ExecutionPlan`, `FileSource`, or `DataSource` traits. No API change. -- 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]
