[GitHub] [calcite] libenchao commented on a diff in pull request #3189: [CALCITE-5674] Cast expr to target type should respect nullable when …

[via GitHub]

libenchao commented on code in PR #3189:
URL: https://github.com/apache/calcite/pull/3189#discussion_r1199578965


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlCastFunction.java:
##########
@@ -122,8 +130,62 @@ static SqlReturnTypeInference returnTypeInference(boolean 
safe) {
   /** Derives the type of "CAST(expression AS targetType)". */
   public static RelDataType deriveType(RelDataTypeFactory typeFactory,
       RelDataType expressionType, RelDataType targetType, boolean safe) {
-    return typeFactory.createTypeWithNullability(targetType,
-        expressionType.isNullable() || safe);
+    return createTypeWithNullabilityFromExpr(typeFactory, expressionType, 
targetType, safe);
+  }
+
+  private static RelDataType 
createTypeWithNullabilityFromExpr(RelDataTypeFactory typeFactory,
+      RelDataType expressionType, RelDataType targetType, boolean safe) {
+    boolean isNullable = expressionType.isNullable() || safe;
+
+    if (isCollection(expressionType)) {
+      RelDataType expressionElementType = expressionType.getComponentType();
+      RelDataType targetElementType = targetType.getComponentType();
+      requireNonNull(expressionElementType, () -> "componentType of " + 
expressionType);
+      requireNonNull(targetElementType, () -> "componentType of " + 
targetType);
+      RelDataType newElementType =
+          createTypeWithNullabilityFromExpr(
+              typeFactory, expressionElementType, targetElementType, safe);
+      return isArray(expressionType)
+          ? SqlTypeUtil.createArrayType(typeFactory, newElementType, 
isNullable)
+          : SqlTypeUtil.createMultisetType(typeFactory, newElementType, 
isNullable);
+    }
+
+    if (isRow(expressionType)) {
+      final int fieldCount = expressionType.getFieldCount();
+      List<RelDataType> typeList = new ArrayList<>();

Review Comment:
   NIT: add `final`, and initialize `ArrayList` with `fieldCount` 



##########
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java:
##########
@@ -7033,7 +7036,7 @@ void testGroupExpressionEquivalenceParams() {
     sql("select cast(a as row(f0 int, f1 varchar)) from COMPLEXTYPES.CTC_T1")
         .withExtendedCatalog()
         .columnType("RecordType(INTEGER NOT NULL F0, VARCHAR NOT NULL F1) NOT 
NULL");
-    sql("select cast(b as row(f0 int not null, f1 varchar null))\n"
+    sql("select cast(b as row(f0 int, f1 varchar))\n"

Review Comment:
   why are you changing this test and the below one?



##########
core/src/main/java/org/apache/calcite/sql/fun/SqlCastFunction.java:
##########
@@ -122,8 +130,62 @@ static SqlReturnTypeInference returnTypeInference(boolean 
safe) {
   /** Derives the type of "CAST(expression AS targetType)". */
   public static RelDataType deriveType(RelDataTypeFactory typeFactory,
       RelDataType expressionType, RelDataType targetType, boolean safe) {
-    return typeFactory.createTypeWithNullability(targetType,
-        expressionType.isNullable() || safe);
+    return createTypeWithNullabilityFromExpr(typeFactory, expressionType, 
targetType, safe);
+  }
+
+  private static RelDataType 
createTypeWithNullabilityFromExpr(RelDataTypeFactory typeFactory,
+      RelDataType expressionType, RelDataType targetType, boolean safe) {
+    boolean isNullable = expressionType.isNullable() || safe;
+
+    if (isCollection(expressionType)) {
+      RelDataType expressionElementType = expressionType.getComponentType();
+      RelDataType targetElementType = targetType.getComponentType();
+      requireNonNull(expressionElementType, () -> "componentType of " + 
expressionType);
+      requireNonNull(targetElementType, () -> "componentType of " + 
targetType);
+      RelDataType newElementType =
+          createTypeWithNullabilityFromExpr(
+              typeFactory, expressionElementType, targetElementType, safe);
+      return isArray(expressionType)
+          ? SqlTypeUtil.createArrayType(typeFactory, newElementType, 
isNullable)
+          : SqlTypeUtil.createMultisetType(typeFactory, newElementType, 
isNullable);
+    }
+
+    if (isRow(expressionType)) {
+      final int fieldCount = expressionType.getFieldCount();
+      List<RelDataType> typeList = new ArrayList<>();
+      for (int i = 0; i < fieldCount; ++i) {
+        RelDataType expressionElementType = 
expressionType.getFieldList().get(i).getType();
+        RelDataType targetElementType = 
targetType.getFieldList().get(i).getType();
+        typeList.add(
+            createTypeWithNullabilityFromExpr(typeFactory, 
expressionElementType,
+            targetElementType, safe));

Review Comment:
   NIT: add indent.



-- 
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: commits-unsubscr...@calcite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org