geoffreyclaude commented on code in PR #17378:
URL: https://github.com/apache/datafusion/pull/17378#discussion_r2318941195
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -2469,6 +2469,20 @@ impl Window {
window_func_dependencies.extend(new_deps);
}
+ // Validate that FILTER if present is only used with aggregate window
functions
Review Comment:
How about simply:
```rust
// Validate that FILTER clauses are only used with aggregate window functions
```
##########
datafusion/proto/src/logical_plan/to_proto.rs:
##########
@@ -317,6 +317,7 @@ pub fn serialize_expr(
// TODO: support null treatment in proto
null_treatment: _,
distinct: _,
+ filter: _,
Review Comment:
Hmm... which project might you be thinking about? ;) I agree. There is
already the `// TODO: support null treatment in proto` above. I hope whoever
implements it will also pick up `distinct` and `filter`!
##########
datafusion/expr/src/expr.rs:
##########
@@ -2993,6 +3006,10 @@ impl Display for SchemaDisplay<'_> {
write!(f, " {null_treatment}")?;
}
+ if let Some(filter) = filter {
+ write!(f, " FILTER (WHERE {filter})")?;
+ }
Review Comment:
good point, `datafusion/sql/src/unparser/expr.rs` is the right place for
this test.
##########
datafusion/sqllogictest/test_files/window.slt:
##########
@@ -5898,3 +5898,115 @@ physical_plan
06)----------RepartitionExec: partitioning=Hash([k@1], 2), input_partitions=2
07)------------ProjectionExec: expr=[CAST(v@1 AS Int64) as __common_expr_1,
k@0 as k, time@2 as time]
08)--------------DataSourceExec: partitions=2, partition_sizes=[5, 4]
+
+
+# FILTER clause with window functions
Review Comment:
nice suggestions ! I'll add them.
--
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]
