gustavodemorais commented on code in PR #28948: URL: https://github.com/apache/flink/pull/28948#discussion_r3853958755
########## flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/strategies/ArrayOfEntriesArgumentTypeStrategyTest.java: ########## @@ -0,0 +1,136 @@ +/* + * 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.api.common.serialization.SerializerConfigImpl; +import org.apache.flink.api.java.typeutils.runtime.kryo.KryoSerializer; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.types.inference.ArgumentTypeStrategy; +import org.apache.flink.table.types.inference.InputTypeStrategiesTestBase; +import org.apache.flink.table.types.inference.InputTypeStrategy; +import org.apache.flink.table.types.utils.DataTypeFactoryMock; + +import org.junit.jupiter.api.Test; + +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; + +/** Tests for {@link ArrayOfEntriesArgumentTypeStrategy}. */ +class ArrayOfEntriesArgumentTypeStrategyTest extends InputTypeStrategiesTestBase { + + private static final InputTypeStrategy MAP_FROM_ENTRIES_INPUT_STRATEGY = + BuiltInFunctionDefinitions.MAP_FROM_ENTRIES + .getTypeInference(new DataTypeFactoryMock()) + .getInputTypeStrategy(); + + @Override + protected Stream<TestSpec> testData() { + return Stream.of( + TestSpec.forStrategy( + "Array of two-field rows is accepted", + MAP_FROM_ENTRIES_INPUT_STRATEGY) + .calledWithArgumentTypes( + DataTypes.ARRAY( + DataTypes.ROW( + DataTypes.FIELD("key", DataTypes.INT()), + DataTypes.FIELD("value", DataTypes.STRING())))) + .expectSignature("f(input ARRAY<ROW<key, value>>)") + .expectArgumentTypes( + DataTypes.ARRAY( + DataTypes.ROW( + DataTypes.FIELD("key", DataTypes.INT()), + DataTypes.FIELD("value", DataTypes.STRING())))), + TestSpec.forStrategy( + "Nested and NOT NULL element types are preserved", + MAP_FROM_ENTRIES_INPUT_STRATEGY) + .calledWithArgumentTypes( + DataTypes.ARRAY( + DataTypes.ROW( + DataTypes.FIELD( + "key", DataTypes.STRING()), + DataTypes.FIELD( + "value", + DataTypes.ARRAY( + DataTypes.INT()))) + .notNull()) + .notNull()) + .expectArgumentTypes( + DataTypes.ARRAY( + DataTypes.ROW( + DataTypes.FIELD( + "key", DataTypes.STRING()), + DataTypes.FIELD( + "value", + DataTypes.ARRAY( + DataTypes.INT()))) + .notNull()) + .notNull()), + TestSpec.forStrategy( + "Non-array argument is rejected", MAP_FROM_ENTRIES_INPUT_STRATEGY) + .calledWithArgumentTypes(DataTypes.STRING()) + .expectErrorMessage("The input argument should be ARRAY<ROW<key, value>>"), Review Comment: This test is currently failing - ran it locally, 1 failure. The message got reworded in the last commit but this assertion still expects the old text. ```suggestion .expectErrorMessage("The 'input' argument must be ARRAY<ROW<key, value>>"), ``` ########## docs/data/sql_functions.yml: ########## @@ -928,6 +928,30 @@ collection: - sql: MAP_FROM_ARRAYS(array_of_keys, array_of_values) table: mapFromArrays(array_of_keys, array_of_values) description: Returns a map created from an arrays of keys and values. Note that the lengths of two arrays should be the same. + - sql: MAP_FROM_ENTRIES(array_of_entries) + table: array.mapFromEntries() + description: | + Returns a map created from the given array of entries. Each entry must be a ROW with exactly + two fields, where the first field becomes the key and the second one the value. + + If there are duplicate keys, the value of the last entry with that key wins; NULL keys are + treated as equal and collapse into a single entry. If the array itself or any of its entries + is null, null is returned. Review Comment: nit: NULL/null casing is mixed in the same paragraph - "NULL keys" a couple lines up, then "is null, null is returned" here. Rest of the file keeps NULL uppercase throughout, let's do the same (same thing in the zh mirror). ########## flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ArrayOfEntriesArgumentTypeStrategy.java: ########## @@ -0,0 +1,92 @@ +/* + * 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.CollectionDataType; +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 java.util.Optional; + +/** + * Strategy for an argument that must be an array of map entries, i.e. an {@code ARRAY} whose + * element is a {@code ROW} with exactly two fields. The first field becomes the map key, the second + * one the map value. + */ +@Internal +public final class ArrayOfEntriesArgumentTypeStrategy implements ArgumentTypeStrategy { + + @Override + public Optional<DataType> inferArgumentType( + CallContext callContext, int argumentPos, boolean throwOnFailure) { + final DataType actualType = callContext.getArgumentDataTypes().get(argumentPos); + if (!actualType.getLogicalType().is(LogicalTypeRoot.ARRAY)) { + return callContext.fail( + throwOnFailure, + "The 'input' argument must be ARRAY<ROW<key, value>>, but actual type was '%s'.", + actualType.getLogicalType().asSummaryString()); + } + + final LogicalType elementType = + ((CollectionDataType) actualType).getElementDataType().getLogicalType(); + if (!elementType.is(LogicalTypeRoot.ROW) + || LogicalTypeChecks.getFieldCount(elementType) != 2) { + return callContext.fail( + throwOnFailure, + "The input argument should be ARRAY<ROW<key, value>>, but the array element " Review Comment: This is actually the root cause of the test failure above - line 50 got polished to quote 'input' and say "must be", but this branch still says "should be" with no quotes. Same method, two different phrasings now. Let's align them. ```suggestion "The 'input' argument must be ARRAY<ROW<key, value>>, but the array element " + "type was '%s'. The element must be a ROW with exactly two fields.", ``` -- 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]
