findepi commented on code in PR #16615:
URL: https://github.com/apache/datafusion/pull/16615#discussion_r2176902740
##########
datafusion/functions-aggregate/src/first_last.rs:
##########
@@ -69,8 +69,8 @@ pub fn first_value(expression: Expr, order_by:
Option<Vec<SortExpr>>) -> Expr {
}
/// Returns the last value in a group of values.
-pub fn last_value(expression: Expr, order_by: Option<Vec<SortExpr>>) -> Expr {
- if let Some(order_by) = order_by {
+pub fn last_value(expression: Expr, order_by: Vec<SortExpr>) -> Expr {
+ if !order_by.is_empty() {
Review Comment:
This condition seems redundant now.
##########
datafusion/expr/src/expr.rs:
##########
@@ -1175,26 +1175,26 @@ impl Exists {
/// User Defined Aggregate Function
///
-/// See [`udaf::AggregateUDF`] for more information.
+/// See [`crate::udaf::AggregateUDF`] for more information.
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
pub struct AggregateUDF {
/// The function
- pub fun: Arc<udaf::AggregateUDF>,
+ pub fun: Arc<crate::AggregateUDF>,
/// List of expressions to feed to the functions as arguments
pub args: Vec<Expr>,
/// Optional filter
pub filter: Option<Box<Expr>>,
/// Optional ORDER BY applied prior to aggregating
- pub order_by: Option<Vec<Expr>>,
+ pub order_by: Vec<Sort>,
Review Comment:
I understand why the Optional was removed (that's the purpose of this PR). I
don't yet understand why Expr was replaced with Sort thought.
##########
datafusion/functions-aggregate/src/first_last.rs:
##########
@@ -55,8 +55,8 @@ create_func!(FirstValue, first_value_udaf);
create_func!(LastValue, last_value_udaf);
/// Returns the first value in a group of values.
-pub fn first_value(expression: Expr, order_by: Option<Vec<SortExpr>>) -> Expr {
- if let Some(order_by) = order_by {
+pub fn first_value(expression: Expr, order_by: Vec<SortExpr>) -> Expr {
+ if !order_by.is_empty() {
Review Comment:
This condition seems redundant now.
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
