You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2023/06/11 01:00:58 UTC

[calcite] branch main updated: [CALCITE-5755] CONCAT function (enabled in Oracle library) should only return NULL when both arguments are NULL

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

jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/main by this push:
     new 9978857a19 [CALCITE-5755] CONCAT function (enabled in Oracle library) should only return NULL when both arguments are NULL
9978857a19 is described below

commit 9978857a195a4de04fa4458615ebb585fd2ac636
Author: ILuffZhe <il...@163.com>
AuthorDate: Sat Jun 3 23:07:43 2023 +0800

    [CALCITE-5755] CONCAT function (enabled in Oracle library) should only return NULL when both arguments are NULL
    
    This behavior applies to the function internally called CONCAT2;
    it does not apply to the CONCAT functions enabled in MySQL
    and BigQuery function libraries.
    
    Close apache/calcite#3255
---
 .../calcite/adapter/enumerable/RexImpTable.java    |  2 +-
 .../org/apache/calcite/runtime/SqlFunctions.java   | 14 ++++++
 .../calcite/sql/fun/SqlLibraryOperators.java       |  8 +++-
 .../org/apache/calcite/sql/type/ReturnTypes.java   |  7 +++
 .../org/apache/calcite/util/BuiltInMethod.java     |  1 +
 .../org/apache/calcite/test/SqlFunctionsTest.java  | 11 +++++
 core/src/test/resources/sql/functions.iq           | 53 ++++++++++++++++++++++
 site/_docs/reference.md                            |  2 +-
 .../org/apache/calcite/test/SqlOperatorTest.java   |  7 ++-
 9 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
index 8f4272f15c..e7b80c68d6 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
@@ -491,7 +491,7 @@ public class RexImpTable {
       map.put(CONCAT, new ConcatImplementor());
       defineMethod(CONCAT_FUNCTION, BuiltInMethod.MULTI_STRING_CONCAT.method,
           NullPolicy.STRICT);
-      defineMethod(CONCAT2, BuiltInMethod.STRING_CONCAT.method, NullPolicy.STRICT);
+      defineMethod(CONCAT2, BuiltInMethod.STRING_CONCAT_WITH_NULL.method, NullPolicy.ALL);
       defineMethod(OVERLAY, BuiltInMethod.OVERLAY.method, NullPolicy.STRICT);
       defineMethod(POSITION, BuiltInMethod.POSITION.method, NullPolicy.STRICT);
       defineMethod(ASCII, BuiltInMethod.ASCII.method, NullPolicy.STRICT);
diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
index 3155cbe35b..7547a5cbe1 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -768,6 +768,20 @@ public class SqlFunctions {
     return s0 + s1;
   }
 
+  /** Concatenates two strings.
+   * Returns null only when both s0 and s1 are null,
+   * otherwise null is treated as empty string. */
+  public static @Nullable String concatWithNull(@Nullable String s0,
+      @Nullable String s1) {
+    if (s0 == null) {
+      return s1;
+    } else if (s1 == null) {
+      return s0;
+    } else {
+      return s0 + s1;
+    }
+  }
+
   /** SQL {@code binary || binary} operator. */
   public static ByteString concat(ByteString s0, ByteString s1) {
     return s0.concat(s1);
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 a8ef9312b9..12c48c9bf0 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
@@ -844,12 +844,18 @@ public abstract class SqlLibraryOperators {
   /** The "CONCAT(arg0, arg1)" function that concatenates strings.
    * For example, "CONCAT('a', 'bc')" returns "abc".
    *
+   * <p>If one of the arguments is null, it will be treated as empty string.
+   * "CONCAT('a', null)" returns "a".
+   *
+   * <p>Returns null only when both arguments are null.
+   * "CONCAT(null, null)" returns null.
+   *
    * <p>It is assigned {@link SqlKind#CONCAT2} to make it not equal to
    * {@link #CONCAT_FUNCTION}. */
   @LibraryOperator(libraries = {ORACLE})
   public static final SqlFunction CONCAT2 =
       SqlBasicFunction.create("CONCAT",
-          ReturnTypes.MULTIVALENT_STRING_SUM_PRECISION_NULLABLE,
+          ReturnTypes.MULTIVALENT_STRING_SUM_PRECISION_NULLABLE_ALL,
           OperandTypes.STRING_SAME_SAME,
           SqlFunctionCategory.STRING)
           .withOperandTypeInference(InferTypes.RETURN_TYPE)
diff --git a/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java b/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java
index e34cc32dd3..1705f13135 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java
@@ -947,6 +947,13 @@ public abstract class ReturnTypes {
   public static final SqlReturnTypeInference MULTIVALENT_STRING_SUM_PRECISION_NULLABLE =
       MULTIVALENT_STRING_SUM_PRECISION.andThen(SqlTypeTransforms.TO_NULLABLE);
 
+  /**
+   * Same as {@link #MULTIVALENT_STRING_SUM_PRECISION} and using
+   * {@link org.apache.calcite.sql.type.SqlTypeTransforms#TO_NULLABLE_ALL}.
+   */
+  public static final SqlReturnTypeInference MULTIVALENT_STRING_SUM_PRECISION_NULLABLE_ALL =
+      MULTIVALENT_STRING_SUM_PRECISION.andThen(SqlTypeTransforms.TO_NULLABLE_ALL);
+
   /**
    * Same as {@link #DYADIC_STRING_SUM_PRECISION} and using
    * {@link org.apache.calcite.sql.type.SqlTypeTransforms#TO_NULLABLE},
diff --git a/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java b/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
index 1810c03c4d..1b34c280fa 100644
--- a/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
+++ b/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
@@ -419,6 +419,7 @@ public enum BuiltInMethod {
   OCTET_LENGTH(SqlFunctions.class, "octetLength", ByteString.class),
   CHAR_LENGTH(SqlFunctions.class, "charLength", String.class),
   STRING_CONCAT(SqlFunctions.class, "concat", String.class, String.class),
+  STRING_CONCAT_WITH_NULL(SqlFunctions.class, "concatWithNull", String.class, String.class),
   MULTI_STRING_CONCAT(SqlFunctions.class, "concatMulti", String[].class),
   FLOOR_DIV(Math.class, "floorDiv", long.class, long.class),
   FLOOR_MOD(Math.class, "floorMod", long.class, long.class),
diff --git a/core/src/test/java/org/apache/calcite/test/SqlFunctionsTest.java b/core/src/test/java/org/apache/calcite/test/SqlFunctionsTest.java
index 7bf7acd14d..a9be7b7bb4 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlFunctionsTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlFunctionsTest.java
@@ -42,6 +42,7 @@ import static org.apache.calcite.avatica.util.DateTimeUtils.timeStringToUnixDate
 import static org.apache.calcite.avatica.util.DateTimeUtils.timestampStringToUnixDate;
 import static org.apache.calcite.runtime.SqlFunctions.charLength;
 import static org.apache.calcite.runtime.SqlFunctions.concat;
+import static org.apache.calcite.runtime.SqlFunctions.concatWithNull;
 import static org.apache.calcite.runtime.SqlFunctions.fromBase64;
 import static org.apache.calcite.runtime.SqlFunctions.greater;
 import static org.apache.calcite.runtime.SqlFunctions.initcap;
@@ -138,6 +139,16 @@ class SqlFunctionsTest {
     assertThat(concat(null, "b"), is("nullb"));
   }
 
+  @Test void testConcatWithNull() {
+    assertThat(concatWithNull("a b", "cd"), is("a bcd"));
+    // Null value could be passed in. If we pass one null value,
+    // it is treated like empty string, if both values are null, returns null.
+    // As the following tests show.
+    assertThat(concatWithNull("a", null), is("a"));
+    assertThat(concatWithNull(null, null), is(nullValue()));
+    assertThat(concatWithNull(null, "b"), is("b"));
+  }
+
   @Test void testPosixRegex() {
     assertThat(posixRegex("abc", "abc", true), is(true));
     assertThat(posixRegex("abc", "^a", true), is(true));
diff --git a/core/src/test/resources/sql/functions.iq b/core/src/test/resources/sql/functions.iq
index 9d828d2b58..64c24a4367 100644
--- a/core/src/test/resources/sql/functions.iq
+++ b/core/src/test/resources/sql/functions.iq
@@ -78,6 +78,18 @@ SELECT CONCAT('c', 'h', 'a', 'r');
 
 !ok
 
+# CONCAT in MySQL, BigQuery returns NULL if any argument is NULL.
+# (CONCAT in Oracle and Postgres ignores NULL arguments.)
+SELECT CONCAT('c', 'h', 'a', null, 'r');
++--------+
+| EXPR$0 |
++--------+
+|        |
++--------+
+(1 row)
+
+!ok
+
 # Compression Functions
 
 SELECT COMPRESS('sample');
@@ -277,6 +289,47 @@ select concat('a', 'b');
 
 !ok
 
+# [CALCITE-5745] CONCAT function (enabled in Oracle library) should only return NULL when both arguments are NULL
+select concat('a', null);
++--------+
+| EXPR$0 |
++--------+
+| a      |
++--------+
+(1 row)
+
+!ok
+
+select concat('a', cast(null as varchar));
++--------+
+| EXPR$0 |
++--------+
+| a      |
++--------+
+(1 row)
+
+!ok
+
+select concat(cast(null as varchar), 'a');
++--------+
+| EXPR$0 |
++--------+
+| a      |
++--------+
+(1 row)
+
+!ok
+
+select concat(null, null);
++--------+
+| EXPR$0 |
++--------+
+|        |
++--------+
+(1 row)
+
+!ok
+
 SELECT XMLTRANSFORM(
                    '<?xml version="1.0"?>
                     <Article>
diff --git a/site/_docs/reference.md b/site/_docs/reference.md
index 08091dcfa6..22387144f8 100644
--- a/site/_docs/reference.md
+++ b/site/_docs/reference.md
@@ -2670,7 +2670,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | * | ATANH(numeric)                                 | Returns the inverse hyperbolic tangent of *numeric*
 | m s | CHAR(integer)                                | Returns the character whose ASCII code is *integer* % 256, or null if *integer* &lt; 0
 | b o p | CHR(integer)                               | Returns the character whose UTF-8 code is *integer*
-| o | CONCAT(string, string)                         | Concatenates two strings
+| o | CONCAT(string, string)                         | Concatenates two strings, returns null only when both string arguments are null, otherwise treats null as empty string
 | b m p | CONCAT(string [, string ]*)                | Concatenates two or more strings
 | m | COMPRESS(string)                               | Compresses a string using zlib compression and returns the result as a binary string
 | q | CONVERT(type, expression [ , style ])          | Equivalent to `CAST(expression AS type)`; ignores the *style* operand
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 271648c8bf..63fc2deb7a 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -1970,7 +1970,12 @@ public class SqlOperatorTest {
     f.checkString("concat('', '')", "", "VARCHAR(0) NOT NULL");
     f.checkString("concat('', 'a')", "a", "VARCHAR(1) NOT NULL");
     f.checkString("concat('a', 'b')", "ab", "VARCHAR(2) NOT NULL");
-    f.checkNull("concat('a', cast(null as varchar))");
+    // treat null value as empty string
+    f.checkString("concat('a', cast(null as varchar))", "a", "VARCHAR NOT NULL");
+    f.checkString("concat(null, 'b')", "b", "VARCHAR NOT NULL");
+    // return null when both arguments are null
+    f.checkNull("concat(null, cast(null as varchar))");
+    f.checkNull("concat(null, null)");
     f.checkFails("^concat('a', 'b', 'c')^", INVALID_ARGUMENTS_NUMBER, false);
     f.checkFails("^concat('a')^", INVALID_ARGUMENTS_NUMBER, false);
   }