(calcite) 03/03: Revert "[CALCITE-6127] The spark array function gives NullPointerException when element is row type"

[taoran]

This is an automated email from the ASF dual-hosted git repository.

taoran pushed a commit to branch revert-3579-CALCITE-6127
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit c08fa970e5cf8f8e92618ea69c6eeff5de70ccc2
Author: Ran Tao <chucheng...@gmail.com>
AuthorDate: Mon Dec 18 16:15:59 2023 +0800

    Revert "[CALCITE-6127] The spark array function gives NullPointerException 
when element is row type"
    
    This reverts commit 668d9df4f2d55b79ed07c2ede2801c2546fdbbb4.
---
 .../calcite/sql/fun/SqlLibraryOperators.java       | 16 +++++----------
 .../org/apache/calcite/test/SqlOperatorTest.java   | 23 ----------------------
 2 files changed, 5 insertions(+), 34 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java 
b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
index a81f93910b..a5440a211d 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
@@ -1061,22 +1061,13 @@ public abstract class SqlLibraryOperators {
   private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
     final List<RelDataType> operandTypes = opBinding.collectOperandTypes();
 
-    // only numeric & character types check, this is a special spark array case
-    // the form like ARRAY(1, 2, '3') will return ["1", "2", "3"]
+    // only numeric & character types check
     boolean hasNumeric = false;
     boolean hasCharacter = false;
     boolean hasOthers = false;
     for (RelDataType type : operandTypes) {
       SqlTypeFamily family = type.getSqlTypeName().getFamily();
-      // some types such as Row, the family is null, fallback to normal 
inferred type logic
-      if (family == null) {
-        hasOthers = true;
-        break;
-      }
-      // skip it because we allow NULL literal
-      if (SqlTypeUtil.isNull(type)) {
-        continue;
-      }
+      requireNonNull(family, "array element type family");
       switch (family) {
       case NUMERIC:
         hasNumeric = true;
@@ -1084,6 +1075,9 @@ public abstract class SqlLibraryOperators {
       case CHARACTER:
         hasCharacter = true;
         break;
+      case NULL:
+        // skip it becase we allow null
+        break;
       default:
         hasOthers = true;
         break;
diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java 
b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
index 1d15d3cfa9..9578522bd8 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -10537,29 +10537,6 @@ public class SqlOperatorTest {
         "[null, foo]", "CHAR(3) ARRAY NOT NULL");
     f2.checkScalar("array(null)",
         "[null]", "NULL ARRAY NOT NULL");
-    // check complex type
-    f2.checkScalar("array(row(1))", "[{1}]",
-        "RecordType(INTEGER NOT NULL EXPR$0) NOT NULL ARRAY NOT NULL");
-    f2.checkScalar("array(row(1, null))", "[{1, null}]",
-        "RecordType(INTEGER NOT NULL EXPR$0, NULL EXPR$1) NOT NULL ARRAY NOT 
NULL");
-    f2.checkScalar("array(row(null, 1))", "[{null, 1}]",
-        "RecordType(NULL EXPR$0, INTEGER NOT NULL EXPR$1) NOT NULL ARRAY NOT 
NULL");
-    f2.checkScalar("array(row(1, 2))", "[{1, 2}]",
-        "RecordType(INTEGER NOT NULL EXPR$0, INTEGER NOT NULL EXPR$1) NOT NULL 
ARRAY NOT NULL");
-    f2.checkFails("^array(row(1, 2), null)^",
-        "Parameters must be of the same type", false);
-    f2.checkFails("^array(null, row(1, 2))^",
-        "Parameters must be of the same type", false);
-    f2.checkScalar("array(row(1, null), row(2, null))", "[{1, null}, {2, 
null}]",
-        "RecordType(INTEGER NOT NULL EXPR$0, NULL EXPR$1) NOT NULL ARRAY NOT 
NULL");
-    f2.checkScalar("array(row(null, 1), row(null, 2))", "[{null, 1}, {null, 
2}]",
-        "RecordType(NULL EXPR$0, INTEGER NOT NULL EXPR$1) NOT NULL ARRAY NOT 
NULL");
-    f2.checkScalar("array(row(1, null), row(null, 2))", "[{1, null}, {null, 
2}]",
-        "RecordType(INTEGER EXPR$0, INTEGER EXPR$1) NOT NULL ARRAY NOT NULL");
-    f2.checkScalar("array(row(null, 1), row(2, null))", "[{null, 1}, {2, 
null}]",
-        "RecordType(INTEGER EXPR$0, INTEGER EXPR$1) NOT NULL ARRAY NOT NULL");
-    f2.checkScalar("array(row(1, 2), row(3, 4))", "[{1, 2}, {3, 4}]",
-        "RecordType(INTEGER NOT NULL EXPR$0, INTEGER NOT NULL EXPR$1) NOT NULL 
ARRAY NOT NULL");
     // calcite default cast char type will fill extra spaces
     f2.checkScalar("array(1, 2, 'Hi')",
         "[1 , 2 , Hi]", "CHAR(2) NOT NULL ARRAY NOT NULL");