findepi commented on code in PR #12224:
URL: https://github.com/apache/datafusion/pull/12224#discussion_r1738528813
##########
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:
yes, that's my intuition
except that i would keep building StringArray and just declare return type
as Utf8 always
from the issue https://github.com/apache/datafusion/issues/11836
> Currently, a call to CONCAT with a Utf8View datatypes induces a cast.
After the change that fixes this issue, it should not.
this is about _inputs_ to the function, not the return type
Side note:
String view could be an interesting return type if we wanted to optimize for
single non-null string view input and let it pass-through; but the code doesn't
do this today, not sure it's worth implementing for this edge case and it
should be independent of arguments order, ie not tied to the first input's type.
end of side note.
--
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
