Jimexist commented on a change in pull request #571:
URL: https://github.com/apache/arrow-datafusion/pull/571#discussion_r657099995
##########
File path: datafusion/src/logical_plan/expr.rs
##########
@@ -1452,11 +1452,18 @@ impl fmt::Debug for Expr {
}
Expr::WindowFunction {
fun,
- ref args,
+ args,
+ partition_by,
Review comment:
> It seems like the partition_by and order_by more logically belong on
the LogicalPlan::Window node if they can be shared across Expr::WindowFunction
for logically planned window function node it's not necessarily the case,
because _sort order_ is defined by the concatenations of `partition_by` and
then `order_by`, but consider:
```sql
select max(c1) over (partition by c2 order by c3), max(c1) over (order by
c2, c3) from test;
```
both window functions will have the same sort key of `[c2 asc nulls first,
c3 asc nulls first]` but they mean different things, and they might be
logically planned together (meaning same pre-sorting happens) but not the same
evaluation will happen
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
