[CALCITE-5884] ARRAY_TO_STRING function should return NULL if its 'nu…

…llValue' argument is NULL

Signed-off-by: Mihai Budiu <[email protected]>

core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java

@@ -5583,6 +5583,10 @@ public static String arrayToString(List list, String delimiter) {
 
   /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
   public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
+    // Note that the SQL function ARRAY_TO_STRING that we implement will return
+    // 'NULL' when the nullText argument is NULL. However, that is handled by
+    // the nullPolicy of the RexToLixTranslator. So here a NULL value
+    // for the nullText argument can only come from the above 2-argument version.
     StringBuilder sb = new StringBuilder();
     boolean isFirst = true;
     for (Object item : list) {

core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java

@@ -117,7 +117,7 @@ public boolean isExact() {
           scaleValue);
     }
 
-    // else we have a a float, real or double.  make them all double for
+    // else we have a FLOAT, REAL or DOUBLE.  make them all DOUBLE for
     // now.
     return typeFactory.createSqlType(SqlTypeName.DOUBLE);
   }

core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java

@@ -591,7 +591,7 @@ public static SqlCall stripSeparator(SqlCall call) {
   };
 
   /**
-   * Returns the element type of a ARRAY or MULTISET.
+   * Returns the element type of an ARRAY or MULTISET.
    *
    * <p>For example, given <code>INTEGER ARRAY or MULTISET ARRAY</code>, returns
    * <code>INTEGER</code>.

core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java

@@ -225,7 +225,8 @@ private static boolean skipItem(RexNode expr) {
   }
 
   /**
-   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5813">[CALCITE-5813]
+   * Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5813">[CALCITE-5813]
    * Type inference for sql functions REPEAT, SPACE, XML_TRANSFORM,
    * and XML_EXTRACT is incorrect</a>. */
   @Test void testRepeat() {
@@ -246,7 +247,8 @@ private static boolean skipItem(RexNode expr) {
   }
 
   /**
-   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5813">[CALCITE-5813]
+   * Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5813">[CALCITE-5813]
    * Type inference for sql functions REPEAT, SPACE, XML_TRANSFORM,
    * and XML_EXTRACT is incorrect</a>. */
   @Test void testReplace() {
@@ -263,7 +265,8 @@ private static boolean skipItem(RexNode expr) {
   }
 
   /**
-   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5989">[CALCITE-5989]
+   * Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5989">[CALCITE-5989]
    * Type inference for RPAD and LPAD functions (BIGQUERY) is incorrect</a>. */
   @Test void testRpad() {
     HepProgramBuilder builder = new HepProgramBuilder();
@@ -283,7 +286,8 @@ private static boolean skipItem(RexNode expr) {
   }
 
   /**
-   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5989">[CALCITE-5989]
+   * Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5989">[CALCITE-5989]
    * Type inference for RPAD and LPAD functions (BIGQUERY) is incorrect</a>. */
   @Test void testLpad() {
     HepProgramBuilder builder = new HepProgramBuilder();
@@ -303,7 +307,8 @@ private static boolean skipItem(RexNode expr) {
   }
 
   /**
-   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5971">[CALCITE-5971]
+   * Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5971">[CALCITE-5971]
    * Add the RelRule to rewrite the bernoulli sample as Filter</a>. */
   @Test void testSampleToFilter() {
     final String sql = "select deptno from emp tablesample bernoulli(50)";
@@ -313,7 +318,8 @@ private static boolean skipItem(RexNode expr) {
   }
 
   /**
-   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5971">[CALCITE-5971]
+   * Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5971">[CALCITE-5971]
    * Add the RelRule to rewrite the bernoulli sample as Filter</a>. */
   @Test void testSampleToFilterWithSeed() {
     final String sql = "select deptno from emp tablesample bernoulli(50) REPEATABLE(10)";
@@ -323,7 +329,8 @@ private static boolean skipItem(RexNode expr) {
   }
 
   /**
-   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5813">[CALCITE-5813]
+   * Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5813">[CALCITE-5813]
    * Type inference for sql functions REPEAT, SPACE, XML_TRANSFORM,
    * and XML_EXTRACT is incorrect</a>. */
   @Test void testSpace() {
@@ -2034,6 +2041,21 @@ private void checkSemiOrAntiJoinProjectTranspose(JoinRelType type) {
         .check();
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5884">[CALCITE-5884]
+   * ARRAY_TO_STRING function should return NULL if its 'nullValue' argument is NULL</a>. */
+  @Test void testArrayToString() {
+    final String sql = "select array_to_string(array['1','2','3','4',NULL,'6'], ',', NULL)";
+    // We expect the result to be NULL, since array_to_string returns NULL if
+    // any argument is NULL.
+    sql(sql).withFactory(
+        t -> t.withOperatorTable(
+            opTab -> SqlLibraryOperatorTableFactory.INSTANCE.getOperatorTable(
+                SqlLibrary.STANDARD, SqlLibrary.BIG_QUERY))) // for array_to_string function
+        .withRule(CoreRules.PROJECT_REDUCE_EXPRESSIONS)
+        .check();
+  }
+
   /** Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-1558">[CALCITE-1558]
    * AggregateExpandDistinctAggregatesRule gets field mapping wrong if groupKey
@@ -2733,7 +2755,8 @@ private void checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
   }
 
   /**
-   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5073">[CALCITE-5073]
+   * Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5073">[CALCITE-5073]
    * JoinConditionPushRule cannot infer 'LHS.C1 = LHS.C2' from
    * 'LHS.C1 = RHS.C1 AND LHS.C2 = RHS.C1'</a>.
    */
@@ -2750,7 +2773,8 @@ private void checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
   }
 
   /**
-   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5073">[CALCITE-5073]
+   * Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5073">[CALCITE-5073]
    * JoinConditionPushRule cannot infer 'LHS.C1 = LHS.C2' from
    * 'LHS.C1 = RHS.C1 AND LHS.C2 = RHS.C1'</a>.
    */
@@ -5483,7 +5507,8 @@ private HepProgram getTransitiveProgram() {
   }
 
   /**
-   * Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5861">
+   * Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5861">
    * [CALCITE-5861] ReduceExpressionsRule rules should constant-fold
    * expressions in window bounds</a>.
    */
@@ -5682,7 +5707,8 @@ private HepProgram getTransitiveProgram() {
         .checkUnchanged();
   }
 
-  /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5879">
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5879">
    * AssertionError during constant reduction of SPLIT expression that returns NULL</a>. */
   @Test public void testSplitNull() {
     final String query = "select split('1|2|3', NULL)";
@@ -5695,7 +5721,8 @@ private HepProgram getTransitiveProgram() {
         .check();
   }
 
-  /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5879">
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5879">
    * AssertionError during constant reduction of SPLIT expression that returns NULL</a>. */
   @Test public void testSplitNull1() {
     final String query = "select split(NULL, '|')";
@@ -5708,7 +5735,8 @@ private HepProgram getTransitiveProgram() {
         .check();
   }
 
-  /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5879">
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5879">
    * AssertionError during constant reduction of SPLIT expression that returns NULL</a>. */
   @Test public void testSplitNull2() {
     final String query = "select split(NULL, NULL)";
@@ -5721,7 +5749,8 @@ private HepProgram getTransitiveProgram() {
         .check();
   }
 
-  /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-5882">
+  /** 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', '|')";

core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml

@@ -1283,6 +1283,23 @@ LogicalProject(DEPTNO=[$0], EXPR$1=[OR(AND(IS NOT NULL($5), <>($2, 0)), AND(<($3
     LogicalAggregate(group=[{0}], i=[LITERAL_AGG(true)])
       LogicalProject(MGR=[$3])
         LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+    </Resource>
+  </TestCase>
+  <TestCase name="testArrayToString">
+    <Resource name="sql">
+      <![CDATA[select array_to_string(array['1','2','3','4',NULL,'6'], ',', NULL)]]>
+    </Resource>
+    <Resource name="planBefore">
+      <![CDATA[
+LogicalProject(EXPR$0=[ARRAY_TO_STRING(ARRAY('1', '2', '3', '4', null:CHAR(1), '6'), ',', null:NULL)])
+  LogicalValues(tuples=[[{ 0 }]])
+]]>
+    </Resource>
+    <Resource name="planAfter">
+      <![CDATA[
+LogicalProject(EXPR$0=[null:VARCHAR])
+  LogicalValues(tuples=[[{ 0 }]])
 ]]>
     </Resource>
   </TestCase>

site/_docs/reference.md

@@ -2673,7 +2673,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | s | ARRAY_REPEAT(element, count)                   | Returns the array containing element count times.
 | b | ARRAY_REVERSE(array)                           | Reverses elements of *array*
 | s | ARRAY_SIZE(array)                              | Synonym for `CARDINALITY`
-| b | ARRAY_TO_STRING(array, delimiter [, nullText ])| Returns a concatenation of the elements in *array* as a STRING and take *delimiter* as the delimiter. If the *nullText* parameter is used, the function replaces any `NULL` values in the array with the value of *nullText*. If the *nullText* parameter is not used, the function omits the `NULL` value and its preceding delimiter
+| b | ARRAY_TO_STRING(array, delimiter [, nullText ])| Returns a concatenation of the elements in *array* as a STRING and take *delimiter* as the delimiter. If the *nullText* parameter is used, the function replaces any `NULL` values in the array with the value of *nullText*. If the *nullText* parameter is not used, the function omits the `NULL` value and its preceding delimiter. Returns `NULL` if any argument is `NULL`
 | s | ARRAY_UNION(array1, array2)                    | Returns an array of the elements in the union of *array1* and *array2*, without duplicates
 | s | ARRAYS_OVERLAP(array1, array2)                 | Returns true if *array1 contains at least a non-null element present also in *array2*. If the arrays have no common element and they are both non-empty and either of them contains a null element null is returned, false otherwise
 | s | ARRAYS_ZIP(array [, array ]*)                  | Returns a merged *array* of structs in which the N-th struct contains all N-th values of input arrays

testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java

@@ -6600,6 +6600,7 @@ private static void checkIf(SqlOperatorFixture f) {
     f.checkScalar("array_to_string(array['', ''], '-')", "-", "VARCHAR NOT NULL");
     f.checkNull("array_to_string(null, '-')");
     f.checkNull("array_to_string(array['a', 'b', null], null)");
+    f.checkNull("array_to_string(array['1','2','3'], ',', null)");
     f.checkFails("^array_to_string(array[1, 2, 3], '-', ' ')^",
         "Cannot apply 'ARRAY_TO_STRING' to arguments of type 'ARRAY_TO_STRING"
             + "\\(<INTEGER ARRAY>, <CHAR\\(1\\)>, <CHAR\\(1\\)>\\)'\\. Supported form\\(s\\):"