xiedeyantu commented on code in PR #4497:
URL: https://github.com/apache/calcite/pull/4497#discussion_r2272031983


##########
core/src/main/java/org/apache/calcite/sql/validate/implicit/AbstractTypeCoercion.java:
##########
@@ -582,16 +582,53 @@ private RelDataType getTightestCommonTypeOrThrow(
     }
 
     if (SqlTypeUtil.isString(type1) && SqlTypeUtil.isString(type2)) {
-      // Return the string with the larger precision
-      if (type1.getPrecision() == RelDataType.PRECISION_NOT_SPECIFIED) {
-        return factory.createTypeWithNullability(type1, anyNullable);
-      } else if (type2.getPrecision() == RelDataType.PRECISION_NOT_SPECIFIED) {
-        return factory.createTypeWithNullability(type2, anyNullable);
-      } else if (type1.getPrecision() > type2.getPrecision()) {
-        return factory.createTypeWithNullability(type1, anyNullable);
+      // Note that isString covers VAR/BINARY and VAR/CHAR
+      SqlTypeName type1Name = type1.getSqlTypeName();
+      SqlTypeName type2Name = type2.getSqlTypeName();
+      SqlTypeName resultName;
+
+      int precision;
+      if (type1.getPrecision() == RelDataType.PRECISION_NOT_SPECIFIED
+          || type2.getPrecision() == RelDataType.PRECISION_NOT_SPECIFIED) {
+        precision = RelDataType.PRECISION_NOT_SPECIFIED;
       } else {
-        return factory.createTypeWithNullability(type2, anyNullable);
+        precision = Math.max(type1.getPrecision(), type2.getPrecision());
+      }
+
+      if (type1Name == SqlTypeName.VARCHAR || type2Name == 
SqlTypeName.VARCHAR) {
+        resultName = SqlTypeName.VARCHAR;
+      } else if (type1Name == SqlTypeName.CHAR || type2Name == 
SqlTypeName.CHAR) {
+        resultName = SqlTypeName.CHAR;
+        // If any is BINARY, use VARCHAR
+        if (type1Name == SqlTypeName.BINARY || type2Name == SqlTypeName.BINARY
+            || type1Name == SqlTypeName.VARBINARY || type2Name == 
SqlTypeName.VARBINARY) {
+          resultName = SqlTypeName.VARCHAR;
+          // We use unlimited precision in this case
+          precision = RelDataType.PRECISION_NOT_SPECIFIED;
+        }
+      } else if (type1Name == SqlTypeName.VARBINARY || type2Name == 
SqlTypeName.VARBINARY) {
+        resultName = SqlTypeName.VARBINARY;
+      } else {
+        resultName = SqlTypeName.BINARY;
       }
+
+      if (type1.getCharset() != null && type2.getCharset() != null

Review Comment:
   Would it be better to put the following two if logics above (line 585)? 
There doesn't seem to be any test coverage here.



-- 
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]

Reply via email to