mihaibudiu commented on code in PR #3684:
URL: https://github.com/apache/calcite/pull/3684#discussion_r1486819673
##########
babel/src/test/java/org/apache/calcite/test/BabelQuidemTest.java:
##########
@@ -131,6 +131,16 @@ public static void main(String[] args) throws Exception {
SqlConformanceEnum.BABEL)
.with(CalciteConnectionProperty.LENIENT_OPERATOR_LOOKUP, true)
.connect();
+ case "scott-spark":
Review Comment:
Is this related to the JIRA case?
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1074,6 +1080,26 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding
operatorBinding,
.withOperandTypeInference(InferTypes.RETURN_TYPE)
.withKind(SqlKind.CONCAT_WS_MSSQL);
+ /** The "CONCAT_WS(separator[, str | array(str)]+)" function in (SPARK).
Review Comment:
according to this documentation it also allows a mix of strings and arrays,
is that correct?
If it accepts just one argument the + should be a *
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -1074,6 +1080,26 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding
operatorBinding,
.withOperandTypeInference(InferTypes.RETURN_TYPE)
.withKind(SqlKind.CONCAT_WS_MSSQL);
+ /** The "CONCAT_WS(separator[, str | array(str)]+)" function in (SPARK).
+ *
+ * <p>Differs from {@link #CONCAT_WS} (MySQL, Postgres) in that it accepts
+ * 1 arguments
+ *
+ * <ul>
+ * <li>{@code CONCAT_WS(',', 'a', 'b')} returns "{@code a,b}";
+ * <li>{@code CONCAT_WS(null, 'a', 'b')} returns NULL";
+ * <li>{@code CONCAT_WS('s')} returns "";
+ * <li>{@code CONCAT_WS('/', 'a', null, 'b')} returns "{@code a/b}".
+ * </ul> */
+ @LibraryOperator(libraries = {SPARK})
+ public static final SqlFunction CONCAT_WS_SPARK =
+ SqlBasicFunction.create("CONCAT_WS",
+
ReturnTypes.MULTIVALENT_STRING_WITH_SEP_SUM_PRECISION_ARG0_NULLABLE,
Review Comment:
I don't see where the case for an argument with type array is handled
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -2259,6 +2272,27 @@ private static void
checkConcatWithSeparatorInMSSQL(SqlOperatorFixture f) {
f.checkString("concat_ws('', '', '', '')", "", "VARCHAR(0) NOT NULL");
}
+ private static void checkConcatWithSeparatorInSpark(SqlOperatorFixture f) {
+ f.setFor(SqlLibraryOperators.CONCAT_WS_SPARK);
+ f.checkString("concat_ws(',', 'a')", "a", "VARCHAR(1) NOT NULL");
+ f.checkString("concat_ws(',', 'a', 'b', null, 'c')", "a,b,c",
+ "VARCHAR NOT NULL");
+ f.checkString("concat_ws(',', cast('a' as varchar), cast('b' as varchar))",
+ "a,b", "VARCHAR NOT NULL");
+ f.checkString("concat_ws(',', cast('a' as varchar(2)), cast('b' as
varchar(1)))",
+ "a,b", "VARCHAR(4) NOT NULL");
+ f.checkString("concat_ws(',', '', '', '')", ",,", "VARCHAR(2) NOT NULL");
+ f.checkString("concat_ws(',', null, null, null)", "", "VARCHAR NOT NULL");
+ // returns null if the separator is null
+ f.checkNull("concat_ws(null, 'a', 'b')");
+ f.checkNull("concat_ws(null, null, null)");
+ f.checkString("concat_ws(',')", "", "VARCHAR(0) NOT NULL");
+ // if the separator is empty string, it's equivalent to CONCAT
+ f.checkString("concat_ws('', cast('a' as varchar(2)), cast('b' as
varchar(1)))",
Review Comment:
how about a few tests with arguments of types that are not string/null?
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -2259,6 +2272,27 @@ private static void
checkConcatWithSeparatorInMSSQL(SqlOperatorFixture f) {
f.checkString("concat_ws('', '', '', '')", "", "VARCHAR(0) NOT NULL");
}
+ private static void checkConcatWithSeparatorInSpark(SqlOperatorFixture f) {
+ f.setFor(SqlLibraryOperators.CONCAT_WS_SPARK);
+ f.checkString("concat_ws(',', 'a')", "a", "VARCHAR(1) NOT NULL");
+ f.checkString("concat_ws(',', 'a', 'b', null, 'c')", "a,b,c",
+ "VARCHAR NOT NULL");
+ f.checkString("concat_ws(',', cast('a' as varchar), cast('b' as varchar))",
+ "a,b", "VARCHAR NOT NULL");
+ f.checkString("concat_ws(',', cast('a' as varchar(2)), cast('b' as
varchar(1)))",
+ "a,b", "VARCHAR(4) NOT NULL");
+ f.checkString("concat_ws(',', '', '', '')", ",,", "VARCHAR(2) NOT NULL");
+ f.checkString("concat_ws(',', null, null, null)", "", "VARCHAR NOT NULL");
+ // returns null if the separator is null
+ f.checkNull("concat_ws(null, 'a', 'b')");
+ f.checkNull("concat_ws(null, null, null)");
+ f.checkString("concat_ws(',')", "", "VARCHAR(0) NOT NULL");
+ // if the separator is empty string, it's equivalent to CONCAT
+ f.checkString("concat_ws('', cast('a' as varchar(2)), cast('b' as
varchar(1)))",
+ "ab", "VARCHAR(3) NOT NULL");
+ f.checkString("concat_ws('', '', '', '')", "", "VARCHAR(0) NOT NULL");
Review Comment:
you are not testing arrays as arguments.
##########
site/_docs/reference.md:
##########
@@ -2700,12 +2700,14 @@ In the following:
| s | BIT_GET(value, position) | Returns the bit (0 or
1) value at the specified *position* of numeric *value*. The positions are
numbered from right to left, starting at zero. The *position* argument cannot
be negative
| b | CEIL(value) | Similar to standard
`CEIL(value)` except if *value* is an integer type, the return type is a double
| m s | CHAR(integer) | Returns the character
whose ASCII code is *integer* % 256, or null if *integer* < 0
+| s | CHR(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*
| b | CODE_POINTS_TO_BYTES(integers) | Converts *integers*, an
array of integers between 0 and 255 inclusive, into bytes; throws error if any
element is out of range
| b | CODE_POINTS_TO_STRING(integers) | Converts *integers*, an
array of integers between 0 and 0xD7FF or between 0xE000 and 0x10FFFF
inclusive, into string; throws error if any element is out of range
| o | CONCAT(string, string) | Concatenates two
strings, returns null only when both string arguments are null, otherwise
treats null as empty string
| b m | CONCAT(string [, string ]*) | Concatenates one or
more strings, returns null if any of the arguments is null
| p q | CONCAT(string [, string ]*) | Concatenates one or
more strings, null is treated as empty string
+| s | CONCAT_WS(separator, [, string ]*) | Concatenates one or
more strings, returns null only when separator is null, otherwise treats null
arguments as empty strings, returns empty when there are only separator
Review Comment:
this does not say anything about arrays of strings
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
