You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@flink.apache.org by ja...@apache.org on 2019/08/09 13:02:49 UTC

[flink] 01/04: [FLINK-13547][table-planner-blink] Refactor CONCAT() and CONCAT_WS() to keep it compatible with old planner

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

jark pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/flink.git

commit 5e697cf148f2d4ea09b0117d4c22102de98440c4
Author: Zhenghua Gao <do...@gmail.com>
AuthorDate: Fri Aug 2 17:29:47 2019 +0800

    [FLINK-13547][table-planner-blink] Refactor CONCAT() and CONCAT_WS() to keep it compatible with old planner
    
    CONCAT(string1, string2, ...) should returns NULL if any argument is NULL.
    CONCAT_WS(sep, string1, string2,...) should returns NULL if sep is NULL and automatically skips NULL arguments.
---
 .../planner/expressions/ScalarFunctionsTest.scala  |  6 ++---
 .../flink/table/dataformat/BinaryStringUtil.java   | 28 +++++++++++-----------
 .../flink/table/dataformat/BinaryStringTest.java   | 12 +++++-----
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/expressions/ScalarFunctionsTest.scala b/flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/expressions/ScalarFunctionsTest.scala
index 0fb58bf..56fa16d 100644
--- a/flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/expressions/ScalarFunctionsTest.scala
+++ b/flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/expressions/ScalarFunctionsTest.scala
@@ -731,7 +731,7 @@ class ScalarFunctionsTest extends ScalarTypesTestBase {
       concat("xx", 'f33),
       "concat('xx', f33)",
       "CONCAT('xx', f33)",
-      "xx")
+      "null")
     testAllApis(
       concat("AA", "BB", "CC", "---"),
       "concat('AA','BB','CC','---')",
@@ -745,7 +745,7 @@ class ScalarFunctionsTest extends ScalarTypesTestBase {
 
     testSqlApi("concat(f35)", "a")
     testSqlApi("concat(f35,f36)", "ab")
-    testSqlApi("concat(f35,f36,f33)", "ab")
+    testSqlApi("concat(f35,f36,f33)", "null")
   }
 
   @Test
@@ -754,7 +754,7 @@ class ScalarFunctionsTest extends ScalarTypesTestBase {
       concat_ws('f33, "AA"),
       "concat_ws(f33, 'AA')",
       "CONCAT_WS(f33, 'AA')",
-      "AA")
+      "null")
     testAllApis(
       concat_ws("~~~~", "AA"),
       "concat_ws('~~~~','AA')",
diff --git a/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/dataformat/BinaryStringUtil.java b/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/dataformat/BinaryStringUtil.java
index ecc6152..6faa51a 100644
--- a/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/dataformat/BinaryStringUtil.java
+++ b/flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/dataformat/BinaryStringUtil.java
@@ -717,22 +717,22 @@ public class BinaryStringUtil {
 
 	/**
 	 * Concatenates input strings together into a single string.
+	 * Returns NULL if any argument is NULL.
 	 */
 	public static BinaryString concat(BinaryString... inputs) {
 		return concat(Arrays.asList(inputs));
 	}
 
-	/**
-	 * Concatenates input strings together into a single string.
-	 */
 	public static BinaryString concat(Iterable<BinaryString> inputs) {
 		// Compute the total length of the result.
 		int totalLength = 0;
 		for (BinaryString input : inputs) {
-			if (input != null) {
-				input.ensureMaterialized();
-				totalLength += input.getSizeInBytes();
+			if (input == null) {
+				return null;
 			}
+
+			input.ensureMaterialized();
+			totalLength += input.getSizeInBytes();
 		}
 
 		// Allocate a new byte array, and copy the inputs one by one into it.
@@ -749,21 +749,21 @@ public class BinaryStringUtil {
 	}
 
 	/**
-	 * Concatenates input strings together into a single string using the separator.
-	 * A null input is skipped. For example, concat(",", "a", null, "c") would yield "a,c".
+	 * <p>Concatenates input strings together into a single string using the separator.
+	 * Returns NULL If the separator is NULL.</p>
+	 *
+	 * <p>Note: CONCAT_WS() does not skip any empty strings, however it does skip any NULL values after
+	 * the separator. For example, concat_ws(",", "a", null, "c") would yield "a,c".</p>
 	 */
 	public static BinaryString concatWs(BinaryString separator, BinaryString... inputs) {
 		return concatWs(separator, Arrays.asList(inputs));
 	}
 
-	/**
-	 * Concatenates input strings together into a single string using the separator.
-	 * A null input is skipped. For example, concat(",", "a", null, "c") would yield "a,c".
-	 */
 	public static BinaryString concatWs(BinaryString separator, Iterable<BinaryString> inputs) {
-		if (null == separator || EMPTY_UTF8.equals(separator)) {
-			return concat(inputs);
+		if (null == separator) {
+			return null;
 		}
+
 		separator.ensureMaterialized();
 
 		int numInputBytes = 0;  // total number of bytes from the inputs
diff --git a/flink-table/flink-table-runtime-blink/src/test/java/org/apache/flink/table/dataformat/BinaryStringTest.java b/flink-table/flink-table-runtime-blink/src/test/java/org/apache/flink/table/dataformat/BinaryStringTest.java
index 59c93ec..a3e2c63 100644
--- a/flink-table/flink-table-runtime-blink/src/test/java/org/apache/flink/table/dataformat/BinaryStringTest.java
+++ b/flink-table/flink-table-runtime-blink/src/test/java/org/apache/flink/table/dataformat/BinaryStringTest.java
@@ -247,22 +247,22 @@ public class BinaryStringTest {
 	@Test
 	public void concatTest() {
 		assertEquals(empty, concat());
-		assertEquals(empty, concat((BinaryString) null));
+		assertEquals(null, concat((BinaryString) null));
 		assertEquals(empty, concat(empty));
 		assertEquals(fromString("ab"), concat(fromString("ab")));
 		assertEquals(fromString("ab"), concat(fromString("a"), fromString("b")));
 		assertEquals(fromString("abc"), concat(fromString("a"), fromString("b"), fromString("c")));
-		assertEquals(fromString("ac"), concat(fromString("a"), null, fromString("c")));
-		assertEquals(fromString("a"), concat(fromString("a"), null, null));
-		assertEquals(empty, concat(null, null, null));
+		assertEquals(null, concat(fromString("a"), null, fromString("c")));
+		assertEquals(null, concat(fromString("a"), null, null));
+		assertEquals(null, concat(null, null, null));
 		assertEquals(fromString("数据砖头"), concat(fromString("数据"), fromString("砖头")));
 	}
 
 	@Test
 	public void concatWsTest() {
 		// Returns empty if the separator is null
-		assertEquals(empty, concatWs(null, (BinaryString) null));
-		assertEquals(fromString("a"), concatWs(null, fromString("a")));
+		assertEquals(null, concatWs(null, (BinaryString) null));
+		assertEquals(null, concatWs(null, fromString("a")));
 
 		// If separator is null, concatWs should skip all null inputs and never return null.
 		BinaryString sep = fromString("哈哈");