snuyanzin commented on code in PR #21947:
URL: https://github.com/apache/flink/pull/21947#discussion_r1108313393


##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/CollectionFunctionsITCase.java:
##########
@@ -160,6 +160,142 @@ Stream<TestSetSpec> getTestSetSpecs() {
                                     null
                                 },
                                 DataTypes.ARRAY(
-                                        DataTypes.ROW(DataTypes.BOOLEAN(), 
DataTypes.DATE()))));
+                                        DataTypes.ROW(DataTypes.BOOLEAN(), 
DataTypes.DATE()))),
+                TestSetSpec.forFunction(BuiltInFunctionDefinitions.ARRAY_SIZE)
+                        .onFieldsWithData(
+                                new Integer[] {1, 2, 3},
+                                new Integer[] {null, 1, 2, 3, 4, 5, 4, 3, 2, 
1, null},
+                                null,
+                                new String[] {"Hello", "Hello", "Hello"},
+                                new Row[] {
+                                    Row.of(true, LocalDate.of(2022, 4, 20)),
+                                    Row.of(true, LocalDate.of(1990, 10, 14)),
+                                    Row.of(true, LocalDate.of(1990, 10, 14)),
+                                    Row.of(true, LocalDate.of(1990, 10, 14)),
+                                    null
+                                })
+                        .andDataTypes(
+                                DataTypes.ARRAY(DataTypes.INT()),
+                                DataTypes.ARRAY(DataTypes.INT()),
+                                DataTypes.ARRAY(DataTypes.INT()),
+                                DataTypes.ARRAY(DataTypes.STRING()).notNull(),
+                                DataTypes.ARRAY(
+                                        DataTypes.ROW(DataTypes.BOOLEAN(), 
DataTypes.DATE())))
+                        .testResult(
+                                $("f0").arraySize(),
+                                "ARRAY_SIZE(f0)",
+                                3,
+                                DataTypes.INT().nullable())
+                        .testResult(
+                                $("f1").arraySize(),
+                                "ARRAY_SIZE(f1)",
+                                11,
+                                DataTypes.INT().nullable())
+                        .testResult(
+                                $("f2").arraySize(),
+                                "ARRAY_SIZE(f2)",
+                                null,
+                                DataTypes.INT().nullable())
+                        // ARRAY<STRING> NOT NULL
+                        .testResult(
+                                $("f3").arraySize(), "ARRAY_SIZE(f3)", 3, 
DataTypes.INT().notNull())
+                        .testResult(
+                                $("f4").arraySize(),
+                                "ARRAY_SIZE(f4)",
+                                5,
+                                DataTypes.INT().nullable()),
+                
TestSetSpec.forFunction(BuiltInFunctionDefinitions.ARRAY_REMOVE)
+                        .onFieldsWithData(
+                                new Integer[] {1, 2, 2},
+                                null,
+                                new String[] {"Hello", "World"},
+                                new Row[] {
+                                    Row.of(true, LocalDate.of(2022, 4, 20)),
+                                    Row.of(true, LocalDate.of(1990, 10, 14)),
+                                    null
+                                },
+                                new Integer[] {null, null, 1})
+                        .andDataTypes(
+                                DataTypes.ARRAY(DataTypes.INT()),
+                                DataTypes.ARRAY(DataTypes.INT()),
+                                DataTypes.ARRAY(DataTypes.STRING()).notNull(),
+                                DataTypes.ARRAY(
+                                        DataTypes.ROW(DataTypes.BOOLEAN(), 
DataTypes.DATE())),
+                                DataTypes.ARRAY(DataTypes.INT()))
+                        // ARRAY<INT>
+                        .testResult(
+                                $("f0").arrayRemove(2),
+                                "ARRAY_REMOVE(f0, 2)",
+                                new Integer[] {1},
+                                DataTypes.ARRAY(DataTypes.INT()).nullable())
+                        .testResult(
+                                $("f0").arrayRemove(42),
+                                "ARRAY_REMOVE(f0, 42)",
+                                new Integer[] {1, 2, 2},
+                                DataTypes.ARRAY(DataTypes.INT()).nullable())
+                        // ARRAY<INT> of null value
+                        .testResult(
+                                $("f1").arrayRemove(12),
+                                "ARRAY_REMOVE(f1, 12)",
+                                null,
+                                DataTypes.ARRAY(DataTypes.INT()).nullable())
+                        .testResult(
+                                $("f1").arrayRemove(null),
+                                "ARRAY_REMOVE(f1, NULL)",
+                                null,
+                                DataTypes.ARRAY(DataTypes.INT()).nullable())
+                        // ARRAY<STRING> NOT NULL
+                        .testResult(
+                                $("f2").arrayRemove("Hello"),
+                                "ARRAY_REMOVE(f2, 'Hello')",
+                                new String[] {"World"},
+                                DataTypes.ARRAY(DataTypes.STRING()).notNull())
+                        // ARRAY<ROW<BOOLEAN, DATE>>
+                        .testResult(
+                                $("f3").arrayRemove(row(true, 
LocalDate.of(1990, 10, 14))),
+                                "ARRAY_REMOVE(f3, (TRUE, DATE '1990-10-14'))",
+                                new Row[] {Row.of(true, LocalDate.of(2022, 4, 
20)), null},
+                                DataTypes.ARRAY(
+                                                DataTypes.ROW(
+                                                        DataTypes.BOOLEAN(), 
DataTypes.DATE()))
+                                        .nullable())
+                        .testResult(
+                                $("f3").arrayRemove(row(false, 
LocalDate.of(1990, 10, 14))),
+                                "ARRAY_REMOVE(f3, (FALSE, DATE '1990-10-14'))",
+                                new Row[] {
+                                    Row.of(true, LocalDate.of(2022, 4, 20)),
+                                    Row.of(true, LocalDate.of(1990, 10, 14)),
+                                    null
+                                },
+                                DataTypes.ARRAY(
+                                                DataTypes.ROW(
+                                                        DataTypes.BOOLEAN(), 
DataTypes.DATE()))
+                                        .nullable())
+                        .testResult(
+                                $("f3").arrayRemove(null),
+                                "ARRAY_REMOVE(f3, null)",
+                                new Row[] {
+                                    Row.of(true, LocalDate.of(2022, 4, 20)),
+                                    Row.of(true, LocalDate.of(1990, 10, 14)),
+                                },
+                                DataTypes.ARRAY(
+                                                DataTypes.ROW(
+                                                        DataTypes.BOOLEAN(), 
DataTypes.DATE()))
+                                        .nullable())
+                        // ARRAY<INT> with null elements

Review Comment:
   I would also add tests for cases with not null elements and removal of 
`null`.
   E.g.
   ```sql
   select array_remove(array[1, 2], cast(null as int));
   ```
   i would expect same array as output, or how do other system behave?



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/CollectionFunctionsITCase.java:
##########
@@ -160,6 +160,142 @@ Stream<TestSetSpec> getTestSetSpecs() {
                                     null
                                 },
                                 DataTypes.ARRAY(
-                                        DataTypes.ROW(DataTypes.BOOLEAN(), 
DataTypes.DATE()))));
+                                        DataTypes.ROW(DataTypes.BOOLEAN(), 
DataTypes.DATE()))),
+                TestSetSpec.forFunction(BuiltInFunctionDefinitions.ARRAY_SIZE)
+                        .onFieldsWithData(
+                                new Integer[] {1, 2, 3},
+                                new Integer[] {null, 1, 2, 3, 4, 5, 4, 3, 2, 
1, null},
+                                null,
+                                new String[] {"Hello", "Hello", "Hello"},
+                                new Row[] {
+                                    Row.of(true, LocalDate.of(2022, 4, 20)),
+                                    Row.of(true, LocalDate.of(1990, 10, 14)),
+                                    Row.of(true, LocalDate.of(1990, 10, 14)),
+                                    Row.of(true, LocalDate.of(1990, 10, 14)),
+                                    null
+                                })
+                        .andDataTypes(
+                                DataTypes.ARRAY(DataTypes.INT()),
+                                DataTypes.ARRAY(DataTypes.INT()),
+                                DataTypes.ARRAY(DataTypes.INT()),
+                                DataTypes.ARRAY(DataTypes.STRING()).notNull(),
+                                DataTypes.ARRAY(
+                                        DataTypes.ROW(DataTypes.BOOLEAN(), 
DataTypes.DATE())))
+                        .testResult(
+                                $("f0").arraySize(),
+                                "ARRAY_SIZE(f0)",
+                                3,
+                                DataTypes.INT().nullable())
+                        .testResult(
+                                $("f1").arraySize(),
+                                "ARRAY_SIZE(f1)",
+                                11,
+                                DataTypes.INT().nullable())
+                        .testResult(
+                                $("f2").arraySize(),
+                                "ARRAY_SIZE(f2)",
+                                null,
+                                DataTypes.INT().nullable())
+                        // ARRAY<STRING> NOT NULL
+                        .testResult(
+                                $("f3").arraySize(), "ARRAY_SIZE(f3)", 3, 
DataTypes.INT().notNull())
+                        .testResult(
+                                $("f4").arraySize(),
+                                "ARRAY_SIZE(f4)",
+                                5,
+                                DataTypes.INT().nullable()),
+                
TestSetSpec.forFunction(BuiltInFunctionDefinitions.ARRAY_REMOVE)
+                        .onFieldsWithData(
+                                new Integer[] {1, 2, 2},
+                                null,
+                                new String[] {"Hello", "World"},
+                                new Row[] {
+                                    Row.of(true, LocalDate.of(2022, 4, 20)),
+                                    Row.of(true, LocalDate.of(1990, 10, 14)),
+                                    null
+                                },
+                                new Integer[] {null, null, 1})
+                        .andDataTypes(
+                                DataTypes.ARRAY(DataTypes.INT()),
+                                DataTypes.ARRAY(DataTypes.INT()),
+                                DataTypes.ARRAY(DataTypes.STRING()).notNull(),
+                                DataTypes.ARRAY(
+                                        DataTypes.ROW(DataTypes.BOOLEAN(), 
DataTypes.DATE())),
+                                DataTypes.ARRAY(DataTypes.INT()))
+                        // ARRAY<INT>
+                        .testResult(
+                                $("f0").arrayRemove(2),
+                                "ARRAY_REMOVE(f0, 2)",
+                                new Integer[] {1},
+                                DataTypes.ARRAY(DataTypes.INT()).nullable())
+                        .testResult(
+                                $("f0").arrayRemove(42),
+                                "ARRAY_REMOVE(f0, 42)",
+                                new Integer[] {1, 2, 2},
+                                DataTypes.ARRAY(DataTypes.INT()).nullable())
+                        // ARRAY<INT> of null value
+                        .testResult(
+                                $("f1").arrayRemove(12),
+                                "ARRAY_REMOVE(f1, 12)",
+                                null,
+                                DataTypes.ARRAY(DataTypes.INT()).nullable())
+                        .testResult(
+                                $("f1").arrayRemove(null),
+                                "ARRAY_REMOVE(f1, NULL)",
+                                null,
+                                DataTypes.ARRAY(DataTypes.INT()).nullable())
+                        // ARRAY<STRING> NOT NULL
+                        .testResult(
+                                $("f2").arrayRemove("Hello"),
+                                "ARRAY_REMOVE(f2, 'Hello')",
+                                new String[] {"World"},
+                                DataTypes.ARRAY(DataTypes.STRING()).notNull())
+                        // ARRAY<ROW<BOOLEAN, DATE>>
+                        .testResult(
+                                $("f3").arrayRemove(row(true, 
LocalDate.of(1990, 10, 14))),
+                                "ARRAY_REMOVE(f3, (TRUE, DATE '1990-10-14'))",
+                                new Row[] {Row.of(true, LocalDate.of(2022, 4, 
20)), null},
+                                DataTypes.ARRAY(
+                                                DataTypes.ROW(
+                                                        DataTypes.BOOLEAN(), 
DataTypes.DATE()))
+                                        .nullable())
+                        .testResult(
+                                $("f3").arrayRemove(row(false, 
LocalDate.of(1990, 10, 14))),
+                                "ARRAY_REMOVE(f3, (FALSE, DATE '1990-10-14'))",
+                                new Row[] {
+                                    Row.of(true, LocalDate.of(2022, 4, 20)),
+                                    Row.of(true, LocalDate.of(1990, 10, 14)),
+                                    null
+                                },
+                                DataTypes.ARRAY(
+                                                DataTypes.ROW(
+                                                        DataTypes.BOOLEAN(), 
DataTypes.DATE()))
+                                        .nullable())
+                        .testResult(
+                                $("f3").arrayRemove(null),
+                                "ARRAY_REMOVE(f3, null)",
+                                new Row[] {
+                                    Row.of(true, LocalDate.of(2022, 4, 20)),
+                                    Row.of(true, LocalDate.of(1990, 10, 14)),
+                                },
+                                DataTypes.ARRAY(
+                                                DataTypes.ROW(
+                                                        DataTypes.BOOLEAN(), 
DataTypes.DATE()))
+                                        .nullable())
+                        // ARRAY<INT> with null elements

Review Comment:
   I would also add tests for cases with not null elements and removal of 
`null`.
   E.g.
   ```sql
   select array_remove(array[1, 2], cast(null as int));
   ```
   i would expect same array as output, or how do other systems behave?



-- 
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: issues-unsubscr...@flink.apache.org

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

Reply via email to