This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new ee9d26bcb5d [fix](function) Require the array_sort comparator to
return -1, 0 or 1 (#68612)
ee9d26bcb5d is described below
commit ee9d26bcb5d7baf45a2b1ae8d87684eb87c78592
Author: Chenyang Sun <[email protected]>
AuthorDate: Thu Oct 15 16:53:49 2026 +0800
[fix](function) Require the array_sort comparator to return -1, 0 or 1
(#68612)
```sql
SELECT array_sort((x, y) -> x < y, [3, 2, 1]);
-- before: [FATAL_ERROR]assert cast err ... Bad cast from
type:doris::ColumnNullable* to doris::ColumnVector<(doris::PrimitiveType)3>
const* (a debug/ASAN BE aborts)
-- after: When using lambda as the parameter of array_sort, the lambda
must return -1, 0 or 1 for less than, equal to or greater than, but it returns
BOOLEAN
```
---------
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
.../exprs/lambda_function/varray_sort_function.cpp | 63 +++++++++--
.../lambda_function/array_map_function_test.cpp | 119 +++++++++++++++++++++
.../expressions/functions/scalar/ArraySort.java | 6 ++
.../nereids/rules/analysis/BindFunctionTest.java | 13 +++
.../test_array_sort_comparator_type.out | 10 ++
.../test_array_sort_comparator_type.groovy | 86 +++++++++++++++
6 files changed, 291 insertions(+), 6 deletions(-)
diff --git a/be/src/exprs/lambda_function/varray_sort_function.cpp
b/be/src/exprs/lambda_function/varray_sort_function.cpp
index 978d037cbcf..14f7e4981dd 100644
--- a/be/src/exprs/lambda_function/varray_sort_function.cpp
+++ b/be/src/exprs/lambda_function/varray_sort_function.cpp
@@ -34,6 +34,9 @@
#include "core/column/column_vector.h"
#include "core/custom_allocator.h"
#include "core/data_type/data_type.h"
+#include "core/data_type/data_type_nullable.h"
+#include "core/data_type/primitive_type.h"
+#include "core/value/large_int_value.h"
#include "exec/common/util.hpp"
#include "exprs/lambda_function/lambda_execution_context.h"
#include "exprs/lambda_function/lambda_function.h"
@@ -95,6 +98,16 @@ public:
DCHECK_EQ(children.size(), 2);
+ // The comparator must return -1, 0 or 1. FE rejects a non-integer
comparator, but an older
+ // FE does not check it.
+ const auto& comparator_type = children[0]->get_child(0)->data_type();
+ const auto comparator_result_type =
remove_nullable(comparator_type)->get_primitive_type();
+ if (!is_int(comparator_result_type)) {
+ return Status::InvalidArgument(
+ "array_sort comparator must return -1, 0 or 1, but it
returns {}",
+ comparator_type->get_name());
+ }
+
// 1. get data, we need to obtain this actual data and type.
ColumnPtr column_ptr;
RETURN_IF_ERROR(
@@ -145,11 +158,11 @@ public:
/**
* suppose the data_type is nullable(int). The first two rows are the
parameter columns, and the
- * last row is the result column(type: tinyint). every column's size
is 1. the lambda_block is
+ * last row is the result column(an integer type). every column's
size is 1. the lambda_block is
* row data nullmap type
* 0 10 0 nullable(int)
* 1 20 1 nullable(int)
- * 2 1/-1/0 ... tinyint
+ * 2 1/-1/0 ... integer
* The size of a column is always 1; we only need to use it to store
the specific values in the array for comparison.
*/
Block lambda_block;
@@ -222,12 +235,11 @@ public:
ColumnPtr raw_res_col =
lambda_block.get_by_position(lambda_res_id).column;
ColumnPtr full_res_col =
raw_res_col->convert_to_full_column_if_const();
- // only -1, 0, 1
- long cmp =
- assert_cast<const
ColumnInt8*>(full_res_col.get())->get_data()[0];
+ bool is_less =
_is_negative_comparator_result(*full_res_col,
+
comparator_result_type);
lambda_block.erase_tail(lambda_result_base);
- return cmp < 0;
+ return is_less;
};
// The comparator is user SQL and may violate strict weak
ordering, or
@@ -268,6 +280,45 @@ public:
}
private:
+ // Returns whether the comparator result in the first row is -1. Any value
other than -1, 0 or 1
+ // fails the query.
+ static bool _is_negative_comparator_result(const IColumn& column,
PrimitiveType type) {
+ const IColumn* values = &column;
+ if (const auto* nullable =
check_and_get_column<ColumnNullable>(&column)) {
+ if (nullable->is_null_at(0)) {
+ throw Exception(Status::InvalidArgument(
+ "array_sort comparator returns NULL, but it must
return -1, 0 or 1"));
+ }
+ values = &nullable->get_nested_column();
+ }
+ Int128 result = 0;
+ switch (type) {
+ case TYPE_TINYINT:
+ result = assert_cast<const ColumnInt8&>(*values).get_element(0);
+ break;
+ case TYPE_SMALLINT:
+ result = assert_cast<const ColumnInt16&>(*values).get_element(0);
+ break;
+ case TYPE_INT:
+ result = assert_cast<const ColumnInt32&>(*values).get_element(0);
+ break;
+ case TYPE_BIGINT:
+ result = assert_cast<const ColumnInt64&>(*values).get_element(0);
+ break;
+ case TYPE_LARGEINT:
+ result = assert_cast<const ColumnInt128&>(*values).get_element(0);
+ break;
+ default:
+ DORIS_CHECK(false) << "array_sort comparator must return -1, 0 or
1";
+ }
+ if (result != -1 && result != 0 && result != 1) {
+ throw Exception(Status::InvalidArgument(
+ "array_sort comparator returns {}, but it must return -1,
0 or 1",
+ LargeIntValue::to_string(result)));
+ }
+ return result < 0;
+ }
+
Status _set_comparator_argument_gap(const VExprSPtr& expr,
const std::vector<std::string>*
argument_names) const {
if (expr->is_column_ref()) {
diff --git a/be/test/exprs/lambda_function/array_map_function_test.cpp
b/be/test/exprs/lambda_function/array_map_function_test.cpp
index 2e815617707..137994e06bf 100644
--- a/be/test/exprs/lambda_function/array_map_function_test.cpp
+++ b/be/test/exprs/lambda_function/array_map_function_test.cpp
@@ -25,6 +25,7 @@
#include <vector>
#include "common/config.h"
+#include "common/exception.h"
#include "core/assert_cast.h"
#include "core/block/block.h"
#include "core/column/column_array.h"
@@ -1810,6 +1811,124 @@ TEST(ArrayMapFunctionTest,
NestedArraySortComparatorCapturingOuterArgumentReturn
EXPECT_NE(status.to_string().find("captured column ref 'i'"),
std::string::npos);
}
+// Builds array_sort((x, y) -> body, arr).
+static VExprSPtr make_array_sort_call(const DataTypePtr& comparator_type,
const VExprSPtr& body,
+ ColumnPtr array_column) {
+ auto int_type = std::make_shared<DataTypeInt32>();
+ auto array_int_type = std::make_shared<DataTypeArray>(int_type);
+ auto root = VLambdaFunctionCallExpr::create_shared(
+ make_lambda_call_node(array_int_type, 2, "array_sort"));
+ auto comparator =
+
VLambdaFunctionExpr::create_shared(make_lambda_expr_node(comparator_type, {"x",
"y"}));
+ comparator->add_child(body);
+ root->add_child(comparator);
+ root->add_child(
+ std::make_shared<MockColumnExpr>(std::move(array_column),
array_int_type, "arr"));
+ return root;
+}
+
+TEST(ArrayMapFunctionTest, ArraySortComparatorReturningIntSorts) {
+ auto int_type = std::make_shared<DataTypeInt32>();
+
+ // This mirrors array_sort((x, y) -> x - y, [1, 0]). The comparator
returns -1 or 1 in an INT
+ // column, so the INT result must be read instead of a TINYINT one.
+ auto subtract = std::make_shared<MockSubtractExpr>(int_type);
+ subtract->add_child(VColumnRef::create_shared(make_column_ref_node(0, "x",
int_type)));
+ subtract->add_child(VColumnRef::create_shared(make_column_ref_node(1, "y",
int_type)));
+ auto root = make_array_sort_call(int_type, subtract,
make_int_array_column({{1, 0}}));
+
+ VExprContext context(root);
+ open_expr(root, &context);
+
+ Block block;
+ block.insert({make_int_column({0}), int_type, "ordinary_input"});
+
+ ColumnPtr result;
+ auto status = root->execute_column(&context, &block, nullptr,
block.rows(), result);
+ ASSERT_TRUE(status.ok()) << status.to_string();
+
+ const auto& values = get_int_array_values(result);
+ ASSERT_EQ(values.size(), 2);
+ EXPECT_EQ(values.get_element(0), 0);
+ EXPECT_EQ(values.get_element(1), 1);
+}
+
+TEST(ArrayMapFunctionTest, ArraySortComparatorReturningOutOfRangeReturnsError)
{
+ auto int_type = std::make_shared<DataTypeInt32>();
+
+ // The comparator always returns 2, like array_sort((x, y) -> 2, arr).
Only -1, 0 and 1 are
+ // allowed.
+ auto body = std::make_shared<MockColumnExpr>(ColumnInt32::create(1, 2),
int_type, "two");
+ auto root = make_array_sort_call(int_type, body,
make_int_array_column({{2, 1}}));
+
+ VExprContext context(root);
+ open_expr(root, &context);
+
+ Block block;
+ block.insert({make_int_column({0}), int_type, "ordinary_input"});
+
+ ColumnPtr result;
+ try {
+ auto status = root->execute_column(&context, &block, nullptr,
block.rows(), result);
+ FAIL() << "array_sort should fail, but it returns " <<
status.to_string();
+ } catch (const Exception& e) {
+ EXPECT_NE(std::string(e.what()).find(
+ "array_sort comparator returns 2, but it must return
-1, 0 or 1"),
+ std::string::npos)
+ << e.what();
+ }
+}
+
+TEST(ArrayMapFunctionTest, ArraySortComparatorReturningNullReturnsError) {
+ auto int_type = std::make_shared<DataTypeInt32>();
+ auto nullable_int_type = make_nullable(int_type);
+
+ // The comparator always returns NULL, like array_sort((x, y) -> x - y,
[2, NULL]).
+ auto null_result = ColumnNullable::create(ColumnInt32::create(1, 0),
ColumnUInt8::create(1, 1));
+ auto body = std::make_shared<MockColumnExpr>(std::move(null_result),
nullable_int_type,
+ "null_result");
+ auto root = make_array_sort_call(nullable_int_type, body,
make_int_array_column({{2, 1}}));
+
+ VExprContext context(root);
+ open_expr(root, &context);
+
+ Block block;
+ block.insert({make_int_column({0}), int_type, "ordinary_input"});
+
+ ColumnPtr result;
+ try {
+ auto status = root->execute_column(&context, &block, nullptr,
block.rows(), result);
+ FAIL() << "array_sort should fail, but it returns " <<
status.to_string();
+ } catch (const Exception& e) {
+ EXPECT_NE(std::string(e.what()).find("array_sort comparator returns
NULL"),
+ std::string::npos)
+ << e.what();
+ }
+}
+
+TEST(ArrayMapFunctionTest, ArraySortComparatorReturningBooleanReturnsError) {
+ auto int_type = std::make_shared<DataTypeInt32>();
+ auto bool_type = std::make_shared<DataTypeUInt8>();
+
+ // An older FE may send array_sort((x, y) -> x < y, arr), whose comparator
returns BOOLEAN.
+ auto body =
+ std::make_shared<MockColumnExpr>(ColumnUInt8::create(1, 1),
bool_type, "bool_result");
+ auto root = make_array_sort_call(bool_type, body,
make_int_array_column({{2, 1}}));
+
+ VExprContext context(root);
+ open_expr(root, &context);
+
+ Block block;
+ block.insert({make_int_column({0}), int_type, "ordinary_input"});
+
+ ColumnPtr result;
+ auto status = root->execute_column(&context, &block, nullptr,
block.rows(), result);
+ EXPECT_FALSE(status.ok());
+ EXPECT_NE(status.to_string().find("array_sort comparator must return -1, 0
or 1"),
+ std::string::npos)
+ << status.to_string();
+}
+
TEST(ArrayMapFunctionTest,
NestedArraySortComparatorNestedLambdaCapturingComparatorArgumentReturnsError) {
auto int_type = std::make_shared<DataTypeInt32>();
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArraySort.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArraySort.java
index 0af1201c745..2bfcc6ea6a9 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArraySort.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArraySort.java
@@ -79,6 +79,12 @@ public class ArraySort extends ScalarFunction
if
(!ArrayFunctionUtils.isSupportedByArraySortLambdaFunction(sourceType.getItemType()))
{
throw new AnalysisException("array_sort does not support
types: " + sourceType.toSql());
}
+ // The comparator returns -1, 0 or 1 for less than, equal to or
greater than.
+ if (!lambda.getRetType().isIntegralType()) {
+ throw new AnalysisException("When using lambda as the
parameter of array_sort, the lambda must"
+ + " return -1, 0 or 1 for less than, equal to or
greater than, but it returns "
+ + lambda.getRetType().toSql());
+ }
}
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/BindFunctionTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/BindFunctionTest.java
index 3697af015dc..2761ebe6e26 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/BindFunctionTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/BindFunctionTest.java
@@ -139,6 +139,19 @@ public class BindFunctionTest extends TestWithFeService
implements MemoPatternMa
exception.getMessage());
}
+ @Test
+ void testArraySortComparatorMustReturnInteger() {
+ // The comparator returns -1, 0 or 1 for less than, equal to or
greater than.
+ String sql = "SELECT array_sort((x, y) -> x < y, arr2) FROM t_arr";
+ AnalysisException exception =
Assertions.assertThrows(AnalysisException.class,
+ () -> PlanChecker.from(connectContext).analyze(sql));
+ Assertions.assertTrue(exception.getMessage().contains("the lambda must
return -1, 0 or 1 for less than,"
+ + " equal to or greater than, but it returns BOOLEAN"),
exception.getMessage());
+
+ // Any integer type passes analysis. BE checks that the value is -1, 0
or 1.
+ PlanChecker.from(connectContext).analyze("SELECT array_sort((x, y) ->
x - y, arr2) FROM t_arr");
+ }
+
private static Lambda innermostLambda(Plan plan) {
List<Lambda> innermostLambdas = plan.<Plan>collect(node ->
true).stream()
.flatMap(node -> node.getExpressions().stream())
diff --git
a/regression-test/data/query_p0/sql_functions/array_functions/test_array_sort_comparator_type.out
b/regression-test/data/query_p0/sql_functions/array_functions/test_array_sort_comparator_type.out
new file mode 100644
index 00000000000..24ee4d9c5b0
--- /dev/null
+++
b/regression-test/data/query_p0/sql_functions/array_functions/test_array_sort_comparator_type.out
@@ -0,0 +1,10 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !integer_comparator --
+[1, 2, 3] [3, 2, 1] [0, 128, 256] [1, 2, 3] [1, 2, 3]
[1, 2, 3]
+
+-- !integer_comparator_table --
+1 [1, 2, 3] [9000000000, 0, -9000000000]
+2 [0, 128, 256] [2, 1]
+3 [] []
+4 \N \N
+
diff --git
a/regression-test/suites/query_p0/sql_functions/array_functions/test_array_sort_comparator_type.groovy
b/regression-test/suites/query_p0/sql_functions/array_functions/test_array_sort_comparator_type.groovy
new file mode 100644
index 00000000000..5b771370d95
--- /dev/null
+++
b/regression-test/suites/query_p0/sql_functions/array_functions/test_array_sort_comparator_type.groovy
@@ -0,0 +1,86 @@
+// 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.
+
+suite("test_array_sort_comparator_type") {
+ // The comparator of array_sort returns -1, 0 or 1 for less than, equal to
or greater than.
+ // A comparator that does not return an integer is rejected when the query
is analyzed.
+ test {
+ sql "SELECT array_sort((x, y) -> x < y, [3, 2, 1])"
+ exception "the lambda must return -1, 0 or 1 for less than, equal to
or greater than, but it returns BOOLEAN"
+ }
+ test {
+ sql "SELECT array_sort((x, y) -> x - y, [3.5, 2.5, 1.5])"
+ exception "the lambda must return -1, 0 or 1"
+ }
+ test {
+ sql "SELECT array_sort((x, y) -> concat(x, y), ['b', 'a'])"
+ exception "the lambda must return -1, 0 or 1"
+ }
+
+ // Any integer type works if the comparator returns -1, 0 or 1. The
elements of an array are
+ // nullable, so sign(x - y) returns a nullable TINYINT.
+ order_qt_integer_comparator """
+ SELECT array_sort((x, y) -> sign(x - y), [3, 1, 2]),
+ array_sort((x, y) -> sign(y - x), [3, 1, 2]),
+ array_sort((x, y) -> sign(x - y), [256, 128, 0]),
+ array_sort((x, y) -> cast(sign(x - y) AS INT), [3, 1, 2]),
+ array_sort((x, y) -> cast(sign(x - y) AS BIGINT), [3, 1, 2]),
+ array_sort((x, y) -> cast(sign(x - y) AS LARGEINT), [3, 1, 2])
+ """
+
+ // A comparator that returns another value or NULL fails the query.
+ test {
+ sql "SELECT array_sort((x, y) -> 5, [3, 1, 2])"
+ exception "array_sort comparator returns 5, but it must return -1, 0
or 1"
+ }
+ test {
+ sql "SELECT array_sort((x, y) -> x - y, [30, 10, 20])"
+ exception "but it must return -1, 0 or 1"
+ }
+ test {
+ sql "SELECT array_sort((x, y) -> sign(x - y), [3, NULL, 1])"
+ exception "array_sort comparator returns NULL"
+ }
+
+ sql "DROP TABLE IF EXISTS test_array_sort_comparator_type"
+ sql """
+ CREATE TABLE test_array_sort_comparator_type (
+ id INT,
+ a ARRAY<INT>,
+ b ARRAY<BIGINT>
+ ) DUPLICATE KEY(id)
+ DISTRIBUTED BY HASH(id) BUCKETS 1
+ PROPERTIES ("replication_num" = "1")
+ """
+ sql """
+ INSERT INTO test_array_sort_comparator_type VALUES
+ (1, [3, 1, 2], [9000000000, -9000000000, 0]),
+ (2, [256, 128, 0], [1, 2]),
+ (3, [], []),
+ (4, NULL, NULL)
+ """
+ // The elements of a table column are nullable, so the comparator result
is nullable too. It
+ // works as long as the comparator does not return NULL.
+ order_qt_integer_comparator_table """
+ SELECT id, array_sort((x, y) -> sign(x - y), a), array_sort((x, y) ->
sign(y - x), b)
+ FROM test_array_sort_comparator_type
+ """
+ test {
+ sql "SELECT id, array_sort((x, y) -> x - y, a) FROM
test_array_sort_comparator_type"
+ exception "but it must return -1, 0 or 1"
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
