github-actions[bot] commented on code in PR #67628:
URL: https://github.com/apache/doris/pull/67628#discussion_r4089937629
##########
be/src/exprs/lambda_function/varray_sort_function.cpp:
##########
@@ -202,33 +204,42 @@ class ArraySortFunction : public LambdaFunction {
};
const int lambda_result_base =
static_cast<int>(lambda_block.columns());
- for (int row = 0; row < input_rows; ++row) {
- auto start = off_data[row - 1];
- auto end = off_data[row];
- std::sort(&permutation[start], &permutation[end],
[&](size_t i, size_t j) {
- prepare_lambda_input(i, 0);
- prepare_lambda_input(j, 1);
- int lambda_res_id = lambda_result_base;
- auto status =
- children[0]->execute(context,
&lambda_block, &lambda_res_id);
- if (!status.ok()) [[unlikely]] {
- throw Exception(Status::InternalError(
- "when execute array_sort lambda
function: {}",
- status.to_string()));
- }
+ // Returns true when element i sorts before element j
according to the
+ // user's lambda.
+ auto less = [&](size_t i, size_t j) {
+ prepare_lambda_input(i, 0);
+ prepare_lambda_input(j, 1);
+ int lambda_res_id = lambda_result_base;
+ auto status = children[0]->execute(context,
&lambda_block, &lambda_res_id);
+ if (!status.ok()) [[unlikely]] {
+ throw Exception(Status::InternalError(
+ "when execute array_sort lambda function:
{}",
+ status.to_string()));
+ }
- // raw_res_col maybe columnVector or ColumnConst
- 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();
+ // raw_res_col maybe columnVector or ColumnConst
+ 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];
- lambda_block.erase_tail(lambda_result_base);
+ // only -1, 0, 1
+ long cmp =
+ assert_cast<const
ColumnInt8*>(full_res_col.get())->get_data()[0];
+ lambda_block.erase_tail(lambda_result_base);
- return cmp < 0;
- });
+ return cmp < 0;
+ };
+
+ // The comparator is user SQL and may violate strict weak
ordering, or
+ // even be non-deterministic. Standard library sorts rely
on the comparator
+ // contract to keep their accesses in range, so a broken
comparator crashes
+ // BE. sort_with_untrusted_comparator bounds every access
by the range
+ // length; an inconsistent comparator yields an
unspecified order instead.
+ std::vector<size_t> scratch;
Review Comment:
[P1] Keep the merge scratch on Doris's checked allocator path
This buffer is sized by the largest user array in the block, but
`std::vector<size_t>` does not go through the `Allocator::memory_check` path
used by the surrounding `IColumn::Permutation`. For example, a valid
one-million-element row allocates another 8 MiB after the tracked permutation
has already been charged; if the query is near its task/workload-group limit,
this allocation can take it past the limit instead of failing before allocation
with `MEMORY_LIMIT_EXCEEDED`. A raw `std::bad_alloc` also escapes the
`VExprContext` Doris-exception boundary and is only turned into a generic
internal error by the pipeline scheduler. Please make the helper's scratch
container allocator-generic and pass an allocator-aware Doris buffer (for
example `IColumn::Permutation`/`PaddedPODArray<size_t>` or
`DorisVector<size_t>`), while keeping the current cross-row capacity reuse.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
