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

Reply via email to