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


##########
datafusion/functions/src/encoding/inner.rs:
##########
@@ -49,17 +48,8 @@ impl Default for EncodeFunc {
 
 impl EncodeFunc {
     pub fn new() -> Self {
-        use DataType::*;
         Self {
-            signature: Signature::one_of(

Review Comment:
   It seems to me that moving the signature from a data driven description (aka 
describe "what" is needed and letting some other code compute if the given 
arguments match that signature), this PR is moving many of the functions 
towards more functional (each function has to implement its own custom 
coercion, likely resulting in significant duplication).
   
   What do you think (perhaps as a follow on PR) of adding `DataType::Null` 
support to the Signature calculations somehow rather than inlining / 
duplicating the coercion logic?
   
   Maybe something like 
   ```
   Signature::allow_null(..)
   ```
   that would support automatically coercing arguments from null?
   
   Or maybe we should always support coercing Null to any 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-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