github-actions[bot] commented on code in PR #66590:
URL: https://github.com/apache/doris/pull/66590#discussion_r3749908402
##########
be/src/exprs/lambda_function/varray_map_function.cpp:
##########
@@ -325,45 +414,102 @@ class ArrayMapFunction : public LambdaFunction {
res_type = children[0]->execute_type(&lambda_block);
if (!result_col) {
- result_col = res_col->clone_empty();
+ result_col = IColumn::mutate(std::move(res_col));
+ } else {
+ result_col->insert_range_from(*res_col, 0, res_col->size());
}
- result_col->insert_range_from(*res_col, 0, res_col->size());
lambda_block.clear_column_data(column_size);
} while (args_info.current_row_idx < count);
//4. get the result column after execution, reassemble it into a new
array column, and return.
- if (result_type->is_nullable()) {
- if (res_type->is_nullable()) {
- result_column = ColumnNullable::create(
- ColumnArray::create(std::move(result_col),
std::move(array_column_offset)),
- std::move(outside_null_map));
- } else {
- // deal with eg: select array_map(x -> x is null, [null, 1,
2]);
- // need to create the nested column null map for column array
- auto nested_null_map = ColumnUInt8::create(result_col->size(),
0);
-
- result_column = ColumnNullable::create(
-
ColumnArray::create(ColumnNullable::create(std::move(result_col),
-
std::move(nested_null_map)),
- std::move(array_column_offset)),
- std::move(outside_null_map));
- }
- } else {
- if (res_type->is_nullable()) {
- result_column =
- ColumnArray::create(std::move(result_col),
std::move(array_column_offset));
- } else {
- auto nested_null_map = ColumnUInt8::create(result_col->size(),
0);
+ result_column = _create_result_column(std::move(result_col),
std::move(array_column_offset),
+ std::move(outside_null_map),
res_type, result_type);
+ return Status::OK();
+ }
+
+private:
+ static bool _has_variable_length_column(const VExprSPtr& expr) {
+ return !expr->data_type()->have_maximum_size_of_value() ||
+ std::ranges::any_of(expr->children(), [](const auto& child) {
+ return _has_variable_length_column(child);
+ });
+ }
- result_column = ColumnArray::create(
- ColumnNullable::create(std::move(result_col),
std::move(nested_null_map)),
- std::move(array_column_offset));
+ // A referenced non-const capture is expanded once for every nested array
element before
+ // lambda evaluation. Expanding the full nested cardinality at once can
create multi-gigabyte
+ // temporary columns (for example, a 5,000-byte VARCHAR repeated 1,000,000
times) and exceed
+ // ColumnString's UInt32 offset limit. Keep capture expansion and lambda
evaluation within the
+ // runtime block budget, while retaining direct nested-input reuse when
one batch is sufficient.
+ // Rule: use the external row budget if any lambda input, output, or
intermediate column
+ // is variable-length. Fixed-width columns have predictable memory usage,
so also apply
+ // the external byte budget to their estimated bytes per row.
+ size_t _calculate_lambda_batch_size(const VExprSPtr& lambda_expr,
+ const std::vector<ColumnPtr>&
lambda_datas,
+ const Block* block,
+ const std::set<int>&
required_input_column_ids,
+ bool has_row_dependent_captures) const
{
+ const auto add_bytes_with_saturation = [](size_t current_bytes, size_t
additional_bytes) {
+ constexpr size_t max_bytes = std::numeric_limits<size_t>::max();
+ return additional_bytes > max_bytes - current_bytes ? max_bytes
+ :
current_bytes + additional_bytes;
+ };
+
+ if (_has_variable_length_column(lambda_expr)) {
Review Comment:
[P1] Keep expanded variable-width captures within the byte budget
This early return skips `preferred_block_size_bytes` exactly when a
row-dependent `STRING`/`ARRAY` capture can be most expensive. For example, with
`batch_size = 65535`, one non-const 70,000-byte string capture and a
65,535-element input array take the direct path, which repeats that string into
at least 4,587,450,000 bytes before evaluating an otherwise fixed-width lambda.
That exceeds `ColumnString`'s 4,294,967,295-byte offset limit and fails a valid
query even though the normal block budget is much smaller. Please include the
measured `get_max_row_byte_size()` of expanded non-const captures (and other
known variable inputs) in `effective_max_rows`, while retaining a conservative
policy for unknown intermediates, and add a large variable-capture test that
proves the work is split below the byte/offset limit.
##########
be/src/exprs/lambda_function/varray_map_function.cpp:
##########
@@ -325,45 +414,102 @@ class ArrayMapFunction : public LambdaFunction {
res_type = children[0]->execute_type(&lambda_block);
if (!result_col) {
- result_col = res_col->clone_empty();
+ result_col = IColumn::mutate(std::move(res_col));
+ } else {
+ result_col->insert_range_from(*res_col, 0, res_col->size());
}
- result_col->insert_range_from(*res_col, 0, res_col->size());
lambda_block.clear_column_data(column_size);
} while (args_info.current_row_idx < count);
//4. get the result column after execution, reassemble it into a new
array column, and return.
- if (result_type->is_nullable()) {
- if (res_type->is_nullable()) {
- result_column = ColumnNullable::create(
- ColumnArray::create(std::move(result_col),
std::move(array_column_offset)),
- std::move(outside_null_map));
- } else {
- // deal with eg: select array_map(x -> x is null, [null, 1,
2]);
- // need to create the nested column null map for column array
- auto nested_null_map = ColumnUInt8::create(result_col->size(),
0);
-
- result_column = ColumnNullable::create(
-
ColumnArray::create(ColumnNullable::create(std::move(result_col),
-
std::move(nested_null_map)),
- std::move(array_column_offset)),
- std::move(outside_null_map));
- }
- } else {
- if (res_type->is_nullable()) {
- result_column =
- ColumnArray::create(std::move(result_col),
std::move(array_column_offset));
- } else {
- auto nested_null_map = ColumnUInt8::create(result_col->size(),
0);
+ result_column = _create_result_column(std::move(result_col),
std::move(array_column_offset),
+ std::move(outside_null_map),
res_type, result_type);
+ return Status::OK();
+ }
+
+private:
+ static bool _has_variable_length_column(const VExprSPtr& expr) {
+ return !expr->data_type()->have_maximum_size_of_value() ||
+ std::ranges::any_of(expr->children(), [](const auto& child) {
+ return _has_variable_length_column(child);
+ });
+ }
- result_column = ColumnArray::create(
- ColumnNullable::create(std::move(result_col),
std::move(nested_null_map)),
- std::move(array_column_offset));
+ // A referenced non-const capture is expanded once for every nested array
element before
+ // lambda evaluation. Expanding the full nested cardinality at once can
create multi-gigabyte
+ // temporary columns (for example, a 5,000-byte VARCHAR repeated 1,000,000
times) and exceed
+ // ColumnString's UInt32 offset limit. Keep capture expansion and lambda
evaluation within the
+ // runtime block budget, while retaining direct nested-input reuse when
one batch is sufficient.
+ // Rule: use the external row budget if any lambda input, output, or
intermediate column
+ // is variable-length. Fixed-width columns have predictable memory usage,
so also apply
+ // the external byte budget to their estimated bytes per row.
+ size_t _calculate_lambda_batch_size(const VExprSPtr& lambda_expr,
+ const std::vector<ColumnPtr>&
lambda_datas,
+ const Block* block,
+ const std::set<int>&
required_input_column_ids,
+ bool has_row_dependent_captures) const
{
+ const auto add_bytes_with_saturation = [](size_t current_bytes, size_t
additional_bytes) {
+ constexpr size_t max_bytes = std::numeric_limits<size_t>::max();
+ return additional_bytes > max_bytes - current_bytes ? max_bytes
+ :
current_bytes + additional_bytes;
+ };
+
+ if (_has_variable_length_column(lambda_expr)) {
+ return _lambda_block_budget.max_rows;
+ }
+
+ size_t estimated_lambda_bytes_per_row =
lambda_expr->estimate_memory(1);
Review Comment:
[P2] Exclude alias-only lambda references from the byte estimate
`lambda_expr->estimate_memory(1)` charges every `VColumnRef` occurrence as a
newly allocated value, and the loop below then charges the backing
`lambda_data` again. Inside `array_map` the lambda is executed with a null
selector, so those refs only return the existing lambda-block column. For
example, 64 references to `x` in a fixed-width `greatest(x, ... x)` over 20,000
nullable INTs with a 1 MiB budget are estimated at roughly 320 bytes/row and
split into about seven batches, although the single input plus result is only
about 200 KiB and fits the direct path. Please make this lambda-local estimate
treat forwarding refs (and the forwarding lambda wrapper) as aliases rather
than materialized intermediates, without changing selector-aware estimates
globally, and add an adaptive-budget test with repeated argument refs.
--
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]
