[CALCITE-6361] Uncollect.deriveUncollectRowType throws AssertionFailures

* if the input data is not a collection Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
QAQ-Jun · May 7, 2024 · 19debd3 · 19debd3
1 parent f55c59a
commit 19debd3
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 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
@@ -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
@@ -941,6 +941,9 @@ ExInst<CalciteException> invalidTypesForComparison(String clazzName0, String op,
   @BaseMessage("Invalid character for cast: {0}")
   ExInst<CalciteException> invalidCharacterForCast(String s);
 
+  @BaseMessage("UNNEST argument must be a collection")
+  ExInst<CalciteException> unnestArgument();
+
   @BaseMessage("More than one value in list: {0}")
   ExInst<CalciteException> 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
@@ -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<String> 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
@@ -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: ''<lax|strict> $'{'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
@@ -198,6 +198,27 @@ static Connection connect() throws SQLException {
     }
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-6361">[CALCITE-6361]
+   * Uncollect.deriveUncollectRowType throws AssertionFailures
+   * if the input data is not a collection</a>. */
+  @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()) {