jorgecarleitao commented on a change in pull request #334:
URL: https://github.com/apache/arrow-datafusion/pull/334#discussion_r636596927
##########
File path: datafusion/src/logical_plan/builder.rs
##########
@@ -289,6 +288,52 @@ impl LogicalPlanBuilder {
}))
}
+ /// Apply a window
+ ///
+ /// NOTE: this feature is under development and this API will be changing
+ ///
+ /// - https://github.com/apache/arrow-datafusion/issues/359 basic structure
+ /// - https://github.com/apache/arrow-datafusion/issues/298 empty over
clause
+ /// - https://github.com/apache/arrow-datafusion/issues/299 with partition
clause
+ /// - https://github.com/apache/arrow-datafusion/issues/360 with order by
+ /// - https://github.com/apache/arrow-datafusion/issues/361 with window
frame
+ pub fn window(
+ &self,
+ window_expr: impl IntoIterator<Item = Expr>,
Review comment:
I am not sure we should use `IntoIterator<Item = Expr>` for every field
with 6 fields. This will produce a version of the compiled function for every
combination, which IMO adds an unnecessary compile time and binary size.
`IntoIterator` is more relevant when we want to avoid an extra allocation;
these are very small vectors.
##########
File path: datafusion/src/physical_plan/aggregates.rs
##########
@@ -183,7 +182,7 @@ static TIMESTAMPS: &[DataType] = &[
];
/// the signatures supported by the function `fun`.
-fn signature(fun: &AggregateFunction) -> Signature {
+pub fn signature(fun: &AggregateFunction) -> Signature {
Review comment:
`pub(super)` or `pub(crate)` instead?
--
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:
us...@infra.apache.org
