YiwenWu commented on code in PR #3830:
URL: https://github.com/apache/calcite/pull/3830#discussion_r1659507964


##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -7464,30 +7471,51 @@ void checkRegexpExtract(SqlOperatorFixture f0, 
FunctionAlias functionAlias) {
 
   /** Tests {@code ARRAY_REVERSE} function from BigQuery. */
   @Test void testArrayReverseFunc() {
+    final SqlFunction func = SqlLibraryOperators.ARRAY_REVERSE;
     final SqlOperatorFixture f0 = fixture();
-    f0.setFor(SqlLibraryOperators.ARRAY_REVERSE);
+    f0.setFor(func);
     f0.checkFails("^array_reverse(array[1])^",
         "No match found for function signature ARRAY_REVERSE\\(<INTEGER 
ARRAY>\\)", false);
-    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
-    f.checkScalar("array_reverse(array[1])", "[1]",
-        "INTEGER NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[1, 2])", "[2, 1]",
-        "INTEGER NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[null, 1])", "[1, null]",
-        "INTEGER ARRAY NOT NULL");
-    // elements cast
-    f.checkScalar("array_reverse(array[cast(1 as tinyint), 2])", "[2, 1]",
-        "INTEGER NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[null, 1, cast(2 as tinyint)])", "[2, 1, 
null]",
-        "INTEGER ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[cast(1 as bigint), 2])", "[2, 1]",
-        "BIGINT NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[null, 1, cast(2 as bigint)])", "[2, 1, 
null]",
-        "BIGINT ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[cast(1 as decimal), 2])", "[2, 1]",
-        "DECIMAL(19, 0) NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[null, 1, cast(2 as decimal)])", "[2, 1, 
null]",
-        "DECIMAL(19, 0) ARRAY NOT NULL");
+    checkArrayReverseFunc(f0, func, list(SqlLibrary.BIG_QUERY));
+  }
+
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-6445";>[CALCITE-6445]
+   * Add REVERSE function (enabled in Spark library)</a>. */
+  @Test void testReverseSparkFunc() {
+    final SqlFunction func = SqlLibraryOperators.REVERSE_SPARK;
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(func);
+    Iterable<SqlLibrary> libraries = list(SqlLibrary.SPARK);
+    checkArrayReverseFunc(f0, func, libraries);
+    checReverseFunc(f0, func, libraries);
+  }
+
+  void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
+      Iterable<? extends SqlLibrary> libraries) {
+    final String fn = function.getName();
+    final Consumer<SqlOperatorFixture> consumer = f -> {
+      f.checkScalar(fn + "(array[1])", "[1]",
+          "INTEGER NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[1, 2])", "[2, 1]",
+          "INTEGER NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[null, 1])", "[1, null]",
+          "INTEGER ARRAY NOT NULL");
+      // elements cast
+      f.checkScalar(fn + "(array[cast(1 as tinyint), 2])", "[2, 1]",
+          "INTEGER NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[null, 1, cast(2 as tinyint)])", "[2, 1, 
null]",
+          "INTEGER ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[cast(1 as bigint), 2])", "[2, 1]",
+          "BIGINT NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[null, 1, cast(2 as bigint)])", "[2, 1, null]",
+          "BIGINT ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[cast(1 as decimal), 2])", "[2, 1]",
+          "DECIMAL(19, 0) NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[null, 1, cast(2 as decimal)])", "[2, 1, 
null]",

Review Comment:
   "NULL literal cast to an array type." I don’t quite get this.
   
   Add more cases to validate NULL: 
   <img width="331" alt="image" 
src="https://github.com/apache/calcite/assets/7956306/12fe58af-9296-4dcf-9252-291e5f964d56";>
   



##########
site/_docs/reference.md:
##########
@@ -2850,6 +2850,7 @@ In the following:
 | b | REGEXP_SUBSTR(string, regexp [, position [, occurrence]]) | Synonym for 
REGEXP_EXTRACT
 | b m p s | REPEAT(string, integer)                  | Returns a string 
consisting of *string* repeated of *integer* times; returns an empty string if 
*integer* is less than 1
 | b m | REVERSE(string)                              | Returns *string* with 
the order of the characters reversed
+| s | REVERSE( string | array )                      | Returns *string* or 
*array* with the order of the characters reversed

Review Comment:
   Done, updated the description



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -7464,30 +7471,51 @@ void checkRegexpExtract(SqlOperatorFixture f0, 
FunctionAlias functionAlias) {
 
   /** Tests {@code ARRAY_REVERSE} function from BigQuery. */
   @Test void testArrayReverseFunc() {
+    final SqlFunction func = SqlLibraryOperators.ARRAY_REVERSE;
     final SqlOperatorFixture f0 = fixture();
-    f0.setFor(SqlLibraryOperators.ARRAY_REVERSE);
+    f0.setFor(func);
     f0.checkFails("^array_reverse(array[1])^",
         "No match found for function signature ARRAY_REVERSE\\(<INTEGER 
ARRAY>\\)", false);
-    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
-    f.checkScalar("array_reverse(array[1])", "[1]",
-        "INTEGER NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[1, 2])", "[2, 1]",
-        "INTEGER NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[null, 1])", "[1, null]",
-        "INTEGER ARRAY NOT NULL");
-    // elements cast
-    f.checkScalar("array_reverse(array[cast(1 as tinyint), 2])", "[2, 1]",
-        "INTEGER NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[null, 1, cast(2 as tinyint)])", "[2, 1, 
null]",
-        "INTEGER ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[cast(1 as bigint), 2])", "[2, 1]",
-        "BIGINT NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[null, 1, cast(2 as bigint)])", "[2, 1, 
null]",
-        "BIGINT ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[cast(1 as decimal), 2])", "[2, 1]",
-        "DECIMAL(19, 0) NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[null, 1, cast(2 as decimal)])", "[2, 1, 
null]",
-        "DECIMAL(19, 0) ARRAY NOT NULL");
+    checkArrayReverseFunc(f0, func, list(SqlLibrary.BIG_QUERY));
+  }
+
+  /**
+   * Tests {@code REVERSE} function from Spark.
+   */
+  @Test void testReverseSparkFunc() {
+    final SqlFunction func = SqlLibraryOperators.REVERSE_SPARK;
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(func);
+    Iterable<SqlLibrary> libraries = list(SqlLibrary.SPARK);
+    checkArrayReverseFunc(f0, func, libraries);
+    checReverseFunc(f0, func, libraries);
+  }
+
+  void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
+      Iterable<? extends SqlLibrary> libraries) {
+    final String fn = function.getName();
+    final Consumer<SqlOperatorFixture> consumer = f -> {
+      f.checkScalar(fn + "(array[1])", "[1]",
+          "INTEGER NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[1, 2])", "[2, 1]",
+          "INTEGER NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[null, 1])", "[1, null]",
+          "INTEGER ARRAY NOT NULL");
+      // elements cast
+      f.checkScalar(fn + "(array[cast(1 as tinyint), 2])", "[2, 1]",
+          "INTEGER NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[null, 1, cast(2 as tinyint)])", "[2, 1, 
null]",
+          "INTEGER ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[cast(1 as bigint), 2])", "[2, 1]",
+          "BIGINT NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[null, 1, cast(2 as bigint)])", "[2, 1, null]",
+          "BIGINT ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[cast(1 as decimal), 2])", "[2, 1]",
+          "DECIMAL(19, 0) NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[null, 1, cast(2 as decimal)])", "[2, 1, 
null]",

Review Comment:
   Added more boundary test cases:
   <img width="494" alt="image" 
src="https://github.com/apache/calcite/assets/7956306/c81d5398-8eb3-496e-af9b-6f0e7546dfb7";>
   



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -7464,30 +7471,51 @@ void checkRegexpExtract(SqlOperatorFixture f0, 
FunctionAlias functionAlias) {
 
   /** Tests {@code ARRAY_REVERSE} function from BigQuery. */
   @Test void testArrayReverseFunc() {
+    final SqlFunction func = SqlLibraryOperators.ARRAY_REVERSE;
     final SqlOperatorFixture f0 = fixture();
-    f0.setFor(SqlLibraryOperators.ARRAY_REVERSE);
+    f0.setFor(func);
     f0.checkFails("^array_reverse(array[1])^",
         "No match found for function signature ARRAY_REVERSE\\(<INTEGER 
ARRAY>\\)", false);
-    final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
-    f.checkScalar("array_reverse(array[1])", "[1]",
-        "INTEGER NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[1, 2])", "[2, 1]",
-        "INTEGER NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[null, 1])", "[1, null]",
-        "INTEGER ARRAY NOT NULL");
-    // elements cast
-    f.checkScalar("array_reverse(array[cast(1 as tinyint), 2])", "[2, 1]",
-        "INTEGER NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[null, 1, cast(2 as tinyint)])", "[2, 1, 
null]",
-        "INTEGER ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[cast(1 as bigint), 2])", "[2, 1]",
-        "BIGINT NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[null, 1, cast(2 as bigint)])", "[2, 1, 
null]",
-        "BIGINT ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[cast(1 as decimal), 2])", "[2, 1]",
-        "DECIMAL(19, 0) NOT NULL ARRAY NOT NULL");
-    f.checkScalar("array_reverse(array[null, 1, cast(2 as decimal)])", "[2, 1, 
null]",
-        "DECIMAL(19, 0) ARRAY NOT NULL");
+    checkArrayReverseFunc(f0, func, list(SqlLibrary.BIG_QUERY));
+  }
+
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-6445";>[CALCITE-6445]
+   * Add REVERSE function (enabled in Spark library)</a>. */
+  @Test void testReverseSparkFunc() {
+    final SqlFunction func = SqlLibraryOperators.REVERSE_SPARK;
+    final SqlOperatorFixture f0 = fixture();
+    f0.setFor(func);
+    Iterable<SqlLibrary> libraries = list(SqlLibrary.SPARK);
+    checkArrayReverseFunc(f0, func, libraries);
+    checReverseFunc(f0, func, libraries);
+  }
+
+  void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
+      Iterable<? extends SqlLibrary> libraries) {
+    final String fn = function.getName();
+    final Consumer<SqlOperatorFixture> consumer = f -> {
+      f.checkScalar(fn + "(array[1])", "[1]",
+          "INTEGER NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[1, 2])", "[2, 1]",
+          "INTEGER NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[null, 1])", "[1, null]",
+          "INTEGER ARRAY NOT NULL");
+      // elements cast
+      f.checkScalar(fn + "(array[cast(1 as tinyint), 2])", "[2, 1]",
+          "INTEGER NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[null, 1, cast(2 as tinyint)])", "[2, 1, 
null]",
+          "INTEGER ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[cast(1 as bigint), 2])", "[2, 1]",
+          "BIGINT NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[null, 1, cast(2 as bigint)])", "[2, 1, null]",
+          "BIGINT ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[cast(1 as decimal), 2])", "[2, 1]",
+          "DECIMAL(19, 0) NOT NULL ARRAY NOT NULL");
+      f.checkScalar(fn + "(array[null, 1, cast(2 as decimal)])", "[2, 1, 
null]",

Review Comment:
   Added cases in `spark.iq` and `SqlOperatorTest`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to