dharanad commented on code in PR #11796: URL: https://github.com/apache/datafusion/pull/11796#discussion_r1705053100
########## datafusion/expr/src/type_coercion/binary.rs: ########## @@ -890,15 +890,22 @@ fn dictionary_coercion( /// 2. Data type of the other side should be able to cast to string type fn string_concat_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> { use arrow::datatypes::DataType::*; - string_coercion(lhs_type, rhs_type).or(match (lhs_type, rhs_type) { - (Utf8, from_type) | (from_type, Utf8) => { - string_concat_internal_coercion(from_type, &Utf8) - } - (LargeUtf8, from_type) | (from_type, LargeUtf8) => { - string_concat_internal_coercion(from_type, &LargeUtf8) + match (lhs_type, rhs_type) { + // If Utf8View is in any side, we coerce to Utf8. Review Comment: > I think it would be better to coerce to `Utf8View` as that coercsion will often be faster (it is faster to cast Utf8 -> Utf8View than the other way around) > > Is that possible? How about we do in a seperate PR. Previously, we were coerced to `Utf8View`, so concat was failing. As a temporary workaround to resolve the issue, I've coerce Utf8 instead. -- 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