liuyongvs commented on PR #22917:
URL: https://github.com/apache/flink/pull/22917#issuecomment-1614765756
> > the return type is not right. it should always return nullable.
>
> It seem the return in this class is always nullable. so why you change it?
>
> ```
> /** Specific {@link TypeStrategy} for {@link
BuiltInFunctionDefinitions#ARRAY_MAX}. */
> @Internal
> public class ArrayElementOutputTypeStrategy implements TypeStrategy {
> @Override
> public Optional<DataType> inferType(CallContext callContext) {
> DataType inputDataType = callContext.getArgumentDataTypes().get(0);
> if (inputDataType.getLogicalType().getTypeRoot() !=
LogicalTypeRoot.ARRAY) {
> return Optional.empty();
> }
> final DataType elementDataType = ((CollectionDataType)
inputDataType).getElementDataType();
> if (inputDataType.getLogicalType().isNullable()) {
> return Optional.of(elementDataType.nullable());
> } else {
> return Optional.of(elementDataType);
> }
> }
> }
> ```
>
> So can you tell me which cases will have bug when you use this
TypeStrategy, I've been thinking about this for a long time. By the way, if you
want change code, why don't use change the code base on this class
>
> like this
>
> ```
> /** Specific {@link TypeStrategy} for {@link
BuiltInFunctionDefinitions#ARRAY_MAX}. */
> @Internal
> public class ArrayElementOutputTypeStrategy implements TypeStrategy {
> @Override
> public Optional<DataType> inferType(CallContext callContext) {
> DataType inputDataType = callContext.getArgumentDataTypes().get(0);
> final DataType elementDataType = ((CollectionDataType)
inputDataType).getElementDataType();
> return Optional.of(elementDataType);
> }
> }
> ```
>
> if you remove this Class, it seem you have to change more Classes. If the
bug exits, it seem only remove the if statement can solve the problem. So why
we have to change lots of other Classes? @liuyongvs
@hanyuzheng7
we don't every function's type create a new class.
if you look the calcite code, you will know why？
1) if it can get directly, like this
https://github.com/apache/calcite/commit/baa6b104f33a851e419bd95e18acf3766aa19838
in flink like map_keys/map_values/map_entries
2) if it can not get directly, like this
https://github.com/apache/calcite/commit/313b9580591f3816e498feb26a8953602c6029c0
in flink like other functions type
so flink shold do it too.
--
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]
