Re: [PR] Move min and max to user defined aggregate function, remove `AggregateFunction` / `AggregateFunctionDefinition::BuiltIn` [datafusion]

[via GitHub]

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


##########
datafusion/proto/gen/src/main.rs:
##########
@@ -52,7 +53,11 @@ fn main() -> Result<(), String> {
     let prost = proto_dir.join("src/datafusion.rs");
     let pbjson = proto_dir.join("src/datafusion.serde.rs");
     let common_path = proto_dir.join("src/datafusion_common.rs");
-
+    println!(
+        "Copying {} to {}",
+        prost.clone().display(),
+        proto_dir.join("src/generated/prost.rs").display()
+    );

Review Comment:
   I think it is fine to print out some status reporting while regenerating 
protos 👍 



##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -466,51 +464,6 @@ message InListNode {
   bool negated = 3;
 }
 
-enum AggregateFunction {

Review Comment:
   
![200w](https://github.com/user-attachments/assets/e791bc20-9271-4af4-885d-197419e070bf)
   



##########
datafusion/proto/src/physical_plan/mod.rs:
##########
@@ -477,30 +477,10 @@ impl AsExecutionPlan for protobuf::PhysicalPlanNode {
                             ExprType::AggregateExpr(agg_node) => {
                                 let input_phy_expr: Vec<Arc<dyn PhysicalExpr>> 
= agg_node.expr.iter()
                                     .map(|e| parse_physical_expr(e, registry, 
&physical_schema, extension_codec)).collect::<Result<Vec<_>>>()?;
-                                let ordering_req: Vec<PhysicalSortExpr> = 
agg_node.ordering_req.iter()
+                                let _ordering_req: Vec<PhysicalSortExpr> = 
agg_node.ordering_req.iter()

Review Comment:
   this is an interesting change -- does it mean ordering is not carried into 
the udf? 
   
   Or maybe it is redundant and now is entirely determined by the udf



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