From 98f3048fb1407e2878162ffc80388d4f9dd094b2 Mon Sep 17 00:00:00 2001
From: Mihai Budiu <mbudiu@gmail.com>
Date: Thu, 3 Aug 2023 14:39:39 -0700
Subject: [PATCH] [CALCITE-5882] Compile-time evaluation of SPLIT function
 returns incorrect result

Signed-off-by: Mihai Budiu <mbudiu@gmail.com>
---
 .../calcite/sql/fun/SqlLibraryOperators.java  |  4 +++-
 .../apache/calcite/test/RelOptRulesTest.java  | 13 +++++++++++
 .../apache/calcite/test/RelOptRulesTest.xml   | 23 ++++++++++++++++---
 .../apache/calcite/test/SqlOperatorTest.java  | 18 +++++++--------
 4 files changed, 45 insertions(+), 13 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 7a5d11663154..dce8eb81dff7 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
@@ -333,7 +333,9 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
           Static.RESOURCE.delimiterIsRequired(
               operatorBinding.getOperator().getName(), type.toString()));
     }
-    return type;
+
+    SqlTypeName typeName = SqlTypeUtil.isBinary(type) ? SqlTypeName.VARBINARY : SqlTypeName.VARCHAR;
+    return operatorBinding.getTypeFactory().createSqlType(typeName);
   }
 
   /** The "STRPOS(string, substring)" function. */
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index 762da942543f..a9b66f107630 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -5130,6 +5130,19 @@ private HepProgram getTransitiveProgram() {
         .check();
   }
 
+  /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5882">
+   * [CALCITE-5882] Compile-time evaluation of SPLIT function returns incorrect result</a>. */
+  @Test public void testSplit() {
+    final String query = "select split('1|2|3', '|')";
+    sql(query)
+        .withFactory(
+            t -> t.withOperatorTable(opTab ->
+                SqlLibraryOperatorTableFactory.INSTANCE.getOperatorTable(
+                    SqlLibrary.BIG_QUERY))) // needed for SPLIT function
+        .withRule(CoreRules.PROJECT_REDUCE_EXPRESSIONS)
+        .check();
+  }
+
   /** Test case for right outer join, group by on key same as join
    * key, group by on (left)null generating side. */
   @Test void testPushAggregateThroughOuterJoin12() {
diff --git a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index ea33f58f4eea..aa1330917124 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -13899,6 +13899,23 @@ LogicalProject(B=[ST_BUFFER(ST_POINT(0.0:DECIMAL(2, 1), 0.0:DECIMAL(2, 1)), 1, 4
       <![CDATA[
 LogicalProject(B=[CAST(POLYGON ((1 0, 0.9238795325112867 -0.3826834323650898, 0.7071067811865476 -0.7071067811865475, 0.3826834323650898 -0.9238795325112867, 0.0000000000000001 -1, -0.3826834323650897 -0.9238795325112867, -0.7071067811865475 -0.7071067811865476, -0.9238795325112867 -0.3826834323650899, -1 -0.0000000000000001, -0.9238795325112868 0.3826834323650897, -0.7071067811865477 0.7071067811865475, -0.3826834323650903 0.9238795325112865, -0.0000000000000002 1, 0.38268343236509 0.9238795325112866, 0.7071067811865474 0.7071067811865477, 0.9238795325112865 0.3826834323650904, 1 0)):GEOMETRY):GEOMETRY])
   LogicalTableScan(table=[[CATALOG, GEO, RESTAURANTS]])
+]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testSplit">
+    <Resource name="sql">
+      <![CDATA[select split('1|2|3', '|')]]>
+    </Resource>
+    <Resource name="planBefore">
+      <![CDATA[
+LogicalProject(EXPR$0=[SPLIT('1|2|3', '|')])
+  LogicalValues(tuples=[[{ 0 }]])
+]]>
+    </Resource>
+    <Resource name="planAfter">
+      <![CDATA[
+LogicalProject(EXPR$0=[ARRAY('1':VARCHAR, '2':VARCHAR, '3':VARCHAR)])
+  LogicalValues(tuples=[[{ 0 }]])
 ]]>
     </Resource>
   </TestCase>
@@ -13914,7 +13931,7 @@ LogicalProject(EXPR$0=[SPLIT('1|2|3', null:NULL)])
     </Resource>
     <Resource name="planAfter">
       <![CDATA[
-LogicalProject(EXPR$0=[null:CHAR(5) NOT NULL ARRAY])
+LogicalProject(EXPR$0=[null:VARCHAR NOT NULL ARRAY])
   LogicalValues(tuples=[[{ 0 }]])
 ]]>
     </Resource>
@@ -13931,7 +13948,7 @@ LogicalProject(EXPR$0=[SPLIT(null:NULL, '|')])
     </Resource>
     <Resource name="planAfter">
       <![CDATA[
-LogicalProject(EXPR$0=[null:NULL ARRAY])
+LogicalProject(EXPR$0=[null:VARCHAR NOT NULL ARRAY])
   LogicalValues(tuples=[[{ 0 }]])
 ]]>
     </Resource>
@@ -13948,7 +13965,7 @@ LogicalProject(EXPR$0=[SPLIT(null:NULL, null:NULL)])
     </Resource>
     <Resource name="planAfter">
       <![CDATA[
-LogicalProject(EXPR$0=[null:NULL ARRAY])
+LogicalProject(EXPR$0=[null:VARCHAR NOT NULL ARRAY])
   LogicalValues(tuples=[[{ 0 }]])
 ]]>
     </Resource>
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 3fc1ec425a15..36e4068c423d 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -7651,27 +7651,27 @@ private void testCurrentDateFunc(Pair<String, Hook.Closeable> pair) {
         false);
     final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
     f.checkScalar("SPLIT('h,e,l,l,o')", "[h, e, l, l, o]",
-        "CHAR(9) NOT NULL ARRAY NOT NULL");
+        "VARCHAR NOT NULL ARRAY NOT NULL");
     f.checkScalar("SPLIT('h-e-l-l-o', '-')", "[h, e, l, l, o]",
-        "CHAR(9) NOT NULL ARRAY NOT NULL");
+        "VARCHAR NOT NULL ARRAY NOT NULL");
     f.checkScalar("SPLIT('hello', '-')", "[hello]",
-        "CHAR(5) NOT NULL ARRAY NOT NULL");
+        "VARCHAR NOT NULL ARRAY NOT NULL");
     f.checkScalar("SPLIT('')", "[]",
-        "CHAR(0) NOT NULL ARRAY NOT NULL");
+        "VARCHAR NOT NULL ARRAY NOT NULL");
     f.checkScalar("SPLIT('', '-')", "[]",
-        "CHAR(0) NOT NULL ARRAY NOT NULL");
+        "VARCHAR NOT NULL ARRAY NOT NULL");
     f.checkNull("SPLIT(null)");
     f.checkNull("SPLIT('hello', null)");
 
     // In ASCII, x'41' = 'A', x'42' = 'B', x'43' = 'C'
     f.checkScalar("SPLIT(x'414243', x'ff')", "[ABC]",
-        "BINARY(3) NOT NULL ARRAY NOT NULL");
+        "VARBINARY NOT NULL ARRAY NOT NULL");
     f.checkScalar("SPLIT(x'414243', x'41')", "[, BC]",
-        "BINARY(3) NOT NULL ARRAY NOT NULL");
+        "VARBINARY NOT NULL ARRAY NOT NULL");
     f.checkScalar("SPLIT(x'414243', x'42')", "[A, C]",
-        "BINARY(3) NOT NULL ARRAY NOT NULL");
+        "VARBINARY NOT NULL ARRAY NOT NULL");
     f.checkScalar("SPLIT(x'414243', x'43')", "[AB, ]",
-        "BINARY(3) NOT NULL ARRAY NOT NULL");
+        "VARBINARY NOT NULL ARRAY NOT NULL");
     f.checkFails("^SPLIT(x'aabbcc')^",
         "Call to function 'SPLIT' with argument of type 'BINARY\\(3\\)' "
             + "requires extra delimiter argument", false);