From a22eff5a55d3979b44882c71cd532be9ddd31327 Mon Sep 17 00:00:00 2001 From: Almog Gavra Date: Tue, 7 Jan 2020 13:10:26 -0800 Subject: [PATCH] feat: improve error messages --- .../execution/util/ExpressionTypeManager.java | 36 ++++++++++----- .../util/ExpressionTypeManagerTest.java | 10 ++-- .../query-validation-tests/asarray.json | 46 +++++++++++++++++++ .../query-validation-tests/asmap.json | 33 +++++++++++++ .../insert-values.json | 24 ++++++++++ 5 files changed, 132 insertions(+), 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 index 4482d66b1175..b517046a48d6 100644 --- 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 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 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 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 index a7f29eaa58ac..12e767ed3c09 100644 --- 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 index 350c1381f87e..9e3da2c8c264 100644 --- 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]" + } } ] } \ No newline at end of file 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 index c57e74a54965..2291b1d81b07 100644 --- 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)." + } } ] } \ No newline at end of file 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 index f39e08beffcd..7b53f371642c 100644 --- 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>) 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) 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": [