Omega359 commented on PR #12203:
URL: https://github.com/apache/datafusion/pull/12203#issuecomment-2315978608

   
   > Could you please elaborate more on the return types in the first bullet?
   
   Sure. That was one I saw that wasn't related to anything in this PR but that 
I think might be improved.
   ```
   fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
           use DataType::*;
           Ok(match &arg_types[0] {
               LargeUtf8 | LargeBinary => LargeUtf8,
               Utf8 | Binary => Utf8,
               Utf8View | BinaryView => Utf8View,
               Null => Null,
               Dictionary(_, t) => match **t {
                   LargeUtf8 | LargeBinary => LargeUtf8,
                   Utf8 | Binary => Utf8,
                   Null => Null,
                   _ => {
                       return plan_err!(
                           "the regexp_replace can only accept strings but got 
{:?}",
                           **t
                       );
                   }
               },
               other => {
                   return plan_err!(
                       "The regexp_replace function can only accept strings. 
Got {other}"
                   );
               }
           })
       }
   ```
   Unless I'm mistaken that return type method's args are the same as what is 
passed to invoke (and what is declared in the signature) and that is just Utf8, 
LargeUtf8 and Utf8View (and the signature doesn't even cover the LargeUtf8 
case). Basically, I think it needs to be cleaned up wrt types to be consistent.
   
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to