Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on PR #3684: URL: https://github.com/apache/calcite/pull/3684#issuecomment-2044211803 > What is the status of this PR? @YiwenWu can you please rebase to eliminate the conflicts? Thank you, This PR is out of date and will be split into multiple PRs. I closed it. -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu closed pull request #3684: [CALCITE-6260] Add already implemented functions in Spark library URL: https://github.com/apache/calcite/pull/3684 -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
mihaibudiu commented on PR #3684: URL: https://github.com/apache/calcite/pull/3684#issuecomment-2043741104 What is the status of this PR? @YiwenWu can you please rebase to eliminate the conflicts? Thank you, -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on PR #3684: URL: https://github.com/apache/calcite/pull/3684#issuecomment-1958460920 Thanks for the review, I will split this PR later. -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
tanclary commented on code in PR #3684: URL: https://github.com/apache/calcite/pull/3684#discussion_r1498154787 ## core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java: ## @@ -231,13 +233,15 @@ import static org.apache.calcite.sql.fun.SqlLibraryOperators.PARSE_TIMESTAMP; import static org.apache.calcite.sql.fun.SqlLibraryOperators.PARSE_URL; import static org.apache.calcite.sql.fun.SqlLibraryOperators.POW; +import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_CONTAINS; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_EXTRACT; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_EXTRACT_ALL; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_INSTR; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_REPLACE; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REPEAT; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REVERSE; +import static org.apache.calcite.sql.fun.SqlLibraryOperators.REVERSE2; Review Comment: Can we call it `REVERSE_SPARK` then? Similar to the SUBSTR or FLOOR functions, to me 2 just implies 2 operands, it doesn't tell me its specific to a library. Also you should write a RelToSqlConverter test to ensure it gets unparsed correctly given that the operator name is different than what you defined. -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
mihaibudiu commented on PR #3684: URL: https://github.com/apache/calcite/pull/3684#issuecomment-1957681165 This is turning into a very large PR. It probably should have been broken into 4 different PRs, one for each function to make it easier to review. -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
sonarcloud[bot] commented on PR #3684: URL: https://github.com/apache/calcite/pull/3684#issuecomment-1957212889 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3684) **Quality Gate passed** Issues  [17 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3684=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3684=false=true)  [95.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3684=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3684=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3684) -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684: URL: https://github.com/apache/calcite/pull/3684#discussion_r1497521713 ## 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 Review Comment: @caicancai The difference： - Spark [CHR](https://spark.apache.org/docs/latest/api/sql/index.html#chr)： Returns the ASCII character; - Currently defined CHR: returns the character UTF-8 code -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684:
URL: https://github.com/apache/calcite/pull/3684#discussion_r1497519634
##
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:
done, add the test mix of multiple arrays and 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684:
URL: https://github.com/apache/calcite/pull/3684#discussion_r1497514526
##
core/src/main/java/org/apache/calcite/util/BuiltInMethod.java:
##
@@ -468,6 +468,8 @@ public enum BuiltInMethod {
String[].class),
MULTI_STRING_CONCAT_WITH_SEPARATOR(SqlFunctions.class,
"concatMultiWithSeparator", String[].class),
+ MULTI_STRING_CONCAT_WITH_SEPARATOR_NULLABLE(SqlFunctions.class,
Review Comment:
done, this method has been refactored.
--
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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684:
URL: https://github.com/apache/calcite/pull/3684#discussion_r1497482552
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -1491,6 +1491,19 @@ public static String concatMultiWithNull(String... args)
{
public static String concatMultiWithSeparator(String... args) {
// the separator arg could be null
final String sep = args[0] == null ? "" : args[0];
+return concatMultiWithSeparator(sep, args);
+ }
+
+ /** SQL {@code CONCAT_WS(sep, arg1, arg2, ...)} function,
+ * return null as null sep . */
+ public static @Nullable String concatMultiWithSeparatorNullable(String...
args) {
+if(args[0] == null){
+ return null;
+}
+return concatMultiWithSeparator(args[0], args);
Review Comment:
done
--
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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684: URL: https://github.com/apache/calcite/pull/3684#discussion_r1497481966 ## core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java: ## @@ -231,13 +233,15 @@ import static org.apache.calcite.sql.fun.SqlLibraryOperators.PARSE_TIMESTAMP; import static org.apache.calcite.sql.fun.SqlLibraryOperators.PARSE_URL; import static org.apache.calcite.sql.fun.SqlLibraryOperators.POW; +import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_CONTAINS; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_EXTRACT; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_EXTRACT_ALL; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_INSTR; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_REPLACE; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REPEAT; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REVERSE; +import static org.apache.calcite.sql.fun.SqlLibraryOperators.REVERSE2; Review Comment: Spark supports the `REVERSE` function, but it is different from the `REVERSE` functions already defined in SqlLibraryOperators. Spark [REVERSE](https://spark.apache.org/docs/latest/api/sql/index.html#reverse)：supports (string|array); Currently defined REVERSE: only supports string ## core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java: ## @@ -231,13 +233,15 @@ import static org.apache.calcite.sql.fun.SqlLibraryOperators.PARSE_TIMESTAMP; import static org.apache.calcite.sql.fun.SqlLibraryOperators.PARSE_URL; import static org.apache.calcite.sql.fun.SqlLibraryOperators.POW; +import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_CONTAINS; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_EXTRACT; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_EXTRACT_ALL; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_INSTR; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_REPLACE; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REPEAT; import static org.apache.calcite.sql.fun.SqlLibraryOperators.REVERSE; +import static org.apache.calcite.sql.fun.SqlLibraryOperators.REVERSE2; Review Comment: Spark supports the `REVERSE` function, but it is different from the `REVERSE` functions already defined in SqlLibraryOperators. - Spark [REVERSE](https://spark.apache.org/docs/latest/api/sql/index.html#reverse)：supports (string|array); - Currently defined REVERSE: only supports string -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684: URL: https://github.com/apache/calcite/pull/3684#discussion_r1497477187 ## core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java: ## @@ -152,6 +152,7 @@ import static org.apache.calcite.sql.fun.SqlLibraryOperators.CEIL_BIG_QUERY; import static org.apache.calcite.sql.fun.SqlLibraryOperators.CHAR; import static org.apache.calcite.sql.fun.SqlLibraryOperators.CHR; +import static org.apache.calcite.sql.fun.SqlLibraryOperators.CHR2; Review Comment: Spark supports the `CHR` function, but it is different from the `CHR` functions already defined in SqlLibraryOperators. - Spark [`CHR`](https://spark.apache.org/docs/latest/api/sql/index.html#chr)： Returns the ASCII character; - Currently defined `CHR`：returns the character UTF-8 code -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
tanclary commented on code in PR #3684:
URL: https://github.com/apache/calcite/pull/3684#discussion_r1488379478
##
core/src/main/java/org/apache/calcite/util/BuiltInMethod.java:
##
@@ -468,6 +468,8 @@ public enum BuiltInMethod {
String[].class),
MULTI_STRING_CONCAT_WITH_SEPARATOR(SqlFunctions.class,
"concatMultiWithSeparator", String[].class),
+ MULTI_STRING_CONCAT_WITH_SEPARATOR_NULLABLE(SqlFunctions.class,
Review Comment:
Shouldn't `null` handling be done elsewhere? I don't think we should need an
extra entry.
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -231,13 +233,15 @@
import static org.apache.calcite.sql.fun.SqlLibraryOperators.PARSE_TIMESTAMP;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.PARSE_URL;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.POW;
+import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_CONTAINS;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_EXTRACT;
import static
org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_EXTRACT_ALL;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_INSTR;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_REPLACE;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REPEAT;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REVERSE;
+import static org.apache.calcite.sql.fun.SqlLibraryOperators.REVERSE2;
Review Comment:
Same comment as above
##
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:
+1
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -152,6 +152,7 @@
import static org.apache.calcite.sql.fun.SqlLibraryOperators.CEIL_BIG_QUERY;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.CHAR;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.CHR;
+import static org.apache.calcite.sql.fun.SqlLibraryOperators.CHR2;
Review Comment:
What is `CHR2`? Why do we need it?
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -1491,6 +1491,19 @@ public static String concatMultiWithNull(String... args)
{
public static String concatMultiWithSeparator(String... args) {
// the separator arg could be null
final String sep = args[0] == null ? "" : args[0];
+return concatMultiWithSeparator(sep, args);
+ }
+
+ /** SQL {@code CONCAT_WS(sep, arg1, arg2, ...)} function,
+ * return null as null sep . */
+ public static @Nullable String concatMultiWithSeparatorNullable(String...
args) {
+if(args[0] == null){
+ return null;
+}
+return concatMultiWithSeparator(args[0], args);
Review Comment:
Looks like there are some checkstyle issues.
--
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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684:
URL: https://github.com/apache/calcite/pull/3684#discussion_r1495812154
##
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:
Yes, `spark.iq` has been added under QuidemTest to ensure the correct
execution of new spark functions
--
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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
mihaibudiu commented on code in PR #3684:
URL: https://github.com/apache/calcite/pull/3684#discussion_r1495018416
##
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:
the way you wrote the function description, it can accept a mix of multiple
arrays and strings too.
Can you either write tests for that case or fix the description?
--
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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
caicancai commented on code in PR #3684: URL: https://github.com/apache/calcite/pull/3684#discussion_r1494627130 ## 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 Review Comment: @JiajunBernoulli @YiwenWu If some functions of spark are different from other sql engines, I do not recommend putting them in this jira, as this can easily cause confusion. We need to open a new jira to discuss it, what is the difference and what negative tests need to be added? -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
sonarcloud[bot] commented on PR #3684: URL: https://github.com/apache/calcite/pull/3684#issuecomment-1951227759 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3684) **Quality Gate passed** Issues  [20 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3684=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3684=false=true)  [95.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3684=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3684=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3684) -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
sonarcloud[bot] commented on PR #3684: URL: https://github.com/apache/calcite/pull/3684#issuecomment-1951215496 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3684) **Quality Gate passed** Issues  [20 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3684=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3684=false=true)  [95.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3684=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3684=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3684) -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684:
URL: https://github.com/apache/calcite/pull/3684#discussion_r1493743738
##
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).
+ *
+ * Differs from {@link #CONCAT_WS} (MySQL, Postgres) in that it accepts
+ * 1 arguments
+ *
+ *
+ * {@code CONCAT_WS(',', 'a', 'b')} returns "{@code a,b}";
+ * {@code CONCAT_WS(null, 'a', 'b')} returns NULL";
+ * {@code CONCAT_WS('s')} returns "";
+ * {@code CONCAT_WS('/', 'a', null, 'b')} returns "{@code a/b}".
+ * */
+ @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:
done, add array case
--
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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684: URL: https://github.com/apache/calcite/pull/3684#discussion_r1493743706 ## 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 0x10 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: done -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684:
URL: https://github.com/apache/calcite/pull/3684#discussion_r1493743574
##
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:
done, add int、double test
--
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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684:
URL: https://github.com/apache/calcite/pull/3684#discussion_r1493743503
##
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:
done, add arrays test
--
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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684: URL: https://github.com/apache/calcite/pull/3684#discussion_r1493740036 ## 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 Review Comment: I am not sure, have any suggestions for this case? -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
sonarcloud[bot] commented on PR #3684: URL: https://github.com/apache/calcite/pull/3684#issuecomment-1951145412 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3684) **Quality Gate passed** Issues  [20 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3684=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3684=false=true)  [95.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3684=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3684=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3684) -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
YiwenWu commented on code in PR #3684: URL: https://github.com/apache/calcite/pull/3684#discussion_r1487094978 ## 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: Spark support arrays, I missed it, I will add this case -- 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
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
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).
+ *
+ * Differs from {@link #CONCAT_WS} (MySQL, Postgres) in that it accepts
+ * 1 arguments
+ *
+ *
+ * {@code CONCAT_WS(',', 'a', 'b')} returns "{@code a,b}";
+ * {@code CONCAT_WS(null, 'a', 'b')} returns NULL";
+ * {@code CONCAT_WS('s')} returns "";
+ * {@code CONCAT_WS('/', 'a', null, 'b')} returns "{@code a/b}".
+ * */
+ @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");
Re: [PR] [CALCITE-6260] Add already implemented functions in Spark library [calcite]
sonarcloud[bot] commented on PR #3684: URL: https://github.com/apache/calcite/pull/3684#issuecomment-1939122808 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3684) **Quality Gate passed** Issues  [16 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3684=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3684=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3684=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3684=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3684) -- 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
