alamb commented on code in PR #10893:
URL: https://github.com/apache/datafusion/pull/10893#discussion_r1638065935


##########
datafusion/optimizer/Cargo.toml:
##########
@@ -56,5 +56,6 @@ regex-syntax = "0.8.0"
 [dev-dependencies]
 arrow-buffer = { workspace = true }
 ctor = { workspace = true }
+datafusion-functions-aggregate = { workspace = true }

Review Comment:
   🤔  -- I thought we were trying to avoid making the optimizer depend on the 
function library -- can we use the sub implementation instead?



##########
datafusion/expr/src/test/function_stub.rs:
##########
@@ -189,3 +202,73 @@ impl AggregateUDFImpl for Sum {
         AggregateOrderSensitivity::Insensitive
     }
 }
+
+pub struct Count {

Review Comment:
   Could we perhaps add some basic documentation, like 
   
   ```suggestion
   /// Testing stub implementation of COUNT aggregate
   pub struct Count {
   ```



##########
datafusion/expr/src/aggregate_function.rs:
##########
@@ -33,8 +33,6 @@ use strum_macros::EnumIter;
 // 
https://datafusion.apache.org/contributor-guide/index.html#how-to-add-a-new-aggregate-function
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Hash, EnumIter)]
 pub enum AggregateFunction {
-    /// Count

Review Comment:
   🎉 
   
   There are still a bunch of these functions -- at some point we can probably 
file tickets and spread out the work / fun to migrate them over. 



##########
datafusion/optimizer/src/single_distinct_to_groupby.rs:
##########
@@ -679,14 +681,11 @@ mod tests {
         let table_scan = test_table_scan()?;
 
         // COUNT(DISTINCT a) FILTER (WHERE a > 5)
-        let expr = Expr::AggregateFunction(expr::AggregateFunction::new(
-            AggregateFunction::Count,
-            vec![col("a")],
-            true,
-            Some(Box::new(col("a").gt(lit(5)))),
-            None,
-            None,
-        ));
+        let expr = count_udaf()
+            .call(vec![col("a")])
+            .distinct()

Review Comment:
   ![200w 
(1)](https://github.com/apache/datafusion/assets/490673/1c972599-e1d4-49bd-b082-acd9e561f8f2)
   



-- 
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

Reply via email to