Apply suggestions from code review

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
apache · Jun 9, 2021 · 1b38ae2 · 1b38ae2
1 parent 3b289d8
commit 1b38ae2
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 28 deletions.
diff --git a/datafusion/src/optimizer/utils.rs b/datafusion/src/optimizer/utils.rs
@@ -374,7 +374,7 @@ pub fn rewrite_expression(expr: &Expr, expressions: &[Expr]) -> Result<Expr> {
 
             if partition_index >= sort_index {
                 Err(DataFusionError::Internal(
-                    "Ill-formed window function expressions".to_owned(),
+                    "Ill-formed window function expressions: partition index too large".to_owned(),
                 ))
             } else {
                 Ok(Expr::WindowFunction {

diff --git a/datafusion/src/sql/planner.rs b/datafusion/src/sql/planner.rs
@@ -1139,35 +1139,35 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
                         .as_ref()
                         .map(|window_frame| window_frame.clone().try_into())
                         .transpose()?;
-                    let fun = window_functions::WindowFunction::from_str(&name);
-                    if let Ok(window_functions::WindowFunction::AggregateFunction(
-                        aggregate_fun,
-                    )) = fun
-                    {
-                        return Ok(Expr::WindowFunction {
-                            fun: window_functions::WindowFunction::AggregateFunction(
-                                aggregate_fun.clone(),
-                            ),
-                            args: self.aggregate_fn_to_expr(&aggregate_fun, function)?,
-                            partition_by,
-                            order_by,
-                            window_frame,
-                        });
-                    } else if let Ok(
+                    let fun = window_functions::WindowFunction::from_str(&name)?;
+                    match fun {
+                        window_functions::WindowFunction::AggregateFunction(
+                            aggregate_fun,
+                        ) => {
+                            return Ok(Expr::WindowFunction {
+                                fun: window_functions::WindowFunction::AggregateFunction(
+                                    aggregate_fun.clone(),
+                                ),
+                                args: self
+                                    .aggregate_fn_to_expr(&aggregate_fun, function)?,
+                                partition_by,
+                                order_by,
+                                window_frame,
+                            });
+                        }
                         window_functions::WindowFunction::BuiltInWindowFunction(
                             window_fun,
-                        ),
-                    ) = fun
-                    {
-                        return Ok(Expr::WindowFunction {
-                            fun: window_functions::WindowFunction::BuiltInWindowFunction(
-                                window_fun,
-                            ),
-                            args: self.function_args_to_expr(function)?,
-                            partition_by,
-                            order_by,
-                            window_frame,
-                        });
+                        ) => {
+                            return Ok(Expr::WindowFunction {
+                                fun: window_functions::WindowFunction::BuiltInWindowFunction(
+                                    window_fun,
+                                ),
+                                args: self.function_args_to_expr(function)?,
+                                partition_by,
+                                order_by,
+                                window_frame,
+                            });
+                        }
                     }
                 }