[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6612: Minor: Move functionality into `BuildInScalarFunction`

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #6612:
URL: https://github.com/apache/arrow-datafusion/pull/6612#discussion_r1224179928


##########
datafusion/expr/src/expr_schema.rs:
##########
@@ -107,7 +105,7 @@ impl ExprSchemable for Expr {
                     .iter()
                     .map(|e| e.get_type(schema))
                     .collect::<Result<Vec<_>>>()?;
-                function::return_type(fun, &data_types)
+                fun.return_type(&data_types)

Review Comment:
   this is a pretty good example of the point of this PR -- the functionality is now part of `fun` rather than a free function that takes `&self` as its first argument



##########
datafusion/expr/src/function_err.rs:
##########
@@ -1,125 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one

Review Comment:
   This is recently added code from @2010YOUY01 (which was awesome) which I simply moved into `BuiltInScalarFunction`



##########
datafusion/expr/src/expr_schema.rs:
##########
@@ -107,7 +105,7 @@ impl ExprSchemable for Expr {
                     .iter()
                     .map(|e| e.get_type(schema))
                     .collect::<Result<Vec<_>>>()?;
-                function::return_type(fun, &data_types)
+                fun.return_type(&data_types)

Review Comment:
   (I hope to apply the same pattern to `window_function::return_type` and `aggregate_function::return_type`



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org