NobiGo commented on code in PR #4027:
URL: https://github.com/apache/calcite/pull/4027#discussion_r1825735353


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -914,6 +914,29 @@ public static List<String> split(String s, String 
delimiter) {
     }
   }
 
+
+  /** SQL {@code SPLIT_PART(string, string, int)} function. */
+  public static String splitPart(String s, String delimiter, int n) {
+    if (s == null || s.equals("") || delimiter == null || delimiter.isEmpty() 
|| n == 0) {

Review Comment:
   Using `Strings.isNullOrEmpty(s)` will be simple.



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -10901,6 +10901,26 @@ void checkEndsWith(SqlOperatorFixture f0, 
FunctionAlias functionAlias) {
             + "requires extra delimiter argument", false);
   }
 
+  /** Tests the {@code SPLIT_PART} operator. */
+  @Test void testSplitPartFunction() {
+    final SqlOperatorFixture f0 = 
fixture().setFor(SqlLibraryOperators.SPLIT_PART);
+    f0.checkFails("^split_part('hello', 1)^",
+        "No match found for function signature SPLIT_PART\\(<CHARACTER>, 
<NUMERIC>\\)",
+        false);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.POSTGRESQL);
+
+    f.checkScalar("SPLIT_PART('abc~@~def~@~ghi', '~@~', 2)", "def", 
"VARCHAR(15) NOT NULL");

Review Comment:
   Do we should return `VARCHAR(15)` for this function? I want to confirm is 
there a document explaining?



##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -914,6 +914,29 @@ public static List<String> split(String s, String 
delimiter) {
     }
   }
 
+
+  /** SQL {@code SPLIT_PART(string, string, int)} function. */
+  public static String splitPart(String s, String delimiter, int n) {
+    if (s == null || s.equals("") || delimiter == null || delimiter.isEmpty() 
|| n == 0) {
+      return "";
+    }
+
+    String[] parts = s.split(Pattern.quote(delimiter), -1);

Review Comment:
   Why do we need `Pattern.quote`?



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -10901,6 +10901,26 @@ void checkEndsWith(SqlOperatorFixture f0, 
FunctionAlias functionAlias) {
             + "requires extra delimiter argument", false);
   }
 
+  /** Tests the {@code SPLIT_PART} operator. */

Review Comment:
   Add unit test to cover `REDSHIFT`? I notice this function is except for 
`REDSHIFT`.



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -10901,6 +10901,26 @@ void checkEndsWith(SqlOperatorFixture f0, 
FunctionAlias functionAlias) {
             + "requires extra delimiter argument", false);
   }
 
+  /** Tests the {@code SPLIT_PART} operator. */
+  @Test void testSplitPartFunction() {
+    final SqlOperatorFixture f0 = 
fixture().setFor(SqlLibraryOperators.SPLIT_PART);
+    f0.checkFails("^split_part('hello', 1)^",

Review Comment:
   There should be three parameters?



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -10901,6 +10901,26 @@ void checkEndsWith(SqlOperatorFixture f0, 
FunctionAlias functionAlias) {
             + "requires extra delimiter argument", false);
   }
 
+  /** Tests the {@code SPLIT_PART} operator. */
+  @Test void testSplitPartFunction() {
+    final SqlOperatorFixture f0 = 
fixture().setFor(SqlLibraryOperators.SPLIT_PART);
+    f0.checkFails("^split_part('hello', 1)^",
+        "No match found for function signature SPLIT_PART\\(<CHARACTER>, 
<NUMERIC>\\)",
+        false);
+    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.POSTGRESQL);
+
+    f.checkScalar("SPLIT_PART('abc~@~def~@~ghi', '~@~', 2)", "def", 
"VARCHAR(15) NOT NULL");
+    f.checkScalar("SPLIT_PART('abc,def,ghi,jkl', ',', 3)", "ghi", "VARCHAR(15) 
NOT NULL");
+
+    f.checkScalar("SPLIT_PART('abc~@~def~@~ghi', '~@~', -1)", "ghi", 
"VARCHAR(15) NOT NULL");
+    f.checkScalar("SPLIT_PART('abc,def,ghi,jkl', ',', -2)", "ghi", 
"VARCHAR(15) NOT NULL");
+
+    f.checkScalar("SPLIT_PART('h,e,l,l,o', ',', 7)", "", "VARCHAR(9) NOT 
NULL");
+    f.checkScalar("SPLIT_PART('h,e,l,l,o', ',', -7)", "", "VARCHAR(9) NOT 
NULL");
+
+    f.checkScalar("SPLIT_PART('abc,,ghi', ',', 2)", "", "VARCHAR(8) NOT NULL");

Review Comment:
   Add unit test to cover NULL parameter.



-- 
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]

Reply via email to