VasShabu commented on code in PR #29416: URL: https://github.com/apache/flink/pull/29416#discussion_r4360918220
########## flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/EqualsComparableElementArgumentTypeStrategy.java: ########## @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.table.types.inference.strategies; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.functions.FunctionDefinition; +import org.apache.flink.table.types.DataType; +import org.apache.flink.table.types.inference.ArgumentTypeStrategy; +import org.apache.flink.table.types.inference.CallContext; +import org.apache.flink.table.types.inference.Signature.Argument; +import org.apache.flink.table.types.logical.LogicalType; +import org.apache.flink.table.types.logical.LogicalTypeRoot; +import org.apache.flink.table.types.logical.StructuredType.StructuredComparison; +import org.apache.flink.table.types.logical.utils.LogicalTypeChecks; +import org.apache.flink.util.Preconditions; + Review Comment: ```suggestion import org.apache.flink.table.types.logical.ArrayType; import org.apache.flink.table.types.logical.MapType; ``` may have to run spotless ########## flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/EqualsComparableElementArgumentTypeStrategy.java: ########## @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.table.types.inference.strategies; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.functions.FunctionDefinition; +import org.apache.flink.table.types.DataType; +import org.apache.flink.table.types.inference.ArgumentTypeStrategy; +import org.apache.flink.table.types.inference.CallContext; +import org.apache.flink.table.types.inference.Signature.Argument; +import org.apache.flink.table.types.logical.LogicalType; +import org.apache.flink.table.types.logical.LogicalTypeRoot; +import org.apache.flink.table.types.logical.StructuredType.StructuredComparison; +import org.apache.flink.table.types.logical.utils.LogicalTypeChecks; +import org.apache.flink.util.Preconditions; + +import java.util.List; +import java.util.Optional; + +/** + * An {@link ArgumentTypeStrategy} for an ARRAY argument whose elements, or a MAP argument whose + * keys, a function compares with each other. + * + * <p>The elements or keys must support equality, see {@link + * LogicalTypeChecks#areComparable(LogicalType, LogicalType, StructuredComparison)}. + * + * <p>{@link ArrayComparableElementArgumentTypeStrategy} is not reused. It only accepts numeric, + * datetime, character string and binary string elements, but the functions that use this strategy + * also compare ROW, ARRAY and structured elements. + */ +@Internal +public final class EqualsComparableElementArgumentTypeStrategy implements ArgumentTypeStrategy { + + private final ArgumentTypeStrategy collectionStrategy; + + public EqualsComparableElementArgumentTypeStrategy(LogicalTypeRoot collectionRoot) { + Preconditions.checkArgument( + collectionRoot == LogicalTypeRoot.ARRAY || collectionRoot == LogicalTypeRoot.MAP); + this.collectionStrategy = new RootArgumentTypeStrategy(collectionRoot, null); + } + + @Override + public Optional<DataType> inferArgumentType( + CallContext callContext, int argumentPos, boolean throwOnFailure) { + return collectionStrategy + .inferArgumentType(callContext, argumentPos, throwOnFailure) + .flatMap(type -> checkElementEquality(callContext, type, throwOnFailure)); + } + + @Override + public Argument getExpectedArgument(FunctionDefinition functionDefinition, int argumentPos) { + return collectionStrategy.getExpectedArgument(functionDefinition, argumentPos); + } + + /** Checks that the elements of an ARRAY type, or the keys of a MAP type, support equality. */ + static Optional<DataType> checkElementEquality( + CallContext callContext, DataType collectionType, boolean throwOnFailure) { + // the element type of an ARRAY and the key type of a MAP are the first child + final List<LogicalType> children = collectionType.getLogicalType().getChildren(); + // A hack to support legacy types. To be removed when we drop the legacy types. + if (children.isEmpty()) { + return Optional.of(collectionType); + } + final LogicalType elementType = children.get(0); Review Comment: ```suggestion final LogicalType logicalType = collectionType.getLogicalType(); final LogicalType elementType; if (logicalType instanceof ArrayType) { elementType = ((ArrayType) logicalType).getElementType(); } else if (logicalType instanceof MapType) { elementType = ((MapType) logicalType).getKeyType(); } else { return Optional.of(collectionType); } } ``` Hi Ramin I was talking with claude to understand these changes and realised that this `children.get(0)` depends on fact that a map will return its types ordered [key,value], however if this order is changed at a later date functions like MAP_CONTAINS_KEY may work on inputs of type (VARIANT : INT), and choose not to work on inputs like (INT : VARIANT) -- 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]
