dawidwys commented on code in PR #22951:
URL: https://github.com/apache/flink/pull/22951#discussion_r1487819172


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java:
##########
@@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL)
                             
"org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction")
                     .build();
 
+    public static final BuiltInFunctionDefinition ARRAY_SORT =
+            BuiltInFunctionDefinition.newBuilder()
+                    .name("ARRAY_SORT")
+                    .kind(SCALAR)
+                    .inputTypeStrategy(
+                            or(
+                                    sequence(new 
ArrayComparableElementArgumentTypeStrategy()),
+                                    sequence(
+                                            new 
ArrayComparableElementArgumentTypeStrategy(),
+                                            logical(LogicalTypeRoot.BOOLEAN))))

Review Comment:
   > Ok, thank you, actually, I did this function last summer and follow the 
jira tickect's https://issues.apache.org/jira/browse/FLINK-26948 description.
   
   I understand that. If at some point we find out we either made mistake or 
did not put enough effort into something it's better to fix that sooner rather 
than later when we need to leave with the consequences. I admit I have not 
thoroughly checked the semantics before which I should've.
   
   It's better to do something well rather than fast in my opinion.
   
   I see, so traditional RDBMS do not really support that function. It's also 
worth checking what does:
   * Snowflake: 
https://docs.snowflake.com/en/sql-reference/functions/array_sort (`null` when 
any argument is `null`), null handling separately
   * Spark: 
https://docs.databricks.com/en/sql/language-manual/functions/sort_array.html: 
from the docs it does not say what's the behaviour on `null` `ascendingOrder`, 
nulls first on asc, nulls last on desc
   * Presto: https://prestodb.io/docs/current/functions/array.html: has two 
separate functions for `ASC/DESC`
   
   To me Snowflake's behaviour is the cleanest out there. WDYT? @MartijnVisser 



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