alamb commented on code in PR #8707:
URL: https://github.com/apache/arrow-datafusion/pull/8707#discussion_r1439073463
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -706,17 +670,10 @@ impl LogicalPlan {
}))
}
},
- LogicalPlan::Window(Window {
- window_expr,
- schema,
- ..
- }) => {
+ LogicalPlan::Window(Window { window_expr, .. }) => {
assert_eq!(window_expr.len(), expr.len());
- Ok(LogicalPlan::Window(Window {
- input: Arc::new(inputs[0].clone()),
- window_expr: expr,
- schema: schema.clone(),
- }))
Review Comment:
maybe @mustafasrepo remembers the context -- I vaguely remember something
like this but can't remember the specifics
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -540,38 +540,6 @@ impl LogicalPlan {
}
}
- /// Returns a copy of this `LogicalPlan` with the new inputs
- pub fn with_new_inputs(&self, inputs: &[LogicalPlan]) ->
Result<LogicalPlan> {
Review Comment:
I suggest we mark this deprecated for at least a release or two to help
users migrate? I always apprecate the hint in the deprecation message that
tells me what to change so I don't have to go poking around in the commit log
to see what happened.
Perhaps it could call through to the `with_new_exprs` API, something like
(untested):
```rust
#[deprecated(since = "35.0.0", note = "please use `with_new_exprs`
instead")]
pub fn with_new_inputs(&self, inputs: &[LogicalPlan]) ->
Result<LogicalPlan> {
self.with_new_exprs(self, self.expressions(), inputs)
}
```
--
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]
