Revert "[BugFix] Fix assigning wrong type for empty array/map/struct (S…

…tarRocks#16658)" (StarRocks#17190)
duan4718 · Feb 2, 2023 · 286aa36 · 286aa36
1 parent 187c52c
commit 286aa36
Show file tree

Hide file tree

Showing 10 changed files with 17 additions and 56 deletions.
diff --git a/fe/fe-core/src/main/java/com/starrocks/catalog/ArrayType.java b/fe/fe-core/src/main/java/com/starrocks/catalog/ArrayType.java
@@ -35,7 +35,6 @@ public ArrayType(Type itemType) {
         if (itemType != null && itemType.isDecimalV3()) {
             throw new InternalError("Decimal32/64/128 is not supported in current version");
         }
-        Preconditions.checkState(!Type.NULL.equals(itemType), "array's item type cannot be NULL_TYPE");
         this.itemType = itemType;
     }
 

diff --git a/fe/fe-core/src/main/java/com/starrocks/catalog/MapType.java b/fe/fe-core/src/main/java/com/starrocks/catalog/MapType.java
@@ -42,8 +42,6 @@ public class MapType extends Type {
     public MapType(Type keyType, Type valueType) {
         Preconditions.checkNotNull(keyType);
         Preconditions.checkNotNull(valueType);
-        Preconditions.checkState(!Type.NULL.equals(keyType), "map's key type cannot be NULL_TYPE");
-        Preconditions.checkState(!Type.NULL.equals(valueType), "map's value type cannot be NULL_TYPE");
         selectedFields = new Boolean[] { false, false };
         this.keyType = keyType;
         this.valueType = valueType;

diff --git a/fe/fe-core/src/main/java/com/starrocks/catalog/StructType.java b/fe/fe-core/src/main/java/com/starrocks/catalog/StructType.java
@@ -56,7 +56,6 @@ public StructType(ArrayList<StructField> structFields) {
         Preconditions.checkArgument(structFields.size() > 0);
         this.fields = new ArrayList<>();
         for (StructField field : structFields) {
-            Preconditions.checkState(!Type.NULL.equals(field.getType()), "struct's field type cannot be NULL_TYPE");
             String lowerFieldName = field.getName().toLowerCase();
             if (fieldMap.containsKey(lowerFieldName)) {
                 LOG.warn(String.format("Contains the same struct subfield name: %s, ignore it", lowerFieldName));
@@ -76,7 +75,6 @@ public StructType(ArrayList<StructField> structFields) {
     public StructType(List<Type> fieldTypes) {
         ArrayList<StructField> newFields = new ArrayList<>();
         for (Type fieldType : fieldTypes) {
-            Preconditions.checkState(!Type.NULL.equals(fieldType), "struct's field type cannot be NULL_TYPE");
             newFields.add(new StructField(fieldType));
         }
         this.fields = newFields;

diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/ExpressionAnalyzer.java b/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/ExpressionAnalyzer.java
@@ -298,10 +298,6 @@ public Void visitArrayExpr(ArrayExpr node, Scope scope) {
                     } else {
                         targetItemType = TypeManager.getCommonSuperType(
                                 node.getChildren().stream().map(Expr::getType).collect(Collectors.toList()));
-                        if (targetItemType.isNull()) {
-                            // For empty array, any item type works out, here we pick Boolean
-                            targetItemType = Type.BOOLEAN;
-                        }
                     }
 
                     // Array<DECIMALV3> type is not supported in current version, turn it into DECIMALV2 type
@@ -320,8 +316,7 @@ public Void visitArrayExpr(ArrayExpr node, Scope scope) {
                     throw new SemanticException(e.getMessage());
                 }
             } else {
-                // For empty array, any item type works out, here we pick Boolean
-                node.setType(Type.ARRAY_BOOLEAN);
+                node.setType(new ArrayType(Type.NULL));
             }
             return null;
         }

diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/PolymorphicFunctionAnalyzer.java b/fe/fe-core/src/main/java/com/starrocks/sql/analyzer/PolymorphicFunctionAnalyzer.java
@@ -104,13 +104,12 @@ private static Type[] resolveArgTypes(Type[] declTypes, Type[] inputArgTypes) {
             }
 
             // Need to make input be a valid complex type if the input is Type NULL
-            // We choose boolean as the item type of complex type if input type is NULL_TYPE
             if (declType instanceof AnyArrayType) {
-                resolvedTypes[i] = inputType.isNull() ? Type.ARRAY_BOOLEAN : inputType;
+                resolvedTypes[i] = inputType.isNull() ? new ArrayType(Type.NULL) : inputType;
             } else if (declType instanceof AnyMapType) {
-                resolvedTypes[i] = inputType.isNull() ? new MapType(Type.BOOLEAN, Type.BOOLEAN) : inputType;
+                resolvedTypes[i] = inputType.isNull() ? new MapType(Type.NULL, Type.NULL) : inputType;
             } else if (declType instanceof AnyStructType) {
-                resolvedTypes[i] = inputType.isNull() ? new StructType(Lists.newArrayList(Type.BOOLEAN)) : inputType;
+                resolvedTypes[i] = inputType.isNull() ? new StructType(Lists.newArrayList(Type.NULL)) : inputType;
             } else {
                 resolvedTypes[i] = inputType;
             }
@@ -292,8 +291,8 @@ public static Function checkPolymorphicFunction(Function fn, Type[] paramTypes)
         } else if (typeElement != null) {
             typeArray = new ArrayType(typeElement);
         } else {
-            typeElement = Type.BOOLEAN;
-            typeArray = (ArrayType) Type.ARRAY_BOOLEAN;
+            typeElement = Type.NULL;
+            typeArray = new ArrayType(Type.NULL);
         }
 
         if (!typeArray.getItemType().matchesType(typeElement)) {

diff --git a/fe/fe-core/src/test/java/com/starrocks/catalog/FunctionSetTest.java b/fe/fe-core/src/test/java/com/starrocks/catalog/FunctionSetTest.java
@@ -116,8 +116,8 @@ public void testPolymorphicFunction() {
         desc = new Function(new FunctionName("array_append"), argTypes, Type.INVALID, false);
         fn = functionSet.getFunction(desc, Function.CompareMode.IS_SUPERTYPE_OF);
         Assert.assertNotNull(fn);
-        Assert.assertEquals(Type.ARRAY_BOOLEAN, fn.getReturnType());
-        Assert.assertEquals(Type.ARRAY_BOOLEAN, fn.getArgs()[0]);
+        Assert.assertEquals(new ArrayType(Type.NULL), fn.getReturnType());
+        Assert.assertEquals(new ArrayType(Type.NULL), fn.getArgs()[0]);
 
         // array_append(ARRAY<ARRAY<INT>>, ARRAY<INT>)
         argTypes = new Type[] {INT_ARRAY_ARRAY, INT_ARRAY};
@@ -240,6 +240,6 @@ public void testPolymorphicFunction() {
         fn = functionSet.getFunction(desc, Function.CompareMode.IS_SUPERTYPE_OF);
         Assert.assertNotNull(fn);
         Assert.assertEquals(Type.INT, fn.getReturnType());
-        Assert.assertEquals(Type.ARRAY_BOOLEAN, fn.getArgs()[0]);
+        Assert.assertEquals(new ArrayType(Type.NULL), fn.getArgs()[0]);
     }
 }
diff --git a/fe/fe-core/src/test/java/com/starrocks/connector/analyzer/SimpleExpressionAnalyzer.java b/fe/fe-core/src/test/java/com/starrocks/connector/analyzer/SimpleExpressionAnalyzer.java
@@ -273,8 +273,7 @@ public Void visitArrayExpr(ArrayExpr node, Scope scope) {
                     throw new SemanticException(e.getMessage());
                 }
             } else {
-                // For empty array, any item type works out, here we pick Boolean
-                node.setType(Type.ARRAY_BOOLEAN);
+                node.setType(new ArrayType(Type.NULL));
             }
             return null;
         }

diff --git a/fe/fe-core/src/test/java/com/starrocks/connector/parser/trino/TrinoQueryTest.java b/fe/fe-core/src/test/java/com/starrocks/connector/parser/trino/TrinoQueryTest.java
@@ -209,7 +209,7 @@ public void testSelectArray() throws Exception {
         assertPlanContains(sql, "ARRAY<ARRAY<tinyint(4)>>[[1,2],[3,4]][1][2]");
 
         sql = "select array[][1]";
-        assertPlanContains(sql, "ARRAY<boolean>[][1]");
+        assertPlanContains(sql, "ARRAY<unknown type: NULL_TYPE>[][1]");
 
         sql = "select array[v1 = 1, v2 = 2, true] from t0";
         assertPlanContains(sql, "<slot 4> : ARRAY<boolean>[1: v1 = 1,2: v2 = 2,TRUE]");
@@ -220,7 +220,7 @@ public void testSelectArray() throws Exception {
 
         sql = "select array[NULL][1] + 1, array[1,2,3][1] + array[array[1,2,3],array[1,1,1]][2][2];";
         assertPlanContains(sql, "1:Project\n" +
-                "  |  <slot 2> : CAST(ARRAY<boolean>[NULL][1] AS SMALLINT) + 1\n" +
+                "  |  <slot 2> : NULL\n" +
                 "  |  <slot 3> : CAST(ARRAY<tinyint(4)>[1,2,3][1] AS SMALLINT) + CAST(ARRAY<ARRAY<tinyint(4)>>" +
                 "[[1,2,3],[1,1,1]][2][2] AS SMALLINT)");
 

diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/ArrayTypeTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/ArrayTypeTest.java
@@ -67,7 +67,7 @@ public void testArrayCountDistinctWithOrderBy() throws Exception {
     public void testArrayElementExpr() throws Exception {
         String sql = "select [][1] + 1, [1,2,3][1] + [[1,2,3],[1,1,1]][2][2]";
         String plan = getFragmentPlan(sql);
-        Assert.assertTrue(plan.contains("  |  <slot 2> : CAST(ARRAY<boolean>[][1] AS SMALLINT) + 1\n" +
+        Assert.assertTrue(plan.contains("  |  <slot 2> : NULL\n" +
                 "  |  <slot 3> : CAST(ARRAY<tinyint(4)>[1,2,3][1] AS SMALLINT) + " +
                 "CAST(ARRAY<ARRAY<tinyint(4)>>[[1,2,3],[1,1,1]][2][2] AS SMALLINT)"));
 
@@ -118,15 +118,15 @@ public void testSelectArrayElement() throws Exception {
 
         sql = "select [][1]";
         plan = getFragmentPlan(sql);
-        Assert.assertTrue(plan.contains("ARRAY<boolean>[][1]"));
+        Assert.assertTrue(plan.contains("ARRAY<unknown type: NULL_TYPE>[][1]"));
 
         sql = "select [][1] from t0";
         plan = getFragmentPlan(sql);
-        Assert.assertTrue(plan.contains("ARRAY<boolean>[][1]"));
+        Assert.assertTrue(plan.contains("ARRAY<unknown type: NULL_TYPE>[][1]"));
 
         sql = "select [][1] from (values(1,2,3), (4,5,6)) t";
         plan = getFragmentPlan(sql);
-        Assert.assertTrue(plan.contains("ARRAY<boolean>[][1]"));
+        Assert.assertTrue(plan.contains("ARRAY<unknown type: NULL_TYPE>[][1]"));
 
         sql = "select [v1,v2] from t0";
         plan = getFragmentPlan(sql);
@@ -213,31 +213,4 @@ public void testArrayWindowFunction() throws Exception {
             getFragmentPlan(sql);
         }
     }
-
-    @Test
-    public void testArrayNull() throws Exception {
-        {
-            String sql = "select cast([] as array<varchar(200)>)";
-            String thriftPlan = getThriftPlan(sql);
-            assertNotContains(thriftPlan, "NULL_TYPE");
-
-            String plan = getFragmentPlan(sql);
-            assertContains(plan, "  1:Project\n" +
-                    "  |  <slot 2> : CAST(ARRAY<boolean>[] AS ARRAY<VARCHAR(200)>)\n" +
-                    "  |  \n" +
-                    "  0:UNION\n" +
-                    "     constant exprs: \n" +
-                    "         NUL");
-        }
-        {
-            String sql = "select cast(null as array<varchar(200)>)";
-            String plan = getFragmentPlan(sql);
-            assertContains(plan, "  1:Project\n" +
-                    "  |  <slot 2> : NULL\n" +
-                    "  |  \n" +
-                    "  0:UNION\n" +
-                    "     constant exprs: \n" +
-                    "         NULL");
-        }
-    }
 }
diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/StructTypePlanTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/StructTypePlanTest.java
@@ -148,7 +148,7 @@ public void testStructWithWindow() throws Exception {
     @Test
     public void testLambda() throws Exception {
         String sql = "select array_map(x->x, [])";
-        assertVerbosePlanContains(sql, "ARRAY<boolean>[]");
+        assertVerbosePlanContains(sql, "ARRAY<unknown type: NULL_TYPE>[]");
         sql = "select array_map(x->x+1, c3.d) from test";
         assertVerbosePlanContains(sql, "[STRUCT<d ARRAY<int(11)>>]");
         sql = "select array_sortby(x->x+1, c3.d) from test";