pepijnve commented on code in PR #21739: URL: https://github.com/apache/datafusion/pull/21739#discussion_r3167896282
########## docs/source/library-user-guide/upgrading/54.0.0.md: ########## @@ -25,6 +25,34 @@ in this section pertains to features and changes that have already been merged to the main branch and are awaiting release in this version. +### `AggregateFunctionExpr::human_display()` now returns `Option<&str>` + +`datafusion_physical_expr::aggregate::AggregateFunctionExpr::human_display()` +now returns `Option<&str>` instead of `&str`. + +If your code read the display text directly, handle the `None` case and fall +back to `name()` when needed: + +```rust +let display = agg_expr.human_display().unwrap_or(agg_expr.name()); +``` + +### Physical `EXPLAIN` now shows lowered aggregate execution forms + +Physical `EXPLAIN` now prefers the lowered aggregate expression that the +engine executes, while still keeping the visible output alias. + +Examples: + +- `count(*)` may now appear as `count(1) as count(*)` +- simplified aggregates may show the lowered implementation, such as + `min(...) as percentile_cont(...)` +- internal aggregate aliases may now show the underlying expression instead of + only the alias name + +If you have tooling that parses the `aggr=[...]` text from physical `EXPLAIN`, Review Comment: Are people actually doing this? I guess it might be necessary sometimes, but that seems extremely brittle. The more general is if the explain output is considered part of the stable API of DataFusion or not. -- 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]
