feat: improve error messages

confluentinc · Jan 13, 2020 · 841a14d · 841a14d
1 parent 220c00e
commit 841a14d
Show file tree

Hide file tree

Showing 5 changed files with 132 additions and 17 deletions.
diff --git a/ksql-execution/src/main/java/io/confluent/ksql/execution/util/ExpressionTypeManager.java b/ksql-execution/src/main/java/io/confluent/ksql/execution/util/ExpressionTypeManager.java
@@ -70,6 +70,7 @@
 import io.confluent.ksql.util.KsqlException;
 import io.confluent.ksql.util.VisitorUtil;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
@@ -339,7 +340,9 @@ public Void visitCreateArrayExpression(
         final ExpressionTypeContext context
     ) {
       if (exp.getValues().isEmpty()) {
-        throw new KsqlException("Cannot extract type from empty array!");
+        throw new KsqlException(
+            "Array constructor cannot be empty. Please supply at least one element "
+                + "(see https://github.com/confluentinc/ksql/issues/4239).");
       }
 
       final List<SqlType> sqlTypes = exp
@@ -350,16 +353,20 @@ public Void visitCreateArrayExpression(
             return context.getSqlType();
           })
           .filter(Objects::nonNull)
-          .distinct()
           .collect(Collectors.toList());
 
       if (sqlTypes.size() == 0) {
-        throw new KsqlException("Cannot extract type from array of nulls!");
+        throw new KsqlException("Cannot construct an array with all NULL elements "
+            + "(see https://github.com/confluentinc/ksql/issues/4239). As a workaround, you may "
+            + "cast the NULL value to a desired type.");
       }
 
-      if (sqlTypes.size() != 1) {
+      if (new HashSet<>(sqlTypes).size() != 1) {
         throw new KsqlException(
-            "Invalid array expression! All values must be of the same type." + exp);
+            String.format(
+                "Cannot construct an array with mismatching types (%s) from expression %s.",
+                sqlTypes,
+                exp));
       }
 
       context.setSqlType(SqlArray.of(sqlTypes.get(0)));
@@ -372,19 +379,21 @@ public Void visitCreateMapExpression(
         final ExpressionTypeContext context
     ) {
       if (exp.getMap().isEmpty()) {
-        throw new KsqlException("Cannot extract type from empty map!");
+        throw new KsqlException("Map constructor cannot be empty. Please supply at least one key "
+            + "value pair (see https://github.com/confluentinc/ksql/issues/4239).");
       }
 
-      final boolean anyNonStringKeys = exp.getMap()
+      final List<SqlType> gkeyTypes = exp.getMap()
           .keySet()
           .stream()
           .map(key -> {
             process(key, context);
             return context.getSqlType();
           })
-          .anyMatch(type -> !SqlTypes.STRING.equals(type));
-      if (anyNonStringKeys) {
-        throw new KsqlException("Cannot support non-string keys on maps:" + exp);
+          .collect(Collectors.toList());
+
+      if (gkeyTypes.stream().anyMatch(type -> !SqlTypes.STRING.equals(type))) {
+        throw new KsqlException("Only STRING keys are supported in maps but got: " + gkeyTypes);
       }
 
       final List<SqlType> sqlTypes = exp.getMap()
@@ -399,11 +408,14 @@ public Void visitCreateMapExpression(
 
       if (sqlTypes.size() != 1) {
         throw new KsqlException(
-            "Invalid map expression! All values must be of the same type. " + exp);
+            String.format(
+                "Cannot construct a map with mismatching value types (%s) from expression %s.",
+                sqlTypes,
+                exp));
       }
 
       if (sqlTypes.get(0) == null) {
-        throw new KsqlException("Maps do not accept null values!");
+        throw new KsqlException("Cannot construct a map with NULL values.");
       }
 
       context.setSqlType(SqlMap.of(sqlTypes.get(0)));

diff --git a/ksql-execution/src/test/java/io/confluent/ksql/execution/util/ExpressionTypeManagerTest.java b/ksql-execution/src/test/java/io/confluent/ksql/execution/util/ExpressionTypeManagerTest.java
@@ -365,7 +365,7 @@ public void shouldThrowOnArrayAllNulls() {
 
     // Expect
     expectedException.expect(KsqlException.class);
-    expectedException.expectMessage("Cannot extract type from array of nulls");
+    expectedException.expectMessage("Cannot construct an array with all NULL elements");
 
     // When:
     expressionTypeManager.getExpressionSqlType(expression);
@@ -383,7 +383,7 @@ public void shouldThrowOnArrayMultipleTypes() {
 
     // Expect
     expectedException.expect(KsqlException.class);
-    expectedException.expectMessage("Invalid array expression! All values must be of the same type.");
+    expectedException.expectMessage("Cannot construct an array with mismatching types");
 
     // When:
     expressionTypeManager.getExpressionSqlType(expression);
@@ -417,7 +417,7 @@ public void shouldThrowOnMapOfNonStringKeys() {
 
     // Expect
     expectedException.expect(KsqlException.class);
-    expectedException.expectMessage("Cannot support non-string keys on maps:");
+    expectedException.expectMessage("Only STRING keys are supported in maps");
 
     // When:
     expressionTypeManager.getExpressionSqlType(expression);
@@ -437,7 +437,7 @@ public void shouldThrowOnMapOfMultipleTypes() {
 
     // Expect
     expectedException.expect(KsqlException.class);
-    expectedException.expectMessage("Invalid map expression! All values must be of the same type.");
+    expectedException.expectMessage("Cannot construct a map with mismatching value types");
 
     // When:
     expressionTypeManager.getExpressionSqlType(expression);
@@ -455,7 +455,7 @@ public void shouldThrowOnMapOfNullValues() {
 
     // Expect
     expectedException.expect(KsqlException.class);
-    expectedException.expectMessage("Maps do not accept null values!");
+    expectedException.expectMessage("Cannot construct a map with NULL values");
 
     // When:
     expressionTypeManager.getExpressionSqlType(expression);

diff --git a/ksql-functional-tests/src/test/resources/query-validation-tests/asarray.json b/ksql-functional-tests/src/test/resources/query-validation-tests/asarray.json
@@ -17,6 +17,52 @@
         {"topic": "OUTPUT", "value": {"L": [1, 2, 3]}},
         {"topic": "OUTPUT", "value": {"L": [null, null, 3]}}
       ]
+    },
+    {
+      "name": "construct a list from null casted elements",
+      "statements": [
+        "CREATE STREAM TEST (a INT, b INT) WITH (kafka_topic='test_topic', value_format='JSON');",
+        "CREATE STREAM OUTPUT AS SELECT ARRAY[CAST(NULL AS INT)] as l FROM TEST;"
+      ],
+      "inputs": [
+        {"topic": "test_topic", "value": {"a":  1, "b":  2}}
+      ],
+      "outputs": [
+        {"topic": "OUTPUT", "value": {"L": [null]}}
+      ]
+    },
+    {
+      "name": "construct a list from no elements",
+      "statements": [
+        "CREATE STREAM TEST (a INT, b INT) WITH (kafka_topic='test_topic', value_format='JSON');",
+        "CREATE STREAM OUTPUT AS SELECT ARRAY[] as l FROM TEST;"
+      ],
+      "expectedException": {
+        "type": "io.confluent.ksql.util.KsqlException",
+        "message": "Array constructor cannot be empty. Please supply at least one element (see https://github.com/confluentinc/ksql/issues/4239)."
+      }
+    },
+    {
+      "name": "construct a list from null non-casted elements",
+      "statements": [
+        "CREATE STREAM TEST (a INT, b INT) WITH (kafka_topic='test_topic', value_format='JSON');",
+        "CREATE STREAM OUTPUT AS SELECT ARRAY[NULL] as l FROM TEST;"
+      ],
+      "expectedException": {
+        "type": "io.confluent.ksql.util.KsqlException",
+        "message": "Cannot construct an array with all NULL elements (see https://github.com/confluentinc/ksql/issues/4239). As a workaround, you may cast the NULL value to a desired type."
+      }
+    },
+    {
+      "name": "construct a list from mismatching elements",
+      "statements": [
+        "CREATE STREAM TEST (a INT, b INT) WITH (kafka_topic='test_topic', value_format='JSON');",
+        "CREATE STREAM OUTPUT AS SELECT ARRAY[1, 1.0] as l FROM TEST;"
+      ],
+      "expectedException": {
+        "type": "io.confluent.ksql.util.KsqlException",
+        "message": "Cannot construct an array with mismatching types ([INTEGER, DOUBLE]) from expression ARRAY[1, 1.0]"
+      }
     }
   ]
 }
diff --git a/ksql-functional-tests/src/test/resources/query-validation-tests/asmap.json b/ksql-functional-tests/src/test/resources/query-validation-tests/asmap.json
@@ -34,6 +34,39 @@
         {"topic": "OUTPUT", "value": {"M":  {"a":  1, "b":  2}}},
         {"topic": "OUTPUT", "value": {"M":  {"a":  1, "b":  2, "c":  null}}}
       ]
+    },
+    {
+      "name": "create map from named tuples mismatching types",
+      "statements": [
+        "CREATE STREAM TEST (k1 VARCHAR, k2 VARCHAR, v1 INT) WITH (kafka_topic='test_topic', value_format='JSON');",
+        "CREATE STREAM OUTPUT AS SELECT MAP(k1:=v1, k2:='hello') as M FROM TEST;"
+      ],
+      "expectedException": {
+        "type": "io.confluent.ksql.util.KsqlException",
+        "message": "Cannot construct a map with mismatching value types ([INTEGER, STRING]) from expression MAP(TEST.K1:=TEST.V1, TEST.K2:='hello')."
+      }
+    },
+    {
+      "name": "create map from named tuples null values",
+      "statements": [
+        "CREATE STREAM TEST (k1 VARCHAR, k2 VARCHAR, v1 INT) WITH (kafka_topic='test_topic', value_format='JSON');",
+        "CREATE STREAM OUTPUT AS SELECT MAP(k1:=v1, k2:=NULL) as M FROM TEST;"
+      ],
+      "expectedException": {
+        "type": "io.confluent.ksql.util.KsqlException",
+        "message": "Cannot construct a map with mismatching value types ([INTEGER, null]) from expression MAP(TEST.K1:=TEST.V1, TEST.K2:=null)."
+      }
+    },
+    {
+      "name": "create empty map",
+      "statements": [
+        "CREATE STREAM TEST (k1 VARCHAR, k2 VARCHAR, v1 INT) WITH (kafka_topic='test_topic', value_format='JSON');",
+        "CREATE STREAM OUTPUT AS SELECT MAP() as M FROM TEST;"
+      ],
+      "expectedException": {
+        "type": "io.confluent.ksql.util.KsqlException",
+        "message": "Map constructor cannot be empty. Please supply at least one key value pair (see https://github.com/confluentinc/ksql/issues/4239)."
+      }
     }
   ]
 }
diff --git a/ksql-functional-tests/src/test/resources/rest-query-validation-tests/insert-values.json b/ksql-functional-tests/src/test/resources/rest-query-validation-tests/insert-values.json
@@ -234,6 +234,30 @@
         {"topic": "test_topic", "key": null, "value": {"I": -1, "A": [1, 2, 3]}}
       ]
     },
+    {
+      "name": "should handle arbitrary nested expressions",
+      "statements": [
+        "CREATE STREAM TEST (I INT, A ARRAY<ARRAY<INT>>) WITH (kafka_topic='test_topic', value_format='JSON');",
+        "INSERT INTO TEST (I, A) VALUES (-1, ARRAY[ARRAY[1]]);"
+      ],
+      "inputs": [
+      ],
+      "outputs": [
+        {"topic": "test_topic", "key": null, "value": {"I": -1, "A": [[1]]}}
+      ]
+    },
+    {
+      "name": "should handle map expressions",
+      "statements": [
+        "CREATE STREAM TEST (I INT, A MAP<VARCHAR, INT>) WITH (kafka_topic='test_topic', value_format='JSON');",
+        "INSERT INTO TEST (I, A) VALUES (-1, MAP('a':=0, 'b':=1));"
+      ],
+      "inputs": [
+      ],
+      "outputs": [
+        {"topic": "test_topic", "key": null, "value": {"I": -1, "A": {"a": 0, "b": 1}}}
+      ]
+    },
     {
       "name": "should handle quoted identifiers",
       "statements": [