From 19debd3e90e391bc6ba8250ec81735490b4b0cee Mon Sep 17 00:00:00 2001 From: Mihai Budiu Date: Tue, 7 May 2024 11:38:06 -0700 Subject: [PATCH] [CALCITE-6361] Uncollect.deriveUncollectRowType throws AssertionFailures * if the input data is not a collection Signed-off-by: Mihai Budiu --- .../apache/calcite/rel/core/Uncollect.java | 6 ++- .../calcite/runtime/CalciteResource.java | 3 ++ .../calcite/sql2rel/SqlToRelConverter.java | 37 +++++++++++-------- .../runtime/CalciteResource.properties | 1 + .../org/apache/calcite/test/ServerTest.java | 21 +++++++++++ 5 files changed, 52 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rel/core/Uncollect.java b/core/src/main/java/org/apache/calcite/rel/core/Uncollect.java index 6a4c15e0a86..e19faca671c 100644 --- a/core/src/main/java/org/apache/calcite/rel/core/Uncollect.java +++ b/core/src/main/java/org/apache/calcite/rel/core/Uncollect.java @@ -35,6 +35,8 @@ import java.util.Collections; import java.util.List; +import static org.apache.calcite.util.Static.RESOURCE; + /** * Relational expression that unnests its input's columns into a relation. * @@ -168,7 +170,9 @@ public static RelDataType deriveUncollectRowType(RelNode rel, builder.add(SqlUnnestOperator.MAP_VALUE_COLUMN_NAME, mapType.getValueType()); } else { RelDataType ret = field.getType().getComponentType(); - assert null != ret; + if (null == ret) { + throw RESOURCE.unnestArgument().ex(); + } if (requireAlias) { builder.add(itemAliases.get(i), ret); diff --git a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java index 86e67797f80..6b531c06ec5 100644 --- a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java +++ b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java @@ -941,6 +941,9 @@ ExInst invalidTypesForComparison(String clazzName0, String op, @BaseMessage("Invalid character for cast: {0}") ExInst invalidCharacterForCast(String s); + @BaseMessage("UNNEST argument must be a collection") + ExInst unnestArgument(); + @BaseMessage("More than one value in list: {0}") ExInst moreThanOneValueInList(String list); diff --git a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java index 2b79213838a..e9b9eff95e4 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java @@ -225,6 +225,7 @@ import static org.apache.calcite.sql.SqlUtil.containsDefault; import static org.apache.calcite.sql.SqlUtil.containsIn; import static org.apache.calcite.sql.SqlUtil.stripAs; +import static org.apache.calcite.util.Static.RESOURCE; import static java.util.Objects.requireNonNull; @@ -2499,21 +2500,27 @@ private void convertUnnest(Blackboard bb, SqlCall call, @Nullable List f RelNode child = (null != bb.root) ? bb.root : LogicalValues.createOneRow(cluster); RelNode uncollect; - if (validator().config().conformance().allowAliasUnnestItems()) { - uncollect = relBuilder - .push(child) - .project(exprs) - .uncollect(requireNonNull(fieldNames, "fieldNames"), operator.withOrdinality) - .build(); - } else { - // REVIEW danny 2020-04-26: should we unify the normal field aliases and - // the item aliases? - uncollect = relBuilder - .push(child) - .project(exprs) - .uncollect(Collections.emptyList(), operator.withOrdinality) - .let(r -> fieldNames == null ? r : r.rename(fieldNames)) - .build(); + try { + if (validator().config().conformance().allowAliasUnnestItems()) { + uncollect = relBuilder + .push(child) + .project(exprs) + .uncollect(requireNonNull(fieldNames, "fieldNames"), operator.withOrdinality) + .build(); + } else { + // REVIEW danny 2020-04-26: should we unify the normal field aliases and + // the item aliases? + uncollect = relBuilder + .push(child) + .project(exprs) + .uncollect(Collections.emptyList(), operator.withOrdinality) + .let(r -> fieldNames == null ? r : r.rename(fieldNames)) + .build(); + } + } catch (Exception ex) { + SqlParserPos pos = call.getParserPosition(); + throw RESOURCE.validatorContext( + pos.getLineNum(), pos.getColumnNum(), pos.getEndLineNum(), pos.getEndColumnNum()).ex(ex); } bb.setRoot(uncollect, true); } diff --git a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties index 684e6c7a42a..ab88f0acbac 100644 --- a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties +++ b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties @@ -307,6 +307,7 @@ InvalidTypesForArithmetic=Invalid types for arithmetic: {0} {1} {2} InvalidTypesForComparison=Invalid types for comparison: {0} {1} {2} CannotConvert=Cannot convert {0} to {1} InvalidCharacterForCast=Invalid character for cast: {0} +UnnestArgument=UNNEST argument must be a collection MoreThanOneValueInList=More than one value in list: {0} FailedToAccessField=Failed to access field ''{0}'', index {1,number,#} of object of type {2} IllegalJsonPathSpec=Illegal jsonpath spec ''{0}'', format of the spec should be: '' $'{'expr'}''' diff --git a/server/src/test/java/org/apache/calcite/test/ServerTest.java b/server/src/test/java/org/apache/calcite/test/ServerTest.java index d48b1940de2..881391cf269 100644 --- a/server/src/test/java/org/apache/calcite/test/ServerTest.java +++ b/server/src/test/java/org/apache/calcite/test/ServerTest.java @@ -198,6 +198,27 @@ static Connection connect() throws SQLException { } } + /** Test case for + * [CALCITE-6361] + * Uncollect.deriveUncollectRowType throws AssertionFailures + * if the input data is not a collection. */ + @Test void testUnnest() throws SQLException { + try (Connection c = connect(); + Statement s = c.createStatement()) { + boolean b = s.execute("CREATE TYPE simple AS (s INT, t BOOLEAN)"); + assertThat(b, is(false)); + b = s.execute("CREATE TYPE vec AS (fields SIMPLE ARRAY)"); + assertThat(b, is(false)); + b = s.execute(" CREATE TABLE T(col vec)"); + assertThat(b, is(false)); + SQLException e = + assertThrows( + SQLException.class, + () -> s.executeQuery("SELECT A.* FROM (T CROSS JOIN UNNEST(T.col) A)")); + assertThat(e.getMessage(), containsString("UNNEST argument must be a collection")); + } + } + @Test void testCreateType() throws Exception { try (Connection c = connect(); Statement s = c.createStatement()) {