Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484177231 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,141 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.CollectionDataType; +import org.apache.flink.table.types.DataType; +import org.apache.flink.util.FlinkRuntimeException; + +import javax.annotation.Nullable; + +import java.lang.invoke.MethodHandle; +import java.util.Arrays; +import java.util.Comparator; + +import static org.apache.flink.table.api.Expressions.$; + +/** Implementation of ARRAY_SORT function. */ +@Internal +public class ArraySortFunction extends BuiltInScalarFunction { + +private final ArrayData.ElementGetter elementGetter; +private final SpecializedFunction.ExpressionEvaluator greaterEvaluator; + +private transient MethodHandle greaterHandle; + +public ArraySortFunction(SpecializedFunction.SpecializedContext context) { +super(BuiltInFunctionDefinitions.ARRAY_SORT, context); +final DataType elementDataType = +((CollectionDataType) context.getCallContext().getArgumentDataTypes().get(0)) +.getElementDataType(); +elementGetter = ArrayData.createElementGetter(elementDataType.getLogicalType()); +greaterEvaluator = +context.createEvaluator( +$("element1").isGreater($("element2")), +DataTypes.BOOLEAN(), +DataTypes.FIELD("element1", elementDataType.notNull().toInternal()), +DataTypes.FIELD("element2", elementDataType.notNull().toInternal())); +} + +@Override +public void open(FunctionContext context) throws Exception { +greaterHandle = greaterEvaluator.open(context); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean... ascendingOrder) { +try { +if (array == null || ascendingOrder.length > 0 && ascendingOrder[0] == null) { +return null; +} Review Comment: This is not covered by the output strategy, is it? The output strategy does not depend on the second argument. ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,141 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.Collectio
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484422529 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: Hi, @dawidwys , from the description, if the second argument is null, we will return NULL. `SELECT ARRAY_SLICE(['a', 'b', 'c', 'd', 'e'], null) null ` -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484422529 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: Hi, @dawidwys , from the description, if second argument is null, we will return NUll. `SELECT ARRAY_SLICE(['a', 'b', 'c', 'd', 'e'], null) null ` -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484422529 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: Hi, @dawidwys , from the description, if the second argument is null, we will return NULL. `SELECT ARRAY_SLICE(['a', 'b', 'c', 'd', 'e'], null) null ` https://github.com/apache/flink/pull/22951#issue-1788425522 -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484427940 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementArgumentTypeStrategy.java: ## @@ -0,0 +1,125 @@ +/* + * 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.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; +import org.apache.flink.table.types.logical.LegacyTypeInformationType; +import org.apache.flink.table.types.logical.LogicalType; +import org.apache.flink.table.types.logical.LogicalTypeFamily; +import org.apache.flink.table.types.logical.LogicalTypeRoot; +import org.apache.flink.table.types.logical.StructuredType; + +import java.util.List; +import java.util.Optional; + +/** + * An {@link ArgumentTypeStrategy} that checks if the input argument is an ARRAY type and check + * whether its' elements are comparable. + * + * It requires one argument. + * + * For the rules which types are comparable with which types see {@link + * #areComparable(LogicalType, LogicalType)}. + */ +public final class ArrayComparableElementArgumentTypeStrategy implements ArgumentTypeStrategy { Review Comment: Yes, https://github.com/apache/flink/blob/master/flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementTypeStrategy.java is also write by myself, but that return type is InputTypeStrategy, we need ArgumentTypeStrategy here. Maybe we can extract same code as an utils and reuse the code. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484430188 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,141 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.CollectionDataType; +import org.apache.flink.table.types.DataType; +import org.apache.flink.util.FlinkRuntimeException; + +import javax.annotation.Nullable; + +import java.lang.invoke.MethodHandle; +import java.util.Arrays; +import java.util.Comparator; + +import static org.apache.flink.table.api.Expressions.$; + +/** Implementation of ARRAY_SORT function. */ +@Internal +public class ArraySortFunction extends BuiltInScalarFunction { + +private final ArrayData.ElementGetter elementGetter; +private final SpecializedFunction.ExpressionEvaluator greaterEvaluator; + +private transient MethodHandle greaterHandle; + +public ArraySortFunction(SpecializedFunction.SpecializedContext context) { +super(BuiltInFunctionDefinitions.ARRAY_SORT, context); +final DataType elementDataType = +((CollectionDataType) context.getCallContext().getArgumentDataTypes().get(0)) +.getElementDataType(); +elementGetter = ArrayData.createElementGetter(elementDataType.getLogicalType()); +greaterEvaluator = +context.createEvaluator( +$("element1").isGreater($("element2")), +DataTypes.BOOLEAN(), +DataTypes.FIELD("element1", elementDataType.notNull().toInternal()), +DataTypes.FIELD("element2", elementDataType.notNull().toInternal())); +} + +@Override +public void open(FunctionContext context) throws Exception { +greaterHandle = greaterEvaluator.open(context); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean... ascendingOrder) { +try { +if (array == null || ascendingOrder.length > 0 && ascendingOrder[0] == null) { +return null; +} Review Comment: From the description, it seems that the output strategy depend on the second argument. https://github.com/apache/flink/pull/22951#issue-1788425522 -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484427940 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementArgumentTypeStrategy.java: ## @@ -0,0 +1,125 @@ +/* + * 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.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; +import org.apache.flink.table.types.logical.LegacyTypeInformationType; +import org.apache.flink.table.types.logical.LogicalType; +import org.apache.flink.table.types.logical.LogicalTypeFamily; +import org.apache.flink.table.types.logical.LogicalTypeRoot; +import org.apache.flink.table.types.logical.StructuredType; + +import java.util.List; +import java.util.Optional; + +/** + * An {@link ArgumentTypeStrategy} that checks if the input argument is an ARRAY type and check + * whether its' elements are comparable. + * + * It requires one argument. + * + * For the rules which types are comparable with which types see {@link + * #areComparable(LogicalType, LogicalType)}. + */ +public final class ArrayComparableElementArgumentTypeStrategy implements ArgumentTypeStrategy { Review Comment: Yes, https://github.com/apache/flink/blob/master/flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementTypeStrategy.java is also write by myself, but that return type is InputTypeStrategy. We need ArgumentTypeStrategy here, so I generate a new class which return ArgumentTypeStrategy. Maybe we can extract same code as an utils and reuse the code. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484440158 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( Review Comment: I use sequence here, so I need ArgumentTypeStrategy here. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484427940 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementArgumentTypeStrategy.java: ## @@ -0,0 +1,125 @@ +/* + * 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.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; +import org.apache.flink.table.types.logical.LegacyTypeInformationType; +import org.apache.flink.table.types.logical.LogicalType; +import org.apache.flink.table.types.logical.LogicalTypeFamily; +import org.apache.flink.table.types.logical.LogicalTypeRoot; +import org.apache.flink.table.types.logical.StructuredType; + +import java.util.List; +import java.util.Optional; + +/** + * An {@link ArgumentTypeStrategy} that checks if the input argument is an ARRAY type and check + * whether its' elements are comparable. + * + * It requires one argument. + * + * For the rules which types are comparable with which types see {@link + * #areComparable(LogicalType, LogicalType)}. + */ +public final class ArrayComparableElementArgumentTypeStrategy implements ArgumentTypeStrategy { Review Comment: Yes, https://github.com/apache/flink/blob/master/flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementTypeStrategy.java is also write by myself, but that return type is InputTypeStrategy. We need ArgumentTypeStrategy here, so I generate a new class which return ArgumentTypeStrategy. https://github.com/apache/flink/pull/22951/files#r1484440158 Maybe we can extract same code as an utils and reuse the code. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484467843 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementArgumentTypeStrategy.java: ## @@ -0,0 +1,125 @@ +/* + * 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.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; +import org.apache.flink.table.types.logical.LegacyTypeInformationType; +import org.apache.flink.table.types.logical.LogicalType; +import org.apache.flink.table.types.logical.LogicalTypeFamily; +import org.apache.flink.table.types.logical.LogicalTypeRoot; +import org.apache.flink.table.types.logical.StructuredType; + +import java.util.List; +import java.util.Optional; + +/** + * An {@link ArgumentTypeStrategy} that checks if the input argument is an ARRAY type and check + * whether its' elements are comparable. + * + * It requires one argument. + * + * For the rules which types are comparable with which types see {@link + * #areComparable(LogicalType, LogicalType)}. + */ +public final class ArrayComparableElementArgumentTypeStrategy implements ArgumentTypeStrategy { Review Comment: Would it make sense to convert the old class to `ArgumentTypeStrategy`? Building an `InputTypeStrategy` from `ArgumentTypeStrategy` usually should be simple: e.g. `sequence(comparable())` -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484473796 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,141 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.CollectionDataType; +import org.apache.flink.table.types.DataType; +import org.apache.flink.util.FlinkRuntimeException; + +import javax.annotation.Nullable; + +import java.lang.invoke.MethodHandle; +import java.util.Arrays; +import java.util.Comparator; + +import static org.apache.flink.table.api.Expressions.$; + +/** Implementation of ARRAY_SORT function. */ +@Internal +public class ArraySortFunction extends BuiltInScalarFunction { + +private final ArrayData.ElementGetter elementGetter; +private final SpecializedFunction.ExpressionEvaluator greaterEvaluator; + +private transient MethodHandle greaterHandle; + +public ArraySortFunction(SpecializedFunction.SpecializedContext context) { +super(BuiltInFunctionDefinitions.ARRAY_SORT, context); +final DataType elementDataType = +((CollectionDataType) context.getCallContext().getArgumentDataTypes().get(0)) +.getElementDataType(); +elementGetter = ArrayData.createElementGetter(elementDataType.getLogicalType()); +greaterEvaluator = +context.createEvaluator( +$("element1").isGreater($("element2")), +DataTypes.BOOLEAN(), +DataTypes.FIELD("element1", elementDataType.notNull().toInternal()), +DataTypes.FIELD("element2", elementDataType.notNull().toInternal())); +} + +@Override +public void open(FunctionContext context) throws Exception { +greaterHandle = greaterEvaluator.open(context); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean... ascendingOrder) { +try { +if (array == null || ascendingOrder.length > 0 && ascendingOrder[0] == null) { +return null; +} Review Comment: Where? Still can't see it: > Returns the array in sorted order. Sorts the input array in ascending or descending order according to the natural ordering of the array elements. NULL elements are placed at the beginning of the returned array in ascending order or at the end of the returned array in descending order. If the array itself is null, the function will return null. The optional ascendingOrder argument defaults to true if not specified. It says: output is a sorted array, which might be null if the input array is null. Do I read it wrong? The output type does not depend on the `ascendingOrder` from that description. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484479808 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: Can you check what's the behaviour in other RDBMS? E.g. postgres, oracle, sql server? Personally I find the behaviour a bit weird we accept nullable type for the second argument. ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: Can you check what's the behaviour in other RDBMS? E.g. postgres, oracle, sql server? Personally I find the behaviour a bit weird we accept a nullable type for the second argument. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484478770 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: You should update the output strategy then to take into account the second argument. So far you have only: `nullableIfArgs(argument(0))` -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484523821 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: Ok, I will check other RDNMS. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484524404 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementArgumentTypeStrategy.java: ## @@ -0,0 +1,125 @@ +/* + * 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.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; +import org.apache.flink.table.types.logical.LegacyTypeInformationType; +import org.apache.flink.table.types.logical.LogicalType; +import org.apache.flink.table.types.logical.LogicalTypeFamily; +import org.apache.flink.table.types.logical.LogicalTypeRoot; +import org.apache.flink.table.types.logical.StructuredType; + +import java.util.List; +import java.util.Optional; + +/** + * An {@link ArgumentTypeStrategy} that checks if the input argument is an ARRAY type and check + * whether its' elements are comparable. + * + * It requires one argument. + * + * For the rules which types are comparable with which types see {@link + * #areComparable(LogicalType, LogicalType)}. + */ +public final class ArrayComparableElementArgumentTypeStrategy implements ArgumentTypeStrategy { Review Comment: Ok, I will do that. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484526273 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,141 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.CollectionDataType; +import org.apache.flink.table.types.DataType; +import org.apache.flink.util.FlinkRuntimeException; + +import javax.annotation.Nullable; + +import java.lang.invoke.MethodHandle; +import java.util.Arrays; +import java.util.Comparator; + +import static org.apache.flink.table.api.Expressions.$; + +/** Implementation of ARRAY_SORT function. */ +@Internal +public class ArraySortFunction extends BuiltInScalarFunction { + +private final ArrayData.ElementGetter elementGetter; +private final SpecializedFunction.ExpressionEvaluator greaterEvaluator; + +private transient MethodHandle greaterHandle; + +public ArraySortFunction(SpecializedFunction.SpecializedContext context) { +super(BuiltInFunctionDefinitions.ARRAY_SORT, context); +final DataType elementDataType = +((CollectionDataType) context.getCallContext().getArgumentDataTypes().get(0)) +.getElementDataType(); +elementGetter = ArrayData.createElementGetter(elementDataType.getLogicalType()); +greaterEvaluator = +context.createEvaluator( +$("element1").isGreater($("element2")), +DataTypes.BOOLEAN(), +DataTypes.FIELD("element1", elementDataType.notNull().toInternal()), +DataTypes.FIELD("element2", elementDataType.notNull().toInternal())); +} + +@Override +public void open(FunctionContext context) throws Exception { +greaterHandle = greaterEvaluator.open(context); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean... ascendingOrder) { +try { +if (array == null || ascendingOrder.length > 0 && ascendingOrder[0] == null) { +return null; +} Review Comment: Yes, in the description it said that Returns: if the array is null or ascendingOrder is null return null. if array is empty return empty. And I agree with you, we should not allow second argument NULL, I will update it. ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,141 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484526273 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,141 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.CollectionDataType; +import org.apache.flink.table.types.DataType; +import org.apache.flink.util.FlinkRuntimeException; + +import javax.annotation.Nullable; + +import java.lang.invoke.MethodHandle; +import java.util.Arrays; +import java.util.Comparator; + +import static org.apache.flink.table.api.Expressions.$; + +/** Implementation of ARRAY_SORT function. */ +@Internal +public class ArraySortFunction extends BuiltInScalarFunction { + +private final ArrayData.ElementGetter elementGetter; +private final SpecializedFunction.ExpressionEvaluator greaterEvaluator; + +private transient MethodHandle greaterHandle; + +public ArraySortFunction(SpecializedFunction.SpecializedContext context) { +super(BuiltInFunctionDefinitions.ARRAY_SORT, context); +final DataType elementDataType = +((CollectionDataType) context.getCallContext().getArgumentDataTypes().get(0)) +.getElementDataType(); +elementGetter = ArrayData.createElementGetter(elementDataType.getLogicalType()); +greaterEvaluator = +context.createEvaluator( +$("element1").isGreater($("element2")), +DataTypes.BOOLEAN(), +DataTypes.FIELD("element1", elementDataType.notNull().toInternal()), +DataTypes.FIELD("element2", elementDataType.notNull().toInternal())); +} + +@Override +public void open(FunctionContext context) throws Exception { +greaterHandle = greaterEvaluator.open(context); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean... ascendingOrder) { +try { +if (array == null || ascendingOrder.length > 0 && ascendingOrder[0] == null) { +return null; +} Review Comment: Oh, I see the description in the docs/data/sql_functions.yml is different than https://github.com/apache/flink/pull/22951#issue-1788425522 ``` Returns: if the array is null or ascendingOrder is null return null. if array is empty return empty. ``` And I agree with you, we should not allow second argument NULL, I will update it. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484532093 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,141 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.CollectionDataType; +import org.apache.flink.table.types.DataType; +import org.apache.flink.util.FlinkRuntimeException; + +import javax.annotation.Nullable; + +import java.lang.invoke.MethodHandle; +import java.util.Arrays; +import java.util.Comparator; + +import static org.apache.flink.table.api.Expressions.$; + +/** Implementation of ARRAY_SORT function. */ +@Internal +public class ArraySortFunction extends BuiltInScalarFunction { + +private final ArrayData.ElementGetter elementGetter; +private final SpecializedFunction.ExpressionEvaluator greaterEvaluator; + +private transient MethodHandle greaterHandle; + +public ArraySortFunction(SpecializedFunction.SpecializedContext context) { +super(BuiltInFunctionDefinitions.ARRAY_SORT, context); +final DataType elementDataType = +((CollectionDataType) context.getCallContext().getArgumentDataTypes().get(0)) +.getElementDataType(); +elementGetter = ArrayData.createElementGetter(elementDataType.getLogicalType()); Review Comment: Ok, I will update it. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484532480 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,141 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.CollectionDataType; +import org.apache.flink.table.types.DataType; +import org.apache.flink.util.FlinkRuntimeException; + +import javax.annotation.Nullable; + +import java.lang.invoke.MethodHandle; +import java.util.Arrays; +import java.util.Comparator; + +import static org.apache.flink.table.api.Expressions.$; + +/** Implementation of ARRAY_SORT function. */ +@Internal +public class ArraySortFunction extends BuiltInScalarFunction { + +private final ArrayData.ElementGetter elementGetter; +private final SpecializedFunction.ExpressionEvaluator greaterEvaluator; + +private transient MethodHandle greaterHandle; + +public ArraySortFunction(SpecializedFunction.SpecializedContext context) { +super(BuiltInFunctionDefinitions.ARRAY_SORT, context); +final DataType elementDataType = +((CollectionDataType) context.getCallContext().getArgumentDataTypes().get(0)) +.getElementDataType(); +elementGetter = ArrayData.createElementGetter(elementDataType.getLogicalType()); +greaterEvaluator = +context.createEvaluator( +$("element1").isGreater($("element2")), +DataTypes.BOOLEAN(), +DataTypes.FIELD("element1", elementDataType.notNull().toInternal()), +DataTypes.FIELD("element2", elementDataType.notNull().toInternal())); +} + +@Override +public void open(FunctionContext context) throws Exception { +greaterHandle = greaterEvaluator.open(context); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean... ascendingOrder) { +try { +if (array == null || ascendingOrder.length > 0 && ascendingOrder[0] == null) { +return null; +} +if (array.size() == 0) { +return array; +} +boolean isAscending = ascendingOrder.length > 0 ? ascendingOrder[0] : true; +Object[] elements = new Object[array.size()]; +for (int i = 0; i < array.size(); i++) { +elements[i] = elementGetter.getElementOrNull(array, i); +} +if (isAscending) { +Comparator ascendingComparator = new MyComparator(true); +Arrays.sort(elements, ascendingComparator); +} else { +Comparator ascendingComparator = new MyComparator(false); +Arrays.sort(elements, ascendingComparator); +} Review Comment: Ok, thanks for your suggestion. ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,141 @@ +/* + * 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
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1484533063 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,141 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.CollectionDataType; +import org.apache.flink.table.types.DataType; +import org.apache.flink.util.FlinkRuntimeException; + +import javax.annotation.Nullable; + +import java.lang.invoke.MethodHandle; +import java.util.Arrays; +import java.util.Comparator; + +import static org.apache.flink.table.api.Expressions.$; + +/** Implementation of ARRAY_SORT function. */ +@Internal +public class ArraySortFunction extends BuiltInScalarFunction { + +private final ArrayData.ElementGetter elementGetter; +private final SpecializedFunction.ExpressionEvaluator greaterEvaluator; + +private transient MethodHandle greaterHandle; + +public ArraySortFunction(SpecializedFunction.SpecializedContext context) { +super(BuiltInFunctionDefinitions.ARRAY_SORT, context); +final DataType elementDataType = +((CollectionDataType) context.getCallContext().getArgumentDataTypes().get(0)) +.getElementDataType(); +elementGetter = ArrayData.createElementGetter(elementDataType.getLogicalType()); +greaterEvaluator = +context.createEvaluator( +$("element1").isGreater($("element2")), +DataTypes.BOOLEAN(), +DataTypes.FIELD("element1", elementDataType.notNull().toInternal()), +DataTypes.FIELD("element2", elementDataType.notNull().toInternal())); +} + +@Override +public void open(FunctionContext context) throws Exception { +greaterHandle = greaterEvaluator.open(context); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean... ascendingOrder) { +try { +if (array == null || ascendingOrder.length > 0 && ascendingOrder[0] == null) { +return null; +} +if (array.size() == 0) { +return array; +} +boolean isAscending = ascendingOrder.length > 0 ? ascendingOrder[0] : true; +Object[] elements = new Object[array.size()]; +for (int i = 0; i < array.size(); i++) { +elements[i] = elementGetter.getElementOrNull(array, i); +} +if (isAscending) { +Comparator ascendingComparator = new MyComparator(true); +Arrays.sort(elements, ascendingComparator); +} else { +Comparator ascendingComparator = new MyComparator(false); +Arrays.sort(elements, ascendingComparator); +} +return new GenericArrayData(elements); +} catch (Throwable t) { +throw new FlinkRuntimeException(t); +} +} + +private class MyComparator implements Comparator { +private final boolean isAscending; + +public MyComparator(boolean isAscending) { +this.isAscending = isAscending; +} + +@Override +public int compare(Object o1, Object o2) { +try { +if (isAscending) { +if (o1 == null) { +return -1; +} +if (o2 == null) { +return -1; +} +boolean found = (boolean) greaterHandle.invoke(o1, o2); Review Comment: Ok, I will update it. -- This is an automated message from the Apache Git Service. To respond to
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on PR #22951: URL: https://github.com/apache/flink/pull/22951#issuecomment-1936835154 @flinkbot run azure -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on PR #22951: URL: https://github.com/apache/flink/pull/22951#issuecomment-1936932786 @flinkbot run azure -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on PR #22951: URL: https://github.com/apache/flink/pull/22951#issuecomment-1938091082 @flinkbot run azure -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1485933655 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementArgumentTypeStrategy.java: ## @@ -34,40 +32,33 @@ import org.apache.flink.table.types.logical.StructuredType.StructuredComparison; import org.apache.flink.util.Preconditions; -import java.util.Collections; import java.util.List; import java.util.Optional; /** - * An {@link InputTypeStrategy} that checks if the input argument is an ARRAY type and check whether - * its' elements are comparable. + * An {@link ArgumentTypeStrategy} that checks if the input argument is an ARRAY type and check + * whether its' elements are comparable. * * It requires one argument. * * For the rules which types are comparable with which types see {@link * #areComparable(LogicalType, LogicalType)}. */ @Internal -public final class ArrayComparableElementTypeStrategy implements InputTypeStrategy { +public final class ArrayComparableElementArgumentTypeStrategy implements ArgumentTypeStrategy { + private final StructuredComparison requiredComparison; -private final ConstantArgumentCount argumentCount; -public ArrayComparableElementTypeStrategy(StructuredComparison requiredComparison) { +public ArrayComparableElementArgumentTypeStrategy(StructuredComparison requiredComparison) { Preconditions.checkArgument(requiredComparison != StructuredComparison.NONE); this.requiredComparison = requiredComparison; -this.argumentCount = ConstantArgumentCount.of(1); -} - -@Override -public ArgumentCount getArgumentCount() { -return argumentCount; } @Override -public Optional> inferInputTypes( -CallContext callContext, boolean throwOnFailure) { +public Optional inferArgumentType( +CallContext callContext, int argumentPos, boolean throwOnFailure) { final List argumentDataTypes = callContext.getArgumentDataTypes(); -final DataType argumentType = argumentDataTypes.get(0); +final DataType argumentType = argumentDataTypes.get(argumentPos); if (!argumentType.getLogicalType().is(LogicalTypeRoot.ARRAY)) { return callContext.fail(throwOnFailure, "All arguments requires to be an ARRAY type"); Review Comment: please update the comment It's an argument type strategy now, so it applies to a single argument not `ALL` ## flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTest.java: ## @@ -640,6 +640,28 @@ ANY, explicit(DataTypes.INT()) .expectArgumentTypes( DataTypes.ARRAY(DataTypes.INT().notNull()).notNull(), DataTypes.INT()), + TestSpec.forStrategy(sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.expectSignature("f(>)") + .calledWithArgumentTypes(DataTypes.ARRAY(DataTypes.ROW())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if input argument type is not ARRAY", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes(DataTypes.INT()) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if the number of input arguments are not one", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes( +DataTypes.ARRAY(DataTypes.INT()), +DataTypes.ARRAY(DataTypes.STRING())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), Review Comment: You test the `sequence` strategy here. We have tests for that already. ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +231,22 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486362717 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: If you're looking for ways to verify other databases that's a good start: https://sqlfiddle.com/ -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486372018 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: Ok, thank you, actually, I did this function last summer and follow the jira tickect's https://issues.apache.org/jira/browse/FLINK-26948 description. And I will check other RDNMS today. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486465811 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,124 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.CollectionDataType; +import org.apache.flink.table.types.DataType; +import org.apache.flink.util.FlinkRuntimeException; + +import javax.annotation.Nullable; + +import java.lang.invoke.MethodHandle; +import java.util.Arrays; +import java.util.Comparator; + +import static org.apache.flink.table.api.Expressions.$; + +/** Implementation of ARRAY_SORT function. */ +@Internal +public class ArraySortFunction extends BuiltInScalarFunction { + +private final ArrayData.ElementGetter elementGetter; +private final SpecializedFunction.ExpressionEvaluator greaterEvaluator; + +private transient MethodHandle greaterHandle; + +public ArraySortFunction(SpecializedFunction.SpecializedContext context) { +super(BuiltInFunctionDefinitions.ARRAY_SORT, context); +final DataType elementDataType = +((CollectionDataType) context.getCallContext().getArgumentDataTypes().get(0)) +.getElementDataType() +.toInternal(); +elementGetter = + ArrayData.createElementGetter(elementDataType.toInternal().getLogicalType()); +greaterEvaluator = +context.createEvaluator( +$("element1").isGreater($("element2")), +DataTypes.BOOLEAN(), +DataTypes.FIELD("element1", elementDataType.notNull().toInternal()), +DataTypes.FIELD("element2", elementDataType.notNull().toInternal())); +} + +@Override +public void open(FunctionContext context) throws Exception { +greaterHandle = greaterEvaluator.open(context); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean... ascendingOrder) { +try { +if (array == null) { +return null; +} +if (array.size() == 0) { +return array; +} +boolean isAscending = ascendingOrder.length > 0 ? ascendingOrder[0] : true; +Object[] elements = new Object[array.size()]; +for (int i = 0; i < array.size(); i++) { +elements[i] = elementGetter.getElementOrNull(array, i); +} +Comparator ascendingComparator = new ArraySortComparator(isAscending); +Arrays.sort(elements, ascendingComparator); +return new GenericArrayData(elements); +} catch (Throwable t) { +throw new FlinkRuntimeException(t); +} +} + +private class ArraySortComparator implements Comparator { +private final boolean isAscending; + +public ArraySortComparator(boolean isAscending) { +this.isAscending = isAscending; +} + +@Override +public int compare(Object o1, Object o2) { +Comparator baseComparator = +(a, b) -> { +try { +if (a == null || b == null) { +return 0; +} +boolean isGreater = (boolean) greaterHandle.invoke(a, b); +return isGreater ? 1 : -1; +} catch (Throwable e) { +throw new RuntimeException(e); +} +}; +Comparator comparator = +isAscending +
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486472909 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +231,22 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(ARRAY_COMPARABLE), +sequence( +ARRAY_COMPARABLE, +InputTypeStrategies.explicit( Review Comment: If we not use InputTypeStrategies.explicit here, it will conflict with `import static org.apache.flink.table.types.inference.TypeStrategies.explicit;` ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/SpecificInputTypeStrategies.java: ## @@ -89,6 +90,10 @@ public static InputTypeStrategy windowTimeIndicator() { public static final ArgumentTypeStrategy ARRAY_ELEMENT_ARG = new ArrayElementArgumentTypeStrategy(); +/** Argument type representing the array is comparable. */ +public static final ArgumentTypeStrategy ARRAY_COMPARABLE = Review Comment: ok ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementArgumentTypeStrategy.java: ## @@ -34,40 +32,33 @@ import org.apache.flink.table.types.logical.StructuredType.StructuredComparison; import org.apache.flink.util.Preconditions; -import java.util.Collections; import java.util.List; import java.util.Optional; /** - * An {@link InputTypeStrategy} that checks if the input argument is an ARRAY type and check whether - * its' elements are comparable. + * An {@link ArgumentTypeStrategy} that checks if the input argument is an ARRAY type and check + * whether its' elements are comparable. * * It requires one argument. * * For the rules which types are comparable with which types see {@link * #areComparable(LogicalType, LogicalType)}. */ @Internal -public final class ArrayComparableElementTypeStrategy implements InputTypeStrategy { +public final class ArrayComparableElementArgumentTypeStrategy implements ArgumentTypeStrategy { + private final StructuredComparison requiredComparison; -private final ConstantArgumentCount argumentCount; -public ArrayComparableElementTypeStrategy(StructuredComparison requiredComparison) { +public ArrayComparableElementArgumentTypeStrategy(StructuredComparison requiredComparison) { Preconditions.checkArgument(requiredComparison != StructuredComparison.NONE); this.requiredComparison = requiredComparison; -this.argumentCount = ConstantArgumentCount.of(1); -} - -@Override -public ArgumentCount getArgumentCount() { -return argumentCount; } @Override -public Optional> inferInputTypes( -CallContext callContext, boolean throwOnFailure) { +public Optional inferArgumentType( +CallContext callContext, int argumentPos, boolean throwOnFailure) { final List argumentDataTypes = callContext.getArgumentDataTypes(); -final DataType argumentType = argumentDataTypes.get(0); +final DataType argumentType = argumentDataTypes.get(argumentPos); if (!argumentType.getLogicalType().is(LogicalTypeRoot.ARRAY)) { return callContext.fail(throwOnFailure, "All arguments requires to be an ARRAY type"); Review Comment: ok -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486481830 ## flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTest.java: ## @@ -640,6 +640,28 @@ ANY, explicit(DataTypes.INT()) .expectArgumentTypes( DataTypes.ARRAY(DataTypes.INT().notNull()).notNull(), DataTypes.INT()), + TestSpec.forStrategy(sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.expectSignature("f(>)") + .calledWithArgumentTypes(DataTypes.ARRAY(DataTypes.ROW())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if input argument type is not ARRAY", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes(DataTypes.INT()) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if the number of input arguments are not one", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes( +DataTypes.ARRAY(DataTypes.INT()), +DataTypes.ARRAY(DataTypes.STRING())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), Review Comment: Yes, although I use `sequence`, actually I test `ARRAY_COMPARABLE` strategy here. If I don't use `sequence`, I cannot test ArgumentTypeStrategy here. I saw other contributor using `sequence` testing their `ArgumentTypeStrategy` in this class. The test is similar as flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementTypeStrategyTest.java -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486481830 ## flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTest.java: ## @@ -640,6 +640,28 @@ ANY, explicit(DataTypes.INT()) .expectArgumentTypes( DataTypes.ARRAY(DataTypes.INT().notNull()).notNull(), DataTypes.INT()), + TestSpec.forStrategy(sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.expectSignature("f(>)") + .calledWithArgumentTypes(DataTypes.ARRAY(DataTypes.ROW())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if input argument type is not ARRAY", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes(DataTypes.INT()) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if the number of input arguments are not one", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes( +DataTypes.ARRAY(DataTypes.INT()), +DataTypes.ARRAY(DataTypes.STRING())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), Review Comment: Yes, although I use `sequence`, actually I test `ARRAY_COMPARABLE` strategy here. If I don't use `sequence`, I cannot test ArgumentTypeStrategy here. I saw other contributor using `sequence` testing their `ArgumentTypeStrategy` in this class. The test is same as flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementTypeStrategyTest.java -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486481830 ## flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTest.java: ## @@ -640,6 +640,28 @@ ANY, explicit(DataTypes.INT()) .expectArgumentTypes( DataTypes.ARRAY(DataTypes.INT().notNull()).notNull(), DataTypes.INT()), + TestSpec.forStrategy(sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.expectSignature("f(>)") + .calledWithArgumentTypes(DataTypes.ARRAY(DataTypes.ROW())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if input argument type is not ARRAY", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes(DataTypes.INT()) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if the number of input arguments are not one", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes( +DataTypes.ARRAY(DataTypes.INT()), +DataTypes.ARRAY(DataTypes.STRING())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), Review Comment: Yes, although I use `sequence`, actually I test `ARRAY_COMPARABLE` strategy here. If I don't use `sequence`, I cannot test ArgumentTypeStrategy here. I saw other contributor using `sequence` testing their `ArgumentTypeStrategy` in this class. The test is same as [flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementTypeStrategyTest.java](url) -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486487968 ## flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTest.java: ## @@ -640,6 +640,28 @@ ANY, explicit(DataTypes.INT()) .expectArgumentTypes( DataTypes.ARRAY(DataTypes.INT().notNull()).notNull(), DataTypes.INT()), + TestSpec.forStrategy(sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.expectSignature("f(>)") + .calledWithArgumentTypes(DataTypes.ARRAY(DataTypes.ROW())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if input argument type is not ARRAY", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes(DataTypes.INT()) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if the number of input arguments are not one", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes( +DataTypes.ARRAY(DataTypes.INT()), +DataTypes.ARRAY(DataTypes.STRING())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), Review Comment: Because https://github.com/apache/flink/pull/22951#discussion_r1484467843, I delete old test file and add these test in the `InputTypeStrategiesTest`. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486487968 ## flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTest.java: ## @@ -640,6 +640,28 @@ ANY, explicit(DataTypes.INT()) .expectArgumentTypes( DataTypes.ARRAY(DataTypes.INT().notNull()).notNull(), DataTypes.INT()), + TestSpec.forStrategy(sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.expectSignature("f(>)") + .calledWithArgumentTypes(DataTypes.ARRAY(DataTypes.ROW())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if input argument type is not ARRAY", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes(DataTypes.INT()) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if the number of input arguments are not one", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes( +DataTypes.ARRAY(DataTypes.INT()), +DataTypes.ARRAY(DataTypes.STRING())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), Review Comment: Because https://github.com/apache/flink/pull/22951#discussion_r1484467843, I delete old test file and add these test in the `InputTypeStrategiesTest`. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486481830 ## flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTest.java: ## @@ -640,6 +640,28 @@ ANY, explicit(DataTypes.INT()) .expectArgumentTypes( DataTypes.ARRAY(DataTypes.INT().notNull()).notNull(), DataTypes.INT()), + TestSpec.forStrategy(sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.expectSignature("f(>)") + .calledWithArgumentTypes(DataTypes.ARRAY(DataTypes.ROW())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if input argument type is not ARRAY", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes(DataTypes.INT()) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if the number of input arguments are not one", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes( +DataTypes.ARRAY(DataTypes.INT()), +DataTypes.ARRAY(DataTypes.STRING())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), Review Comment: Yes, although I use `sequence`, actually I test `ARRAY_COMPARABLE` strategy here. If I don't use `sequence`, I cannot test ArgumentTypeStrategy here. I saw other contributor using `sequence` testing their `ArgumentTypeStrategy` in this class. Because https://github.com/apache/flink/pull/22951#discussion_r1484467843, I delete old test file([flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementTypeStrategyTest.java](url)) and add these test in the `InputTypeStrategiesTest`. ## flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTest.java: ## @@ -640,6 +640,28 @@ ANY, explicit(DataTypes.INT()) .expectArgumentTypes( DataTypes.ARRAY(DataTypes.INT().notNull()).notNull(), DataTypes.INT()), + TestSpec.forStrategy(sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.expectSignature("f(>)") + .calledWithArgumentTypes(DataTypes.ARRAY(DataTypes.ROW())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if input argument type is not ARRAY", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes(DataTypes.INT()) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if the number of input arguments are not one", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes( +DataTypes.ARRAY(DataTypes.INT()), +DataTypes.ARRAY(DataTypes.STRING())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), Review Comment: Yes, although I use `sequence`, actually I test `ARRAY_COMPARABLE` strategy here. If I don't use `sequence`, I cannot test ArgumentTypeStrategy here. I saw other contributor using `sequence` testing their `ArgumentTypeStrategy` in this class. Because https://github.com/apache/flink/pull/22951#discussion_r1484467843, I deleted old test file([flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/strategies/ArrayComparableElementTypeStrategyTest.java](url)) and add these test in the `InputTypeStrategiesTest`. -- T
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on PR #22951: URL: https://github.com/apache/flink/pull/22951#issuecomment-1939194309 @flinkbot run azure -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on PR #22951: URL: https://github.com/apache/flink/pull/22951#issuecomment-1939582774 @flinkbot run azure -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on PR #22951: URL: https://github.com/apache/flink/pull/22951#issuecomment-1939820210 @flinkbot run azure -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486986597 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: For postgresql https://www.postgresql.org/docs/8.4/intarray.html it has three ways to sort an array ``` sort(int[], text dir) | int[] | sort array — dir must be asc or desc | sort('{1,2,3}'::int[], 'desc') | {3,2,1} sort(int[]) | int[] | sort in ascending order | sort(array[11,77,44]) | {11,44,77} sort_asc(int[]) | int[] | sort in ascending order | | sort_desc(int[]) | int[] | sort in descending order | ``` -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486986597 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: For postgresql https://www.postgresql.org/docs/8.4/intarray.html it has four ways to sort an array ``` sort(int[], text dir) | int[] | sort array — dir must be asc or desc | sort('{1,2,3}'::int[], 'desc') | {3,2,1} sort(int[]) | int[] | sort in ascending order | sort(array[11,77,44]) | {11,44,77} sort_asc(int[]) | int[] | sort in ascending order | | sort_desc(int[]) | int[] | sort in descending order | ``` -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486986597 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: SQL Server: Uses the ORDER BY clause for sorting based on column names, supporting ascending or descending order. Does not have native array types but allows sorting through table variables or temporary tables. More info. SQLite: Similar to SQL Server, it relies on ORDER BY for sorting without built-in array support. PostgreSQL: Supports arrays and provides functions like array_sort for direct sorting of array elements. For postgresql https://www.postgresql.org/docs/8.4/intarray.html it has four ways to sort an array ``` sort(int[], text dir) | int[] | sort array — dir must be asc or desc | sort('{1,2,3}'::int[], 'desc') | {3,2,1} sort(int[]) | int[] | sort in ascending order | sort(array[11,77,44]) | {11,44,77} sort_asc(int[]) | int[] | sort in ascending order | | sort_desc(int[]) | int[] | sort in descending order | ``` MySQL & MariaDB: Lack built-in array support; sorting is achieved using ORDER BY. For JSON array elements, functions such as JSON_EXTRACT are used. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on PR #22951: URL: https://github.com/apache/flink/pull/22951#issuecomment-1940657344 @flinkbot run azure -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486986597 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: SQL Server: Uses the ORDER BY clause for sorting based on column names, supporting ascending or descending order. Does not have native array types but allows sorting through table variables or temporary tables. SQLite: Similar to SQL Server, it relies on ORDER BY for sorting without built-in array support. PostgreSQL: Supports arrays and provides functions like array_sort for direct sorting of array elements. For postgresql https://www.postgresql.org/docs/8.4/intarray.html it has four ways to sort an array ``` sort(int[], text dir) | int[] | sort array — dir must be asc or desc | sort('{1,2,3}'::int[], 'desc') | {3,2,1} sort(int[]) | int[] | sort in ascending order | sort(array[11,77,44]) | {11,44,77} sort_asc(int[]) | int[] | sort in ascending order | | sort_desc(int[]) | int[] | sort in descending order | ``` MySQL & MariaDB: Lack built-in array support; sorting is achieved using ORDER BY. For JSON array elements, functions such as JSON_EXTRACT are used. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1486986597 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: SQL Server: Uses the ORDER BY clause for sorting based on column names, supporting ascending or descending order. Does not have native array types but allows sorting through table variables or temporary tables. SQLite: Similar to SQL Server, it relies on ORDER BY for sorting without built-in array support. PostgreSQL: Supports arrays and provides functions like array_sort for direct sorting of array elements. For postgresql https://www.postgresql.org/docs/8.4/intarray.html it has four ways to sort an array ``` sort(int[], text dir) | int[] | sort array — dir must be asc or desc | sort('{1,2,3}'::int[], 'desc') | {3,2,1} sort(int[]) | int[] | sort in ascending order | sort(array[11,77,44]) | {11,44,77} sort_asc(int[]) | int[] | sort in ascending order | | sort_desc(int[]) | int[] | sort in descending order | ``` MySQL & MariaDB: Lack built-in array support; sorting is achieved using ORDER BY. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1487819172 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: > Ok, thank you, actually, I did this function last summer and follow the jira tickect's https://issues.apache.org/jira/browse/FLINK-26948 description. I understand that. If at some point we find out we either made mistake or did not put enough effort into something it's better to fix that sooner rather than later when we need to leave with the consequences. I admit I have not thoroughly checked the semantics before which I should've. It's better to do something well rather than fast in my opinion. I see, so traditional RDBMS do not really support that function. It's also worth checking what does: * Snowflake: https://docs.snowflake.com/en/sql-reference/functions/array_sort (`null` on any argument `null`), null handling separately * Spark: https://docs.databricks.com/en/sql/language-manual/functions/sort_array.html: from the docs it does not say what's the behaviour on `null` `ascendingOrder` * Presto: https://prestodb.io/docs/current/functions/array.html: has two separate functions for `ASC/DESC` To me Snowflake's behaviour is the cleanest out there. WDYT? @MartijnVisser -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1487819172 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: > Ok, thank you, actually, I did this function last summer and follow the jira tickect's https://issues.apache.org/jira/browse/FLINK-26948 description. I understand that. If at some point we find out we either made mistake or did not put enough effort into something it's better to fix that sooner rather than later when we need to leave with the consequences. I admit I have not thoroughly checked the semantics before which I should've. It's better to do something well rather than fast in my opinion. I see, so traditional RDBMS do not really support that function. It's also worth checking what does: * Snowflake: https://docs.snowflake.com/en/sql-reference/functions/array_sort (`null` when any argument is `null`), null handling separately * Spark: https://docs.databricks.com/en/sql/language-manual/functions/sort_array.html: from the docs it does not say what's the behaviour on `null` `ascendingOrder`, nulls first on asc, nulls last on desc * Presto: https://prestodb.io/docs/current/functions/array.html: has two separate functions for `ASC/DESC` To me Snowflake's behaviour is the cleanest out there. WDYT? @MartijnVisser -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1487823032 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +231,22 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(ARRAY_COMPARABLE), +sequence( +ARRAY_COMPARABLE, +InputTypeStrategies.explicit( Review Comment: You're right. Sorry, missed that. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1487826357 ## flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTest.java: ## @@ -640,6 +640,28 @@ ANY, explicit(DataTypes.INT()) .expectArgumentTypes( DataTypes.ARRAY(DataTypes.INT().notNull()).notNull(), DataTypes.INT()), + TestSpec.forStrategy(sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.expectSignature("f(>)") + .calledWithArgumentTypes(DataTypes.ARRAY(DataTypes.ROW())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if input argument type is not ARRAY", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes(DataTypes.INT()) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if the number of input arguments are not one", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes( +DataTypes.ARRAY(DataTypes.INT()), +DataTypes.ARRAY(DataTypes.STRING())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), Review Comment: Sorry, I was not clear enough. I meant we don't need to test `sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)` takes a single argument. We don't need to test that, because that's a property of the `sequence`, whatever we use here instead of `SpecificInputTypeStrategies.ARRAY_COMPARABLE` does not really matter. The other two cases make total sense and you do it the right way! 👍 -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1488411926 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: Ok, waiting for @MartijnVisser reply. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1488414914 ## flink-table/flink-table-common/src/test/java/org/apache/flink/table/types/inference/InputTypeStrategiesTest.java: ## @@ -640,6 +640,28 @@ ANY, explicit(DataTypes.INT()) .expectArgumentTypes( DataTypes.ARRAY(DataTypes.INT().notNull()).notNull(), DataTypes.INT()), + TestSpec.forStrategy(sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.expectSignature("f(>)") + .calledWithArgumentTypes(DataTypes.ARRAY(DataTypes.ROW())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if input argument type is not ARRAY", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes(DataTypes.INT()) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), +TestSpec.forStrategy( +"Strategy fails if the number of input arguments are not one", + sequence(SpecificInputTypeStrategies.ARRAY_COMPARABLE)) +.calledWithArgumentTypes( +DataTypes.ARRAY(DataTypes.INT()), +DataTypes.ARRAY(DataTypes.STRING())) +.expectErrorMessage( +"Invalid input arguments. Expected signatures are:\n" ++ "f(>)"), Review Comment: Ok, I will remove this test case. Thank you for your explain. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1487819172 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: > Ok, thank you, actually, I did this function last summer and follow the jira tickect's https://issues.apache.org/jira/browse/FLINK-26948 description. I understand that. If at some point we find out we either made mistake or did not put enough effort into something it's better to fix that sooner rather than later when we need to live with the consequences. I admit I have not thoroughly checked the semantics before which I should've. It's better to do something well rather than fast in my opinion. I see, so traditional RDBMS do not really support that function. It's also worth checking what does: * Snowflake: https://docs.snowflake.com/en/sql-reference/functions/array_sort (`null` when any argument is `null`), null handling separately * Spark: https://docs.databricks.com/en/sql/language-manual/functions/sort_array.html: from the docs it does not say what's the behaviour on `null` `ascendingOrder`, nulls first on asc, nulls last on desc * Presto: https://prestodb.io/docs/current/functions/array.html: has two separate functions for `ASC/DESC` To me Snowflake's behaviour is the cleanest out there. WDYT? @MartijnVisser -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
MartijnVisser commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1489314997 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: This is quite a rabbit hole 😆 1. Databricks has two functions: * sort_array https://docs.databricks.com/en/sql/language-manual/functions/sort_array.html - Sorts ascending, puts NULL at beginning. * array_sort https://docs.databricks.com/en/sql/language-manual/functions/array_sort.html - Sorts ascending, puts NULL at the end. 2. Spark has https://spark.apache.org/docs/latest/api/sql/index.html#sort_array which is indeed not different from the Databricks one. Sorts ascending, puts NULL at beginning 3. Snowflake indeed has https://docs.snowflake.com/en/sql-reference/functions/array_sort - Sorts ascending, puts NULL at the end. 4. Trino, that has `array_sort(x)` - Sorts ascending, puts NULL at the end. 5. StarRocks also has this function https://docs.starrocks.io/docs/2.2/sql-reference/sql-functions/array-functions/array_sort/ - Sorts ascending, puts NULL at beginning 6. Doris has it https://doris.apache.org/docs/1.2/sql-manual/sql-functions/array-functions/array_sort/ - Sorts ascending, puts NULL at beginning 7. Hive uses NULLS FIRST as the default behavior in ascending ordering mode, per https://issues.apache.org/jira/browse/HIVE-12994 - Sorts ascending, puts NULL at beginning. I do think that the Snowflake has the cleanest function signature: there's just one function, and it has two optional arguments to give all the flexibility. I think the default sorting mode should be ascending, and then it boils down to decide on the default location on where to put NULL. Out of the 7 implementations, we have 5 who put NULL at the beginning, and 2 who put them at the end. Naturally, I think we should put NULL at the beginning but I'm +0 on the default location of NULL. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1489331336 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: @MartijnVisser Actually Snowflake does: > nulls_first Default: FALSE if the ARRAY is sorted in ascending order; TRUE if the ARRAY is sorted in descending order. I'd suggest we: 1. adapt Snowflake's signature where we get `ARRAY, BOOLEAN, BOOLEAN`, output is `ARRAY` which is `NULLABLE` if any of the input is nullable 2. We use defaults: ascending and `nulls_first` for ASC, `nulls_last` for DESC 3. If any of the input is null the output is also null How does this sound? -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
MartijnVisser commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1489343962 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: Sounds good to me, thanks! -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
MartijnVisser commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1489314997 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: This is quite a rabbit hole 😆 1. Databricks has two functions: * sort_array https://docs.databricks.com/en/sql/language-manual/functions/sort_array.html - Sorts ascending, puts NULL at beginning. * array_sort https://docs.databricks.com/en/sql/language-manual/functions/array_sort.html - Sorts ascending, puts NULL at the end. 2. Spark has https://spark.apache.org/docs/latest/api/sql/index.html#sort_array which is indeed not different from the Databricks one. Sorts ascending, puts NULL at beginning 3. Snowflake indeed has https://docs.snowflake.com/en/sql-reference/functions/array_sort - Sorts ascending, puts NULL at ~~the end~~ beginning. 4. Trino, that has `array_sort(x)` - Sorts ascending, puts NULL at the end. 5. StarRocks also has this function https://docs.starrocks.io/docs/2.2/sql-reference/sql-functions/array-functions/array_sort/ - Sorts ascending, puts NULL at beginning 6. Doris has it https://doris.apache.org/docs/1.2/sql-manual/sql-functions/array-functions/array_sort/ - Sorts ascending, puts NULL at beginning 7. Hive uses NULLS FIRST as the default behavior in ascending ordering mode, per https://issues.apache.org/jira/browse/HIVE-12994 - Sorts ascending, puts NULL at beginning. I do think that the Snowflake has the cleanest function signature: there's just one function, and it has two optional arguments to give all the flexibility. I think the default sorting mode should be ascending, and then it boils down to decide on the default location on where to put NULL. Out of the 7 implementations, we have 6 who put NULL at the beginning, and only Databricks puts them at the end. Naturally, I think we should put NULL at the beginning but I'm +0 on the default location of NULL. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1489798277 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: The updated function signature is now ARRAY_SORT([, [, ]]), where both sort_ascending and nulls_first parameters are optional, allowing for more flexibility in array sorting behavior. @MartijnVisser @dawidwys -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1489798277 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: The updated function signature is now ARRAY_SORT(, [, [, ]]), where both sort_ascending and nulls_first parameters are optional, allowing for more flexibility in array sorting behavior. @MartijnVisser @dawidwys -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1489798277 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: The updated function signature is now `ARRAY_SORT(,[, [, ]])`, where both sort_ascending and nulls_first parameters are optional, allowing for more flexibility in array sorting behavior. @MartijnVisser @dawidwys -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1489798277 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: The updated function signature is now `ARRAY_SORT(,[, [, ]])`, where both sort_ascending and nulls_first parameters are optional, allowing for more flexibility in array sorting behavior. The description like this `The function sorts an array with ascending_order and null_first as optional inputs. By default, with only the input array, both are true, resulting in ascending order sorting with NULLs at the beginning. Specifying ascending_order without null_first results in sorting where true for ascending_order places NULLs first, and false places NULLs last. With all three inputs, null_first true places NULLs at the start, and false at the end, regardless of ascending_order's value. If any input is null, the function returns null.` @MartijnVisser @dawidwys -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1489798277 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: The updated function signature is now `ARRAY_SORT(,[, [, ]])`, where both sort_ascending and nulls_first parameters are optional, allowing for more flexibility in array sorting behavior. The description like this `Returns the array in sorted order. When only the input array is provided, ascending_order and null_first default to true, sorting the array in ascending order with NULLs at the start. True for ascending_order results in ascending sort with NULLs first, while false leads to descending sort with NULLs last. Regardless of ascending_order, null_first true places NULLs at the beginning, and false at the end. If any input is null, the function returns null.` @MartijnVisser @dawidwys -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1489798277 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: The updated function signature is now `ARRAY_SORT(,[, [, ]])`, where both sort_ascending and nulls_first parameters are optional, allowing for more flexibility in array sorting behavior. The description like this `Returns the array in sorted order. When only the input array is provided, ascending_order and null_first default to true, sorting the array in ascending order with NULLs at the start. Setting ascending_order to true sorts array in ascending order, placing NULLs first. Setting it to false sorts array in descending order, with NULLs last. null_first true places NULLs at the beginning, and false at the end, regardless of sort order. If any input is null, the function returns null.` @MartijnVisser @dawidwys -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1489798277 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: The updated function signature is now `ARRAY_SORT(,[, [, ]])`, where both sort_ascending and nulls_first parameters are optional, allowing for more flexibility in array sorting behavior. The description like this `The function sorts an array, defaulting to ascending order with NULLs at the start when only the array is input. Specifying ascending_order as true orders the array in ascending with NULLs first, and setting it to false orders it in descending with NULLs last. Independently, null_first as true moves NULLs to the beginning, and as false to the end, irrespective of the sorting order. The function returns null if any input is null.` @MartijnVisser @dawidwys -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1490931709 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,127 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.CollectionDataType; +import org.apache.flink.table.types.DataType; +import org.apache.flink.util.FlinkRuntimeException; + +import javax.annotation.Nullable; + +import java.lang.invoke.MethodHandle; +import java.util.Arrays; +import java.util.Comparator; + +import static org.apache.flink.table.api.Expressions.$; + +/** Implementation of ARRAY_SORT function. */ +@Internal +public class ArraySortFunction extends BuiltInScalarFunction { + +private final ArrayData.ElementGetter elementGetter; +private final SpecializedFunction.ExpressionEvaluator greaterEvaluator; + +private transient MethodHandle greaterHandle; + +public ArraySortFunction(SpecializedFunction.SpecializedContext context) { +super(BuiltInFunctionDefinitions.ARRAY_SORT, context); +final DataType elementDataType = +((CollectionDataType) context.getCallContext().getArgumentDataTypes().get(0)) +.getElementDataType() +.toInternal(); +elementGetter = + ArrayData.createElementGetter(elementDataType.toInternal().getLogicalType()); +greaterEvaluator = +context.createEvaluator( +$("element1").isGreater($("element2")), +DataTypes.BOOLEAN(), +DataTypes.FIELD("element1", elementDataType.notNull().toInternal()), +DataTypes.FIELD("element2", elementDataType.notNull().toInternal())); +} + +@Override +public void open(FunctionContext context) throws Exception { +greaterHandle = greaterEvaluator.open(context); +} + +public @Nullable ArrayData eval(ArrayData array) { +return eval(array, true, true); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean ascendingOrder) { +return eval(array, ascendingOrder, ascendingOrder); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean ascendingOrder, Boolean nullFirst) { +try { +if (array == null || ascendingOrder == null || nullFirst == null) { +return null; +} +if (array.size() == 0) { +return array; +} +Object[] elements = new Object[array.size()]; +for (int i = 0; i < array.size(); i++) { +elements[i] = elementGetter.getElementOrNull(array, i); +} +Comparator ascendingComparator = new ArraySortComparator(ascendingOrder); +Arrays.sort( +elements, +nullFirst +? Comparator.nullsFirst(ascendingComparator) +: Comparator.nullsLast(ascendingComparator)); +return new GenericArrayData(elements); +} catch (Throwable t) { +throw new FlinkRuntimeException(t); +} +} + +private class ArraySortComparator implements Comparator { +private final boolean isAscending; + +public ArraySortComparator(boolean isAscending) { +this.isAscending = isAscending; +} + +@Override +public int compare(Object o1, Object o2) { +if (o1 == null || o2 == null) { +return 0; +} Review Comment: Is this necessary? This should be handled by the `nullsFirst/nullsLast`, no? ## flink-table/flink-
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1490935727 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +232,21 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(new ArrayComparableElementArgumentTypeStrategy()), +sequence( +new ArrayComparableElementArgumentTypeStrategy(), +logical(LogicalTypeRoot.BOOLEAN Review Comment: Great description! 👍 -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1491222383 ## flink-table/flink-table-common/src/main/java/org/apache/flink/table/functions/BuiltInFunctionDefinitions.java: ## @@ -231,6 +231,28 @@ ANY, and(logical(LogicalTypeRoot.BOOLEAN), LITERAL) "org.apache.flink.table.runtime.functions.scalar.ArrayContainsFunction") .build(); +public static final BuiltInFunctionDefinition ARRAY_SORT = +BuiltInFunctionDefinition.newBuilder() +.name("ARRAY_SORT") +.kind(SCALAR) +.inputTypeStrategy( +or( +sequence(ARRAY_FULLY_COMPARABLE), +sequence( +ARRAY_FULLY_COMPARABLE, +InputTypeStrategies.explicit( + DataTypes.BOOLEAN().nullable())), +sequence( +ARRAY_FULLY_COMPARABLE, +InputTypeStrategies.explicit( + DataTypes.BOOLEAN().nullable()), +InputTypeStrategies.explicit( + DataTypes.BOOLEAN().nullable() Review Comment: ok, I will update. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1491240847 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,127 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.CollectionDataType; +import org.apache.flink.table.types.DataType; +import org.apache.flink.util.FlinkRuntimeException; + +import javax.annotation.Nullable; + +import java.lang.invoke.MethodHandle; +import java.util.Arrays; +import java.util.Comparator; + +import static org.apache.flink.table.api.Expressions.$; + +/** Implementation of ARRAY_SORT function. */ +@Internal +public class ArraySortFunction extends BuiltInScalarFunction { + +private final ArrayData.ElementGetter elementGetter; +private final SpecializedFunction.ExpressionEvaluator greaterEvaluator; + +private transient MethodHandle greaterHandle; + +public ArraySortFunction(SpecializedFunction.SpecializedContext context) { +super(BuiltInFunctionDefinitions.ARRAY_SORT, context); +final DataType elementDataType = +((CollectionDataType) context.getCallContext().getArgumentDataTypes().get(0)) +.getElementDataType() +.toInternal(); +elementGetter = + ArrayData.createElementGetter(elementDataType.toInternal().getLogicalType()); +greaterEvaluator = +context.createEvaluator( +$("element1").isGreater($("element2")), +DataTypes.BOOLEAN(), +DataTypes.FIELD("element1", elementDataType.notNull().toInternal()), +DataTypes.FIELD("element2", elementDataType.notNull().toInternal())); +} + +@Override +public void open(FunctionContext context) throws Exception { +greaterHandle = greaterEvaluator.open(context); +} + +public @Nullable ArrayData eval(ArrayData array) { +return eval(array, true, true); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean ascendingOrder) { +return eval(array, ascendingOrder, ascendingOrder); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean ascendingOrder, Boolean nullFirst) { +try { +if (array == null || ascendingOrder == null || nullFirst == null) { +return null; +} +if (array.size() == 0) { +return array; +} +Object[] elements = new Object[array.size()]; +for (int i = 0; i < array.size(); i++) { +elements[i] = elementGetter.getElementOrNull(array, i); +} +Comparator ascendingComparator = new ArraySortComparator(ascendingOrder); +Arrays.sort( +elements, +nullFirst +? Comparator.nullsFirst(ascendingComparator) +: Comparator.nullsLast(ascendingComparator)); +return new GenericArrayData(elements); +} catch (Throwable t) { +throw new FlinkRuntimeException(t); +} +} + +private class ArraySortComparator implements Comparator { +private final boolean isAscending; + +public ArraySortComparator(boolean isAscending) { +this.isAscending = isAscending; +} + +@Override +public int compare(Object o1, Object o2) { +if (o1 == null || o2 == null) { +return 0; +} Review Comment: I check it, it not necessary, the IDE suggest `Comparator does not return 0 for equal elements` so I add
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
hanyuzheng7 commented on code in PR #22951: URL: https://github.com/apache/flink/pull/22951#discussion_r1491240847 ## flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArraySortFunction.java: ## @@ -0,0 +1,127 @@ +/* + * 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.runtime.functions.scalar; + +import org.apache.flink.annotation.Internal; +import org.apache.flink.table.api.DataTypes; +import org.apache.flink.table.data.ArrayData; +import org.apache.flink.table.data.GenericArrayData; +import org.apache.flink.table.functions.BuiltInFunctionDefinitions; +import org.apache.flink.table.functions.FunctionContext; +import org.apache.flink.table.functions.SpecializedFunction; +import org.apache.flink.table.types.CollectionDataType; +import org.apache.flink.table.types.DataType; +import org.apache.flink.util.FlinkRuntimeException; + +import javax.annotation.Nullable; + +import java.lang.invoke.MethodHandle; +import java.util.Arrays; +import java.util.Comparator; + +import static org.apache.flink.table.api.Expressions.$; + +/** Implementation of ARRAY_SORT function. */ +@Internal +public class ArraySortFunction extends BuiltInScalarFunction { + +private final ArrayData.ElementGetter elementGetter; +private final SpecializedFunction.ExpressionEvaluator greaterEvaluator; + +private transient MethodHandle greaterHandle; + +public ArraySortFunction(SpecializedFunction.SpecializedContext context) { +super(BuiltInFunctionDefinitions.ARRAY_SORT, context); +final DataType elementDataType = +((CollectionDataType) context.getCallContext().getArgumentDataTypes().get(0)) +.getElementDataType() +.toInternal(); +elementGetter = + ArrayData.createElementGetter(elementDataType.toInternal().getLogicalType()); +greaterEvaluator = +context.createEvaluator( +$("element1").isGreater($("element2")), +DataTypes.BOOLEAN(), +DataTypes.FIELD("element1", elementDataType.notNull().toInternal()), +DataTypes.FIELD("element2", elementDataType.notNull().toInternal())); +} + +@Override +public void open(FunctionContext context) throws Exception { +greaterHandle = greaterEvaluator.open(context); +} + +public @Nullable ArrayData eval(ArrayData array) { +return eval(array, true, true); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean ascendingOrder) { +return eval(array, ascendingOrder, ascendingOrder); +} + +public @Nullable ArrayData eval(ArrayData array, Boolean ascendingOrder, Boolean nullFirst) { +try { +if (array == null || ascendingOrder == null || nullFirst == null) { +return null; +} +if (array.size() == 0) { +return array; +} +Object[] elements = new Object[array.size()]; +for (int i = 0; i < array.size(); i++) { +elements[i] = elementGetter.getElementOrNull(array, i); +} +Comparator ascendingComparator = new ArraySortComparator(ascendingOrder); +Arrays.sort( +elements, +nullFirst +? Comparator.nullsFirst(ascendingComparator) +: Comparator.nullsLast(ascendingComparator)); +return new GenericArrayData(elements); +} catch (Throwable t) { +throw new FlinkRuntimeException(t); +} +} + +private class ArraySortComparator implements Comparator { +private final boolean isAscending; + +public ArraySortComparator(boolean isAscending) { +this.isAscending = isAscending; +} + +@Override +public int compare(Object o1, Object o2) { +if (o1 == null || o2 == null) { +return 0; +} Review Comment: I check it. It is not necessary. The IDE suggests `Comparator does not return 0 for equal elements` so I
Re: [PR] [FLINK-26948][table] Add-ARRAY_SORT-function. [flink]
dawidwys merged PR #22951: URL: https://github.com/apache/flink/pull/22951 -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org