You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "zoudan (via GitHub)" <gi...@apache.org> on 2023/06/02 07:51:16 UTC
[GitHub] [calcite] zoudan opened a new pull request, #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
zoudan opened a new pull request, #3237:
URL: https://github.com/apache/calcite/pull/3237
(no comment)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578174573
Glad to hear that @zoudan !
I'll take a look at the PR when I have some time.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1219427285
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4080,6 +4080,46 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
+ StringBuilder sb = new StringBuilder();
+ int size = list.size();
+ for (int i = 0; i < size; i++) {
+ boolean shouldAddDelimiter = i != 0 && sb.length() > 0;
+ Object item = list.get(i);
+ String str;
+ if (item == null) {
+ str = null;
+ } else if (item instanceof String) {
+ str = (String) item;
+ } else if (item instanceof ByteString) {
+ str = item.toString();
+ } else {
+ throw new IllegalStateException(
+ "Only String or ByteString are supported but get " + item.getClass().getName());
+ }
+ if (str == null) {
Review Comment:
Perhaps we can simplify this code and avoid duplication by doing something like
```
if (str == null) {
if (nullText == null) {
continue;
}
else {
str = nullText;
}
}
if (addDelimiter) {
sb.append(delimiter);
}
sb.append(str);
```
Wdyt?
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4080,6 +4080,46 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
+ StringBuilder sb = new StringBuilder();
+ int size = list.size();
+ for (int i = 0; i < size; i++) {
+ boolean shouldAddDelimiter = i != 0 && sb.length() > 0;
+ Object item = list.get(i);
+ String str;
+ if (item == null) {
+ str = null;
+ } else if (item instanceof String) {
+ str = (String) item;
+ } else if (item instanceof ByteString) {
+ str = item.toString();
+ } else {
+ throw new IllegalStateException(
+ "Only String or ByteString are supported but get " + item.getClass().getName());
+ }
+ if (str == null) {
Review Comment:
Perhaps we can simplify this code and avoid duplication by doing something like
```
if (str == null) {
if (nullText == null) {
continue;
} else {
str = nullText;
}
}
if (addDelimiter) {
sb.append(delimiter);
}
sb.append(str);
```
Wdyt?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1219477427
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -937,6 +937,14 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
ReturnTypes.LEAST_RESTRICTIVE,
OperandTypes.AT_LEAST_ONE_SAME_VARIADIC);
+ /** The "ARRAY_TO_STRING(array, delimiter [, nullText ])" function. */
+ @LibraryOperator(libraries = {BIG_QUERY})
+ public static final SqlFunction ARRAY_TO_STRING =
+ SqlBasicFunction.create(SqlKind.ARRAY_TO_STRING,
+ ReturnTypes.explicit(SqlTypeName.VARCHAR)
Review Comment:
good idea
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1224478220
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,30 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
+ new FamilyOperandTypeChecker(
+ ImmutableList.of(SqlTypeFamily.ARRAY, SqlTypeFamily.STRING, SqlTypeFamily.STRING),
+ i -> i == 2) {
+ @SuppressWarnings("argument.type.incompatible")
+ @Override public boolean checkOperandTypes(
+ SqlCallBinding callBinding,
+ boolean throwOnFailure) {
+ if (!super.checkOperandTypes(callBinding, throwOnFailure)) {
+ return false;
+ }
+ RelDataType elementType = callBinding.getOperandType(0).getComponentType();
+ if (elementType == null || (!SqlTypeFamily.STRING.contains(elementType)
+ && !SqlTypeFamily.BINARY.contains(elementType))) {
+ if (throwOnFailure) {
+ throw callBinding.newValidationSignatureError();
+ }
+ return false;
+ }
+ return true;
+ }
Review Comment:
To have a better validation error, I think we should also override the signature with something like:
```
@Override public String getAllowedSignatures(SqlOperator op, String opName) {
return opName + "(<STRING|BINARY ARRAY>, <STRING>[, <STRING>])";
}
```
Could you please try it? (and adjust the corresponding unit test accordingly)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1223094480
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4102,6 +4102,43 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
+ StringBuilder sb = new StringBuilder();
+ for (Object item : list) {
+ String str;
+ if (item == null) {
+ str = null;
+ } else if (item instanceof String) {
+ str = (String) item;
+ } else if (item instanceof ByteString) {
+ str = item.toString();
+ } else {
+ throw new IllegalStateException(
+ "arrayToString supports only String or ByteString, but got "
+ + item.getClass().getName());
+ }
+ if (str == null) {
+ if (nullText == null) {
+ continue;
+ } else {
+ str = nullText;
+ }
+ }
+
+ if (sb.length() > 0) {
Review Comment:
@rubenada Thank you for your careful observation. Yes, this is the expected behavior:
`array_to_string(array['', 'b'], '-') => '-b'`
`array_to_string(array['', ''], '-') => '-' `
I have updated my code and also squashed the commits.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1224031080
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5474,6 +5474,29 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkNull("array_length(null)");
}
+ @Test void testArrayToStringFunc() {
+ final SqlOperatorFixture f0 = fixture();
+ f0.setFor(SqlLibraryOperators.ARRAY_TO_STRING);
+ f0.checkFails("^array_to_string(array['aa', 'b', 'c'], '-')^", "No match found for function"
+ + " signature ARRAY_TO_STRING\\(<CHAR\\(2\\) ARRAY>, <CHARACTER>\\)", false);
+
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+ f.checkScalar("array_to_string(array['aa', 'b', 'c'], '-')", "aa-b-c",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-', 'empty')",
+ "empty-aa-empty-b-empty", "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-')", "aa-b",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, x'aa', null, x'bb', null], '-')", "aa-bb",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array['', 'b'], '-')", "-b", "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array['', ''], '-')", "-", "VARCHAR NOT NULL");
+ f.checkNull("array_to_string(null, '-')");
+ f.checkNull("array_to_string(array['a', 'b', null], null)");
+ f.checkFails("array_to_string(array[1, 2, 3], '-')",
+ "arrayToString supports only String or ByteString, but got java.lang.Integer", true);
+ }
Review Comment:
good idea
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1224504358
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5474,6 +5474,29 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkNull("array_length(null)");
}
+ @Test void testArrayToStringFunc() {
+ final SqlOperatorFixture f0 = fixture();
+ f0.setFor(SqlLibraryOperators.ARRAY_TO_STRING);
+ f0.checkFails("^array_to_string(array['aa', 'b', 'c'], '-')^", "No match found for function"
+ + " signature ARRAY_TO_STRING\\(<CHAR\\(2\\) ARRAY>, <CHARACTER>\\)", false);
+
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+ f.checkScalar("array_to_string(array['aa', 'b', 'c'], '-')", "aa-b-c",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-', 'empty')",
+ "empty-aa-empty-b-empty", "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-')", "aa-b",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, x'aa', null, x'bb', null], '-')", "aa-bb",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array['', 'b'], '-')", "-b", "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array['', ''], '-')", "-", "VARCHAR NOT NULL");
+ f.checkNull("array_to_string(null, '-')");
+ f.checkNull("array_to_string(array['a', 'b', null], null)");
+ f.checkFails("array_to_string(array[1, 2, 3], '-')",
+ "arrayToString supports only String or ByteString, but got java.lang.Integer", true);
+ }
Review Comment:
@liuyongvs what do you think about the newer version (including the validation check)?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578795066
@rubenada Of course, updated
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1219418991
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4080,6 +4080,46 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
+ StringBuilder sb = new StringBuilder();
+ int size = list.size();
+ for (int i = 0; i < size; i++) {
+ boolean shouldAddDelimiter = i != 0 && sb.length() > 0;
Review Comment:
Would it be enough to just use this?
```
boolean addDelimiter = sb.length() > 0;
```
In which case, we could remove the `size` and for `i` and simply do:
```
for (Object item : list) {
```
Would that work?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578667333
@rubenada Thanks for your valuable advice, I have updated my branch, please take a look when you have time.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578698783
Thanks @zoudan . There is one check style violation (Line longer than 100 characters), could you please fix 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1584807131
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [91.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "JiajunBernoulli (via GitHub)" <gi...@apache.org>.
JiajunBernoulli commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1225037630
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4102,6 +4102,45 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
Review Comment:
Maybe `stream` is more simplify.
```
list.stream()
.map(item -> {
if (item == null) {
return nullText;
} else if (item instanceof String) {
return (String) item;
} else if (item instanceof ByteString) {
return item.toString();
} else {
throw new IllegalStateException(
"arrayToString supports only String or ByteString, but got "
+ item.getClass().getName());
}
}).collect(Collectors.joining(delimiter));
}
```
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1588763531
Thanks @zoudan , I will merge as soon as the CI checks are finished.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "JiajunBernoulli (via GitHub)" <gi...@apache.org>.
JiajunBernoulli commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1225656520
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4102,6 +4102,45 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
Review Comment:
Ok
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1224462179
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,57 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker ARRAY_OF_STRING_OR_BINARY_STRING_STRING_OPTIONAL_STRING
Review Comment:
Thank you for your guidance, I have updated my 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1226236342
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,34 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
+ new FamilyOperandTypeChecker(
Review Comment:
@liuyongvs, If I understand correctly, what @rubenada meant was we should use `SqlTypeFamily.CHARACTER` instead of `SqlTypeFamily.STRING` for the second and third parameters. That make sense, I will updated 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1582397730
@zoudan it seems some recent commits on Calcite master led to conflicts here, could you please resolve them and squash commits into a single one? I'll proceed with the merge afterwards.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1223065806
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4102,6 +4102,43 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
+ StringBuilder sb = new StringBuilder();
+ for (Object item : list) {
+ String str;
+ if (item == null) {
+ str = null;
+ } else if (item instanceof String) {
+ str = (String) item;
+ } else if (item instanceof ByteString) {
+ str = item.toString();
+ } else {
+ throw new IllegalStateException(
+ "arrayToString supports only String or ByteString, but got "
+ + item.getClass().getName());
+ }
+ if (str == null) {
+ if (nullText == null) {
+ continue;
+ } else {
+ str = nullText;
+ }
+ }
+
+ if (sb.length() > 0) {
Review Comment:
Sorry @zoudan , one last detail (rather an edge case): what is the expected behavior if the item (`str` in this point) is the empty string? Will it be appended like any other value with the corresponding delimiter before the precious value?
For example:
```
array_to_string(array['', 'b'], '-') => '-b' ???
array_to_string(array['', ''], '-') => '-' ???
```
If that is the case, I have the impression that this condition is wrong and we would need to find another mechanism, e.g.
```
boolean first = true;
for (Object item : list) {
...
if (!first) {
sb.append(delimiter);
}
sb.append(str);
first = false;
}
```
wdyt?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1584850249
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [91.4% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1225139247
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,34 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
+ new FamilyOperandTypeChecker(
+ ImmutableList.of(SqlTypeFamily.ARRAY, SqlTypeFamily.STRING, SqlTypeFamily.STRING),
+ i -> i == 2) {
+ @SuppressWarnings("argument.type.incompatible")
+ @Override public boolean checkOperandTypes(
+ SqlCallBinding callBinding,
+ boolean throwOnFailure) {
+ if (!super.checkOperandTypes(callBinding, throwOnFailure)) {
+ return false;
+ }
+ RelDataType elementType = callBinding.getOperandType(0).getComponentType();
+ if (elementType == null || (!SqlTypeFamily.STRING.contains(elementType)
+ && !SqlTypeFamily.BINARY.contains(elementType))) {
+ if (throwOnFailure) {
Review Comment:
if i understand correct, it can use SqlTypeUtil.isString.
string familiy has 4 type
```
public static final List<SqlTypeName> CHAR_TYPES =
ImmutableList.of(CHAR, VARCHAR);
public static final List<SqlTypeName> BINARY_TYPES =
ImmutableList.of(BINARY, VARBINARY);
```
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1223120747
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4102,6 +4102,43 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
+ StringBuilder sb = new StringBuilder();
+ for (Object item : list) {
+ String str;
+ if (item == null) {
+ str = null;
+ } else if (item instanceof String) {
+ str = (String) item;
+ } else if (item instanceof ByteString) {
+ str = item.toString();
+ } else {
+ throw new IllegalStateException(
+ "arrayToString supports only String or ByteString, but got "
+ + item.getClass().getName());
+ }
+ if (str == null) {
+ if (nullText == null) {
+ continue;
+ } else {
+ str = nullText;
+ }
+ }
+
+ if (sb.length() > 0) {
Review Comment:
Thanks @zoudan ! I saw that you also added some unit tests for this scenario, that's great.
I'll merge as soon as the CI checks are finished
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1584322389
@liuyongvs Thanks for you comments, I have updated my code, please take a look. cc @rubenada
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1225380618
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5474,6 +5474,29 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkNull("array_length(null)");
}
+ @Test void testArrayToStringFunc() {
+ final SqlOperatorFixture f0 = fixture();
+ f0.setFor(SqlLibraryOperators.ARRAY_TO_STRING);
+ f0.checkFails("^array_to_string(array['aa', 'b', 'c'], '-')^", "No match found for function"
+ + " signature ARRAY_TO_STRING\\(<CHAR\\(2\\) ARRAY>, <CHARACTER>\\)", false);
+
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+ f.checkScalar("array_to_string(array['aa', 'b', 'c'], '-')", "aa-b-c",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-', 'empty')",
+ "empty-aa-empty-b-empty", "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-')", "aa-b",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, x'aa', null, x'bb', null], '-')", "aa-bb",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array['', 'b'], '-')", "-b", "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array['', ''], '-')", "-", "VARCHAR NOT NULL");
+ f.checkNull("array_to_string(null, '-')");
+ f.checkNull("array_to_string(array['a', 'b', null], null)");
+ f.checkFails("array_to_string(array[1, 2, 3], '-')",
+ "arrayToString supports only String or ByteString, but got java.lang.Integer", true);
+ }
Review Comment:
@rubenada i have responsed
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1219428524
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4080,6 +4080,46 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
+ StringBuilder sb = new StringBuilder();
+ int size = list.size();
+ for (int i = 0; i < size; i++) {
+ boolean shouldAddDelimiter = i != 0 && sb.length() > 0;
+ Object item = list.get(i);
+ String str;
+ if (item == null) {
+ str = null;
+ } else if (item instanceof String) {
+ str = (String) item;
+ } else if (item instanceof ByteString) {
+ str = item.toString();
+ } else {
+ throw new IllegalStateException(
+ "Only String or ByteString are supported but get " + item.getClass().getName());
Review Comment:
Nitpick:
```
but get ...
=>
but got ...
```
Also I'd provide a bit more context in the exception message, e.g.
`arrayToString supports only String or ByteString, but got ....`
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1582671745
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [95.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1224342705
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,57 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker ARRAY_OF_STRING_OR_BINARY_STRING_STRING_OPTIONAL_STRING
Review Comment:
It's definitely positive to check at validation time the array has the expected content type.
I'd suggest to try to reuse the FamilyOperandTypeChecker (adding just the relevant code to check the array component type). Proposal (unverified):
```
public static final SqlOperandTypeChecker STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
new FamilyOperandTypeChecker(
ImmutableList.of(SqlTypeFamily.ARRAY, SqlTypeFamily.STRING, SqlTypeFamily.STRING),
i -> i == 2) {
@Override public boolean checkOperandTypes(
SqlCallBinding callBinding,
boolean throwOnFailure) {
if (!super.checkOperandTypes(callBinding, throwOnFailure)) {
return false;
}
RelDataType arrayType = callBinding.getOperandType(0).getComponentType();
if (arrayType == null || (!SqlTypeFamily.STRING.contains(arrayType)
&& !SqlTypeFamily.BINARY.contains(arrayType))) {
if (throwOnFailure) {
throw callBinding.newValidationSignatureError();
}
return false;
}
return true;
}
};
```
Regarding the checker framework error, if you cannot get rid of it, you can use `@SuppressWarnings("argument.type.incompatible")` .
If I am not mistaken, you should be able to run it locally in your dev environment via
`gradlew build -x test -PenableCheckerframework`
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1223631422
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5474,6 +5474,29 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkNull("array_length(null)");
}
+ @Test void testArrayToStringFunc() {
+ final SqlOperatorFixture f0 = fixture();
+ f0.setFor(SqlLibraryOperators.ARRAY_TO_STRING);
+ f0.checkFails("^array_to_string(array['aa', 'b', 'c'], '-')^", "No match found for function"
+ + " signature ARRAY_TO_STRING\\(<CHAR\\(2\\) ARRAY>, <CHARACTER>\\)", false);
+
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+ f.checkScalar("array_to_string(array['aa', 'b', 'c'], '-')", "aa-b-c",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-', 'empty')",
+ "empty-aa-empty-b-empty", "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-')", "aa-b",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, x'aa', null, x'bb', null], '-')", "aa-bb",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array['', 'b'], '-')", "-b", "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array['', ''], '-')", "-", "VARCHAR NOT NULL");
+ f.checkNull("array_to_string(null, '-')");
+ f.checkNull("array_to_string(array['a', 'b', null], null)");
+ f.checkFails("array_to_string(array[1, 2, 3], '-')",
+ "arrayToString supports only String or ByteString, but got java.lang.Integer", true);
+ }
Review Comment:
sorry for late review, it may not be right here "arrayToString supports only String or ByteString, but got java.lang.Integer", it should throw exception in validate type process, instead of runtime process
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1219416934
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5468,6 +5468,27 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkNull("array_length(null)");
}
+ @Test void testArrayToStringFunc() {
+ final SqlOperatorFixture f0 = fixture();
+ f0.setFor(SqlLibraryOperators.ARRAY_TO_STRING);
+ f0.checkFails("^array_to_string(array['aa', 'b', 'c'], '-')^", "No match found for function"
+ + " signature ARRAY_TO_STRING\\(<CHAR\\(2\\) ARRAY>, <CHARACTER>\\)", false);
+
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+ f.checkScalar("array_to_string(array['aa', 'b', 'c'], '-')", "aa-b-c",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-', 'empty')",
+ "empty-aa-empty-b-empty", "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-')", "aa-b",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, x'aa', null, x'bb', null], '-')", "aa-bb",
+ "VARCHAR NOT NULL");
+ f.checkNull("array_to_string(null, '-')");
+ f.checkNull("array_to_string(array['a', 'b', null], null)");
Review Comment:
Is this expected? Having a `null` delimiter shall lead to `null` result?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1219415037
##########
site/_docs/reference.md:
##########
@@ -2661,6 +2661,7 @@ BigQuery's type system uses confusingly different names for types and functions:
| s | ARRAY_REPEAT(element, count) | Returns the array containing element count times.
| b | ARRAY_REVERSE(array) | Reverses elements of *array*
| s | ARRAY_SIZE(array) | Synonym for `CARDINALITY`
+| b | ARRAY_TO_STRING(array, delimiter [, nullText ])| Returns a concatenation of the elements in *array* as a STRING and take *delimiter* as the delimiter. If the *nullText* parameter is used, the function replaces any `NULL` values in the array with the value of *nullText*. If the *nullText* parameter is not used, the function omits the `NULL` value and its preceding delimiter.
Review Comment:
Nitpick:
```
and take *delimiter* as the delimiter ...
=>
and takes *delimiter* as the delimiter ...
```
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578988256
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [94.8% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1225371019
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,34 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
+ new FamilyOperandTypeChecker(
+ ImmutableList.of(SqlTypeFamily.ARRAY, SqlTypeFamily.STRING, SqlTypeFamily.STRING),
+ i -> i == 2) {
+ @SuppressWarnings("argument.type.incompatible")
+ @Override public boolean checkOperandTypes(
+ SqlCallBinding callBinding,
+ boolean throwOnFailure) {
+ if (!super.checkOperandTypes(callBinding, throwOnFailure)) {
+ return false;
+ }
+ RelDataType elementType = callBinding.getOperandType(0).getComponentType();
+ if (elementType == null || (!SqlTypeFamily.STRING.contains(elementType)
+ && !SqlTypeFamily.BINARY.contains(elementType))) {
+ if (throwOnFailure) {
Review Comment:
make sense, updated
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1585680487
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [92.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1219509132
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5468,6 +5468,27 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkNull("array_length(null)");
}
+ @Test void testArrayToStringFunc() {
+ final SqlOperatorFixture f0 = fixture();
+ f0.setFor(SqlLibraryOperators.ARRAY_TO_STRING);
+ f0.checkFails("^array_to_string(array['aa', 'b', 'c'], '-')^", "No match found for function"
+ + " signature ARRAY_TO_STRING\\(<CHAR\\(2\\) ARRAY>, <CHARACTER>\\)", false);
+
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+ f.checkScalar("array_to_string(array['aa', 'b', 'c'], '-')", "aa-b-c",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-', 'empty')",
+ "empty-aa-empty-b-empty", "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-')", "aa-b",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, x'aa', null, x'bb', null], '-')", "aa-bb",
+ "VARCHAR NOT NULL");
+ f.checkNull("array_to_string(null, '-')");
+ f.checkNull("array_to_string(array['a', 'b', null], null)");
Review Comment:
Yes, this is the expected behavior.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578814913
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [94.8% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1219813460
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -5468,6 +5468,27 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkNull("array_length(null)");
}
+ @Test void testArrayToStringFunc() {
+ final SqlOperatorFixture f0 = fixture();
+ f0.setFor(SqlLibraryOperators.ARRAY_TO_STRING);
+ f0.checkFails("^array_to_string(array['aa', 'b', 'c'], '-')^", "No match found for function"
+ + " signature ARRAY_TO_STRING\\(<CHAR\\(2\\) ARRAY>, <CHARACTER>\\)", false);
+
+ final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+ f.checkScalar("array_to_string(array['aa', 'b', 'c'], '-')", "aa-b-c",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-', 'empty')",
+ "empty-aa-empty-b-empty", "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, 'aa', null, 'b', null], '-')", "aa-b",
+ "VARCHAR NOT NULL");
+ f.checkScalar("array_to_string(array[null, x'aa', null, x'bb', null], '-')", "aa-bb",
+ "VARCHAR NOT NULL");
+ f.checkNull("array_to_string(null, '-')");
+ f.checkNull("array_to_string(array['a', 'b', null], null)");
+ f.checkFails("array_to_string(array[1, 2, 3], '-')",
+ "Only String or ByteString are supported but get java.lang.Integer", true);
Review Comment:
I know that this is not really relevant for the test, but please align this message with the actual exception message.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1219413878
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -937,6 +937,14 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
ReturnTypes.LEAST_RESTRICTIVE,
OperandTypes.AT_LEAST_ONE_SAME_VARIADIC);
+ /** The "ARRAY_TO_STRING(array, delimiter [, nullText ])" function. */
+ @LibraryOperator(libraries = {BIG_QUERY})
+ public static final SqlFunction ARRAY_TO_STRING =
+ SqlBasicFunction.create(SqlKind.ARRAY_TO_STRING,
+ ReturnTypes.explicit(SqlTypeName.VARCHAR)
Review Comment:
There are already 5 places (this would be the 6th) in SqlLibraryOperators (and another one in SqlRegexpReplaceFunction) that use the code
`ReturnTypes.explicit(SqlTypeName.VARCHAR).andThen(SqlTypeTransforms.TO_NULLABLE)`
Why don't we just create in `ReturnTypes`:
```
public static final SqlReturnTypeInference VARCHAR_NULLABLE = explicit(SqlTypeName.VARCHAR).andThen(SqlTypeTransforms.TO_NULLABLE)
```
and use simply `ReturnTypes.VARCHAR_NULLABLE` in all those places (include the new ARRAY_TO_STRING definition)?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "JiajunBernoulli (via GitHub)" <gi...@apache.org>.
JiajunBernoulli commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1225024684
##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -427,6 +427,13 @@ public static SqlCall stripSeparator(SqlCall call) {
public static final SqlReturnTypeInference VARCHAR_2000_NULLABLE =
VARCHAR_2000.andThen(SqlTypeTransforms.TO_NULLABLE);
+ /**
+ * Type-inference strategy that always returns "VARCHAR" with nulls
+ * allowed if any of the operands allow nulls.
+ */
+ public static final SqlReturnTypeInference VARCHAR_NULLABLE =
Review Comment:
Can we declare another `VARCHAR`?
Because
`VARCHAR_4` -> `VARCHAR_4_NULLABLE`;
`VARCHAR_2000` -> `VARCHAR_2000_NULLABLE`;
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1588407339
> @liuyongvs @JiajunBernoulli thanks for your comments in the PR, do you have further remarks? Shall we consider squash commits and merge?
I have squashed the commits.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1227642472
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4102,6 +4102,45 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
+ StringBuilder sb = new StringBuilder();
+ boolean isFirst = true;
+ for (Object item : list) {
+ String str;
+ if (item == null) {
+ str = null;
+ } else if (item instanceof String) {
+ str = (String) item;
+ } else if (item instanceof ByteString) {
+ str = item.toString();
+ } else {
+ throw new IllegalStateException(
+ "arrayToString supports only String or ByteString, but got "
+ + item.getClass().getName());
+ }
+ if (str == null) {
Review Comment:
sorry @zoudan , just one last thing before the merge. I realized we could remove this `if` block and move its content to the first `if`, in order to make the code more compact:
```
for (Object item : list) {
String str;
if (item == null) {
if (nullText == null) {
continue;
} else {
str = nullText;
}
} else if (item instanceof String) {
...
```
wdyt?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1226236342
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,34 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
+ new FamilyOperandTypeChecker(
Review Comment:
@liuyongvs, If I understand correctly, what @rubenada meant was we should use `SqlTypeFamily.CHARACTER` instead of `SqlTypeFamily.STRING` for the second and third parameters. That make sense, I will update 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1227669999
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4102,6 +4102,45 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
+ StringBuilder sb = new StringBuilder();
+ boolean isFirst = true;
+ for (Object item : list) {
+ String str;
+ if (item == null) {
+ str = null;
+ } else if (item instanceof String) {
+ str = (String) item;
+ } else if (item instanceof ByteString) {
+ str = item.toString();
+ } else {
+ throw new IllegalStateException(
+ "arrayToString supports only String or ByteString, but got "
+ + item.getClass().getName());
+ }
+ if (str == null) {
Review Comment:
Agree, updated
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1576473560
@gf2121 It seems you have push something that has nothing to do with this issue, I do not known what happened.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578687265
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [94.8% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada merged pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada merged PR #3237:
URL: https://github.com/apache/calcite/pull/3237
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1223626462
##########
site/_docs/reference.md:
##########
@@ -2663,6 +2663,7 @@ BigQuery's type system uses confusingly different names for types and functions:
| s | ARRAY_REPEAT(element, count) | Returns the array containing element count times.
| b | ARRAY_REVERSE(array) | Reverses elements of *array*
| s | ARRAY_SIZE(array) | Synonym for `CARDINALITY`
+| b | ARRAY_TO_STRING(array, delimiter [, nullText ])| Returns a concatenation of the elements in *array* as a STRING and take *delimiter* as the delimiter. If the *nullText* parameter is used, the function replaces any `NULL` values in the array with the value of *nullText*. If the *nullText* parameter is not used, the function omits the `NULL` value and its preceding delimiter.
Review Comment:
remove end with '.'
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1584342623
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [86.8% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1582471285
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [94.8% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1226294799
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,34 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
+ new FamilyOperandTypeChecker(
Review Comment:
ok, i understand, thanks @zoudan
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1588423640
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [92.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578044312
@rubenada It seems a unrelated commit has been pushed to my branch. Do you know what happened? Should I force push to my branch to recover 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1580160013
@rubenada updated, please have a look when you have time
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1225139247
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,34 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
+ new FamilyOperandTypeChecker(
+ ImmutableList.of(SqlTypeFamily.ARRAY, SqlTypeFamily.STRING, SqlTypeFamily.STRING),
+ i -> i == 2) {
+ @SuppressWarnings("argument.type.incompatible")
+ @Override public boolean checkOperandTypes(
+ SqlCallBinding callBinding,
+ boolean throwOnFailure) {
+ if (!super.checkOperandTypes(callBinding, throwOnFailure)) {
+ return false;
+ }
+ RelDataType elementType = callBinding.getOperandType(0).getComponentType();
+ if (elementType == null || (!SqlTypeFamily.STRING.contains(elementType)
+ && !SqlTypeFamily.BINARY.contains(elementType))) {
+ if (throwOnFailure) {
Review Comment:
if i understand correct, it can use SqlTypeUtil.isString.
string familiy has 4 type
public static final List<SqlTypeName> CHAR_TYPES =
ImmutableList.of(CHAR, VARCHAR);
public static final List<SqlTypeName> BINARY_TYPES =
ImmutableList.of(BINARY, VARBINARY);
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1225141126
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,34 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
+ new FamilyOperandTypeChecker(
Review Comment:
personaly, the name STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING is not good. because the string contains BINARY family's type
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1226207689
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,34 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
+ new FamilyOperandTypeChecker(
Review Comment:
Good point, @liuyongvs. Since the STRING contains both CHARACTER and BINARY, I wonder if the type checker for the method should rather be `ImmutableList.of(SqlTypeFamily.ARRAY, SqlTypeFamily.CHARACTER, SqlTypeFamily.CHARACTER)`
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1226231780
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,34 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
+ new FamilyOperandTypeChecker(
Review Comment:
@rubenada it can not be. refers to the bigquery's doc https://cloud.google.com/bigquery/docs/reference/standard-sql/array_functions#array_to_string, the array's element type should be check in compiler process.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578161060
@rubenada This problem has been resolved after I rebase my branch and force push it, Thanks. Btw, it would be great if you could help review this PR
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578101401
I think there was a force push on Calcite main, and somehow you got caught up in between. I can see in this PR the commit 295df907d126ffa059756e2e0cc8b69051ff6da7 , which was my original commit to fix CALCITE-5730, but now it is detached (and the commit to fix that issue is now e34caf70ac97996035b8fd8fc494a64022b07f91 ).
Perhaps the simplest would be to close this PR and start a new one from scratch, using a new branch (making sure that you have the main branch up to date).
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1224342705
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,57 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker ARRAY_OF_STRING_OR_BINARY_STRING_STRING_OPTIONAL_STRING
Review Comment:
I'd suggst to try to reuse the FamilyOperandTypeChecker (adding just the relevant code to check the array component type). Proposal (unverified):
```
public static final SqlOperandTypeChecker STRING_ARRAY_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
new FamilyOperandTypeChecker(
ImmutableList.of(SqlTypeFamily.ARRAY, SqlTypeFamily.STRING, SqlTypeFamily.STRING),
i -> i == 2) {
@Override public boolean checkOperandTypes(
SqlCallBinding callBinding,
boolean throwOnFailure) {
if (!super.checkOperandTypes(callBinding, throwOnFailure)) {
return false;
}
RelDataType arrayType = callBinding.getOperandType(0).getComponentType();
if (arrayType == null || (!SqlTypeFamily.STRING.contains(arrayType)
&& !SqlTypeFamily.BINARY.contains(arrayType))) {
if (throwOnFailure) {
throw callBinding.newValidationSignatureError();
}
return false;
}
return true;
}
};
```
Regarding the checker framework error, if you cannot get rid of it, you can use `@SuppressWarnings("argument.type.incompatible")` .
If I am not mistaken, you should be able to run it locally in your dev environment via
`gradlew build -x test -PenableCheckerframework`
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578111263
@rubenada Maybe this could be resolved after I update my branch and force push 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578194762
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [95.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1578148110
@zoudan update your branch how? What git commands do you intend to execute exactly?
I'm afraid the problem is that [your branch contains the commit](https://github.com/zoudan/calcite/commits/CALCITE-5728) that was detached from Calcite master after the force-push (295df907d126ffa059756e2e0cc8b69051ff6da7), I'm not sure what's the best way to proceed in 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1225066147
##########
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##########
@@ -427,6 +427,13 @@ public static SqlCall stripSeparator(SqlCall call) {
public static final SqlReturnTypeInference VARCHAR_2000_NULLABLE =
VARCHAR_2000.andThen(SqlTypeTransforms.TO_NULLABLE);
+ /**
+ * Type-inference strategy that always returns "VARCHAR" with nulls
+ * allowed if any of the operands allow nulls.
+ */
+ public static final SqlReturnTypeInference VARCHAR_NULLABLE =
Review Comment:
Thanks for reviewing, updated
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1585473812
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [91.6% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1225056926
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4102,6 +4102,45 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
Review Comment:
Considering performance, I tend to not use lambda. WDYT?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1224342705
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,57 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker ARRAY_OF_STRING_OR_BINARY_STRING_STRING_OPTIONAL_STRING
Review Comment:
It's definitely positive to check at validation time the array has the expected content type.
I'd suggest to try to reuse the FamilyOperandTypeChecker (adding just the relevant code to check the array component type). Proposal (unverified):
```
public static final SqlOperandTypeChecker STRING_ARRAY_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
new FamilyOperandTypeChecker(
ImmutableList.of(SqlTypeFamily.ARRAY, SqlTypeFamily.STRING, SqlTypeFamily.STRING),
i -> i == 2) {
@Override public boolean checkOperandTypes(
SqlCallBinding callBinding,
boolean throwOnFailure) {
if (!super.checkOperandTypes(callBinding, throwOnFailure)) {
return false;
}
RelDataType arrayType = callBinding.getOperandType(0).getComponentType();
if (arrayType == null || (!SqlTypeFamily.STRING.contains(arrayType)
&& !SqlTypeFamily.BINARY.contains(arrayType))) {
if (throwOnFailure) {
throw callBinding.newValidationSignatureError();
}
return false;
}
return true;
}
};
```
Regarding the checker framework error, if you cannot get rid of it, you can use `@SuppressWarnings("argument.type.incompatible")` .
If I am not mistaken, you should be able to run it locally in your dev environment via
`gradlew build -x test -PenableCheckerframework`
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1224499961
##########
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##########
@@ -471,6 +471,30 @@ public static SqlOperandTypeChecker variadic(
.or(OperandTypes.family(SqlTypeFamily.MAP))
.or(OperandTypes.family(SqlTypeFamily.ANY));
+ public static final SqlOperandTypeChecker STRING_OR_BINARY_ARRAY_STRING_OPTIONAL_STRING =
+ new FamilyOperandTypeChecker(
+ ImmutableList.of(SqlTypeFamily.ARRAY, SqlTypeFamily.STRING, SqlTypeFamily.STRING),
+ i -> i == 2) {
+ @SuppressWarnings("argument.type.incompatible")
+ @Override public boolean checkOperandTypes(
+ SqlCallBinding callBinding,
+ boolean throwOnFailure) {
+ if (!super.checkOperandTypes(callBinding, throwOnFailure)) {
+ return false;
+ }
+ RelDataType elementType = callBinding.getOperandType(0).getComponentType();
+ if (elementType == null || (!SqlTypeFamily.STRING.contains(elementType)
+ && !SqlTypeFamily.BINARY.contains(elementType))) {
+ if (throwOnFailure) {
+ throw callBinding.newValidationSignatureError();
+ }
+ return false;
+ }
+ return true;
+ }
Review Comment:
updated
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1580213022
Thanks @zoudan , the PR looks in a good shape. Let's wait in case some other reviewer wants to chime in, otherwise we'll move forward.
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zoudan commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1582440689
@rubenada updated
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1573351880
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [94.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1586837256
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3237)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3237&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3237&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list) [92.5% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3237&metric=new_duplicated_lines_density&view=list)
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #3237:
URL: https://github.com/apache/calcite/pull/3237#issuecomment-1586934659
@liuyongvs @liuyongvs thanks for your comments in the PR, do you have further remarks? Shall we consider squash commits and merge?
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3237: [CALCITE-5728] Add ARRAY_TO_STRING function (enabled in BigQuery library)
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #3237:
URL: https://github.com/apache/calcite/pull/3237#discussion_r1219428524
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -4080,6 +4080,46 @@ public static List reverse(List list) {
return list;
}
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter)} function. */
+ public static String arrayToString(List list, String delimiter) {
+ return arrayToString(list, delimiter, null);
+ }
+
+ /** SQL {@code ARRAY_TO_STRING(array, delimiter, nullText)} function. */
+ public static String arrayToString(List list, String delimiter, @Nullable String nullText) {
+ StringBuilder sb = new StringBuilder();
+ int size = list.size();
+ for (int i = 0; i < size; i++) {
+ boolean shouldAddDelimiter = i != 0 && sb.length() > 0;
+ Object item = list.get(i);
+ String str;
+ if (item == null) {
+ str = null;
+ } else if (item instanceof String) {
+ str = (String) item;
+ } else if (item instanceof ByteString) {
+ str = item.toString();
+ } else {
+ throw new IllegalStateException(
+ "Only String or ByteString are supported but get " + item.getClass().getName());
Review Comment:
Nitpick:
```
but get ...
=>
but got ...
```
--
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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org