findepi commented on code in PR #12224:
URL: https://github.com/apache/datafusion/pull/12224#discussion_r1739363646
##########
datafusion/functions/src/string/concat.rs:
##########
@@ -64,13 +66,19 @@ impl ScalarUDFImpl for ConcatFunc {
&self.signature
}
- fn return_type(&self, _arg_types: &[DataType]) -> Result<DataType> {
- Ok(Utf8)
+ fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
+ use DataType::*;
+ Ok(match &arg_types[0] {
+ Utf8View => Utf8View,
+ LargeUtf8 => LargeUtf8,
+ _ => Utf8,
+ })
Review Comment:
i don't know the exact rules for how we handle LargeUtf8.
i focused on the string view portion of the PR. from "string views
perspective", LargeUtf8 is non-issue, so IMO it's fine not to change the return
type with respect to LargeUtf8 in this PR. but i agree that we probably should
return LargeUtf8 when any input is LargeUtf8 (pr what exactly the logic should
be).
in fact, what does the binary concat operator do?
--
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
