tanclary commented on code in PR #3684:
URL: https://github.com/apache/calcite/pull/3684#discussion_r1488379478
##########
core/src/main/java/org/apache/calcite/util/BuiltInMethod.java:
##########
@@ -468,6 +468,8 @@ public enum BuiltInMethod {
String[].class),
MULTI_STRING_CONCAT_WITH_SEPARATOR(SqlFunctions.class,
"concatMultiWithSeparator", String[].class),
+ MULTI_STRING_CONCAT_WITH_SEPARATOR_NULLABLE(SqlFunctions.class,
Review Comment:
Shouldn't `null` handling be done elsewhere? I don't think we should need an
extra entry.
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -231,13 +233,15 @@
import static org.apache.calcite.sql.fun.SqlLibraryOperators.PARSE_TIMESTAMP;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.PARSE_URL;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.POW;
+import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_CONTAINS;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_EXTRACT;
import static
org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_EXTRACT_ALL;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_INSTR;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REGEXP_REPLACE;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REPEAT;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.REVERSE;
+import static org.apache.calcite.sql.fun.SqlLibraryOperators.REVERSE2;
Review Comment:
Same comment as above
##########
babel/src/test/java/org/apache/calcite/test/BabelQuidemTest.java:
##########
@@ -131,6 +131,16 @@ public static void main(String[] args) throws Exception {
SqlConformanceEnum.BABEL)
.with(CalciteConnectionProperty.LENIENT_OPERATOR_LOOKUP, true)
.connect();
+ case "scott-spark":
Review Comment:
+1
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -152,6 +152,7 @@
import static org.apache.calcite.sql.fun.SqlLibraryOperators.CEIL_BIG_QUERY;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.CHAR;
import static org.apache.calcite.sql.fun.SqlLibraryOperators.CHR;
+import static org.apache.calcite.sql.fun.SqlLibraryOperators.CHR2;
Review Comment:
What is `CHR2`? Why do we need it?
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -1491,6 +1491,19 @@ public static String concatMultiWithNull(String... args)
{
public static String concatMultiWithSeparator(String... args) {
// the separator arg could be null
final String sep = args[0] == null ? "" : args[0];
+ return concatMultiWithSeparator(sep, args);
+ }
+
+ /** SQL {@code CONCAT_WS(sep, arg1, arg2, ...)} function,
+ * return null as null sep . */
+ public static @Nullable String concatMultiWithSeparatorNullable(String...
args) {
+ if(args[0] == null){
+ return null;
+ }
+ return concatMultiWithSeparator(args[0], args);
Review Comment:
Looks like there are some checkstyle issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
