kosiew commented on code in PR #23063:
URL: https://github.com/apache/datafusion/pull/23063#discussion_r3755147586
##########
datafusion/expr/src/expr.rs:
##########
@@ -930,21 +930,32 @@ pub struct ScalarFunction {
pub func: Arc<crate::ScalarUDF>,
/// List of expressions to feed to the functions as arguments
pub args: Vec<Expr>,
+ /// Original source code location, if known
+ pub spans: Spans,
Review Comment:
I would advise:
1. **Separate the work.** Keep this PR patch-compatible: no `spans` fields
on `ScalarFunction`, `AggregateFunction`, or `WindowFunction`. The
major-version PR should contain only the API/layout change and the propagation
required for it, rather than bundling a diagnostic behavior change with a
release break.
2. **Coordinate the release boundary first.** Ask maintainers/release owners
to confirm the target major release and its versioning plan. The version in the
relevant public crate manifest(s) must be advanced as part of that coordinated
major-release work. [Version 55 is in progress
already](https://github.com/apache/datafusion/issues/22393), so this may have
to wait till version 56.
3. **Make the break explicit.** State in the PR description and release
notes that public struct literals are source-incompatible. Show the downstream
migration:
```rust
// Before
ScalarFunction { func, args }
// After (if a literal is still required)
ScalarFunction {
func,
args,
spans: Spans::new(),
}
// Prefer the stable constructor where it expresses the use case
ScalarFunction::new_udf(func, args)
```
Equivalent literal users of `AggregateFunction` and `WindowFunction` need
the same update. Add or retain constructors that initialize empty spans so
normal programmatic expression construction does not need parser metadata.
--
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]
