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
commit 94fce144b1864edffec532638496993835468675 Author: ILuffZhe <[email protected]> AuthorDate: Sat Jun 3 23:07:43 2023 +0800 [CALCITE-5745] 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#3241 --- .../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* < 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); }
