alex-plekhanov commented on code in PR #10059:
URL: https://github.com/apache/ignite/pull/10059#discussion_r890161128


##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/logical/SqlScriptRunner.java:
##########
@@ -174,24 +175,36 @@ private List<List<?>> sql(String sql) {
     private static String toString(Object res) {
         if (res instanceof byte[])
             return ByteString.toString((byte[])res, 16);
+        else if (res instanceof List)
+            return listToString((List<?>)res);

Review Comment:
   Looks redundant. Why standard toString for lists is not enough? 



##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/StdSqlOperatorsTest.java:
##########
@@ -101,6 +103,21 @@ public void testAggregates() {
         assertExpression("LISTAGG(val, ',') WITHIN GROUP (ORDER BY val 
DESC)").returns("1").check();
         assertExpression("GROUP_CONCAT(val, ',' ORDER BY val 
DESC)").returns("1").check();
         assertExpression("STRING_AGG(val, ',' ORDER BY val 
DESC)").returns("1").check();
+        assertExpression("ARRAY_AGG(val ORDER BY val 
DESC)").returns(Collections.singletonList(1)).check();
+        assertQuery("SELECT ARRAY_CONCAT_AGG(a ORDER BY CARDINALITY(a)) FROM " 
+
+            "(SELECT 1 as id, ARRAY[3, 4, 5, 6] as a UNION SELECT 1 as id, 
ARRAY[1, 2] as a) GROUP BY id")
+            .returns((IntStream.range(1, 
7).boxed().collect(Collectors.toList()))).check();
+    }
+
+    /** */
+    @Test
+    public void testCollect() {

Review Comment:
   This method duplicate checks for "map from query" and "array from query" in 
`testQueryAsCollections`, let's remove it.



##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/StdSqlOperatorsTest.java:
##########
@@ -271,7 +270,9 @@ public void testPosixRegex() {
 
     /** */
     @Test
-    public void testCollections() {
+    public void testQueryAsCollections() {

Review Comment:
   There is only two queries which tests query as collections, old method names 
were correct.
   



##########
modules/calcite/src/test/sql/types/collections/array_agg.test_slow:
##########
@@ -1,25 +1,6 @@
-# name: test/sql/types/list/array_agg.test
-# description: Test array_agg function
-# group: [list]
-# Ignore https://issues.apache.org/jira/browse/IGNITE-15620
-
-# scalar array agg
-query II
-SELECT ARRAY_AGG(NULL), ARRAY_AGG(42)
-----
-[NULL] [42]
-
-# simple array agg
-query I
-SELECT ARRAY_AGG(i) FROM range(0, 3) tbl(i)
-----
-[0, 1, 2]
-
-# empty array agg
-query I
-SELECT ARRAY_AGG(i) FROM range(0, 0) tbl(i)
-----
-NULL
+# name: test/sql/types/collections/array_agg.test_slow

Review Comment:
   I think it's better to have all array_agg tests in one file (no need to 
divide into .test and .test_slow), but it's up to you.



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to