snuyanzin commented on code in PR #22834:
URL: https://github.com/apache/flink/pull/22834#discussion_r1242647572
##########
docs/data/sql_functions.yml:
##########
@@ -643,6 +643,9 @@ collection:
- sql: ARRAY_REVERSE(haystack)
table: haystack.arrayReverse()
description: Returns an array in reverse order. If the array itself is
null, the function will return null.
+ - sql: ARRAY_SLICE(array, start, length)
+ table: array.arraySlice(start, length)
+ description: Returns an array that is a subset of array, extracted based
on the specified start index and length. Supports both positive and negative
indices. Positive indices start from 1 (the first element) while negative
indices start from the end of the array (-1 being the last element). If array,
or start or length are null, null is returned. If the length exceeds the
available elements starting from the start index, all elements till the end of
the array are returned. If abs(start) is greater than the length of the array
or start equal to 0 or length is smaller than 0 or equal to 0, return null.
Review Comment:
`If array, or start or length are null, null is returned.` - that sounds
complicated... I guess it could be simplified to something like e.g. `In case
any of input arg is null it returns null`.
`If array, or start or length are null, null is returned. If the length
exceeds the available elements starting from the start index...` I feel like it
should be rephrased because it is not clear what length is implied to be here:
input arg or length of the array?
` If the length exceeds the available elements starting from the start
index, all elements till the end of the array are returned. ` What is `end of
array` here? A couple of sentences before it was mentioned `...the end of the
array (-1 being the last element)`. And with this context the statmenet is
incorrect.
Clickhouse uses `offset` [1] word instead of `start` and IMHO it fits better.
Also in case of wrong (non null) `offset` and `length` arg ClickHouse and
DuckDB return empty array which indicates that an empty slice for the input
data. Why should we return `null`?
[1]
https://clickhouse.com/docs/en/sql-reference/functions/array-functions#arrayslice
--
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]
