fix: max_by min_by

src/max_min_by.rs

@@ -95,7 +95,7 @@ impl logical_expr::AggregateUDFImpl for MaxByFunction {
                 aggr_func.params.args.remove(1),
                 aggr_func.params.args.remove(0),
             );
-            let sort = logical_expr::expr::Sort::new(second_arg, true, false);
+            let sort = logical_expr::expr::Sort::new(second_arg, true, true);
             order_by.push(sort);
             let func = logical_expr::expr::Expr::AggregateFunction(
                 logical_expr::expr::AggregateFunction::new_udf(
@@ -193,7 +193,7 @@ impl logical_expr::AggregateUDFImpl for MinByFunction {
                 aggr_func.params.args.remove(0),
             );
 
-            let sort = logical_expr::expr::Sort::new(second_arg, false, false);
+            let sort = logical_expr::expr::Sort::new(second_arg, false, true);
             order_by.push(sort); // false for ascending sort
             let func = logical_expr::expr::Expr::AggregateFunction(
                 logical_expr::expr::AggregateFunction::new_udf(
@@ -325,6 +325,7 @@ mod tests {
 
     #[cfg(test)]
     mod max_by {
+
         use super::*;
 
         #[tokio::test]
@@ -387,6 +388,41 @@ mod tests {
             Ok(())
         }
 
+        #[tokio::test]
+        async fn test_max_by_ignores_nulls() -> error::Result<()> {
+            let query = r#"
+                SELECT max_by(v, k)
+                FROM (
+                    VALUES
+                        ('a', 1),
+                        ('b', CAST(NULL AS INT)),
+                        ('c', 2)
+                ) AS t(v, k)
+            "#;
+            let df = ctx()?.sql(&query).await?;
+            let result = extract_single_value::<String, arrow::array::StringArray>(df).await?;
+            assert_eq!(result, "c", "max_by should ignore NULLs");
+            Ok(())
+        }
+
+        #[tokio::test]
+        async fn test_max_like_main_test() -> error::Result<()> {
+            let query = r#"
+                SELECT max_by(v, k)
+                FROM (
+                    VALUES
+                        (1, 10),
+                        (2, 5),
+                        (3, 15),
+                        (4, 8)
+                ) AS t(v, k)
+            "#;
+            let df = ctx()?.sql(&query).await?;
+            let result = extract_single_value::<i64, arrow::array::Int64Array>(df).await?;
+            assert_eq!(result, 3);
+            Ok(())
+        }
+
         fn ctx() -> error::Result<prelude::SessionContext> {
             let ctx = test_ctx()?;
             let max_by_udaf = MaxByFunction::new();
@@ -460,6 +496,59 @@ mod tests {
             Ok(())
         }
 
+        #[tokio::test]
+        async fn test_min_by_ignores_nulls() -> error::Result<()> {
+            let query = r#"
+                SELECT min_by(v, k)
+                FROM (
+                    VALUES
+                        ('a', 1),
+                        ('b', CAST(NULL AS INT)),
+                        ('c', 2)
+                ) AS t(v, k)
+            "#;
+            let df = ctx()?.sql(&query).await?;
+            let result = extract_single_value::<String, arrow::array::StringArray>(df).await?;
+            assert_eq!(result, "a", "min_by should ignore NULLs");
+            Ok(())
+        }
+
+        #[tokio::test]
+        async fn test_min_like_main_test_str() -> error::Result<()> {
+            let query = r#"
+                SELECT min_by(v, k)
+                FROM (
+                    VALUES
+                        ('a', 10),
+                        ('b', 5),
+                        ('c', 15),
+                        ('d', 8)
+                ) AS t(v, k)
+            "#;
+            let df = ctx()?.sql(&query).await?;
+            let result = extract_single_value::<String, arrow::array::StringArray>(df).await?;
+            assert_eq!(result, "b");
+            Ok(())
+        }
+
+        #[tokio::test]
+        async fn test_min_like_main_test_int() -> error::Result<()> {
+            let query = r#"
+                SELECT min_by(v, k)
+                FROM (
+                    VALUES
+                        (1, 10),
+                        (2, 5),
+                        (3, 15),
+                        (4, 8)
+                ) AS t(v, k)
+            "#;
+            let df = ctx()?.sql(&query).await?;
+            let result = extract_single_value::<i64, arrow::array::Int64Array>(df).await?;
+            assert_eq!(result, 2);
+            Ok(())
+        }
+
         fn ctx() -> error::Result<prelude::SessionContext> {
             let ctx = test_ctx()?;
             let min_by_udaf = MinByFunction::new();

tests/main.rs

@@ -185,7 +185,7 @@ async fn test_max_by_and_min_by() {
     - +---------------------+
     - "| max_by(tab.x,tab.y) |"
     - +---------------------+
-    - "| 2                   |"
+    - "| 3                   |"
     - +---------------------+
     "###);
 
@@ -200,7 +200,7 @@ async fn test_max_by_and_min_by() {
     - +---------------------+
     - "| min_by(tab.x,tab.y) |"
     - +---------------------+
-    - "| 2                   |"
+    - "|                     |"
     - +---------------------+
     "###);