hanyuzheng7 commented on code in PR #22834:
URL: https://github.com/apache/flink/pull/22834#discussion_r1251051509
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java:
##########
@@ -275,6 +275,20 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL)
"org.apache.flink.table.runtime.functions.scalar.ArrayReverseFunction")
.build();
+ public static final BuiltInFunctionDefinition ARRAY_SLICE =
+ BuiltInFunctionDefinition.newBuilder()
+ .name("ARRAY_SLICE")
+ .kind(SCALAR)
+ .inputTypeStrategy(
+ sequence(
+ logical(LogicalTypeRoot.ARRAY),
+ logical(LogicalTypeRoot.INTEGER),
+ logical(LogicalTypeRoot.INTEGER)))
+ .outputTypeStrategy(nullableIfArgs(argument(0)))
Review Comment:
I saw the jira again, no one suggest third arg could be optional.
```
1. Rename args. IMHO Google Spanner[1] has the best naming
ARRAY_SLICE(array_to_slice, start_offset, end_offset), other also have ok
naming. The main issue is that the vendors I mentioned here support zero and
negative third arg and in this case it is not clear what is negative length for
instance.
2. Return NULL only if one of the input args is NULL. Otherwise return
sliced array or empty array. This is the behavior of Google Spanner[1], Cosmos
DB[2], ClickHouse[3], DuckDB[4], Snowflake[5].
3. It is ok to have the third arg 0 or negative, non-NULL value should be
returned
4. Clickhouse[3] and Cosmos DB[2] have the third arg as optional meaning
that if it is not specified the array will be sliced till the end (similar to
substring). Itwould make sense to have this as well
```
--
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]