github-actions[bot] commented on code in PR #68022:
URL: https://github.com/apache/doris/pull/68022#discussion_r4045954616
##########
be/src/exprs/function/array/function_array_enumerate_uniq.cpp:
##########
@@ -116,36 +121,111 @@ class FunctionArrayEnumerateUniq : public IFunction {
Status execute_impl(FunctionContext* context, Block& block, const
ColumnNumbers& arguments,
uint32_t result, size_t input_rows_count) const
override {
- ColumnRawPtrs data_columns(arguments.size());
- const ColumnArray::Offsets64* offsets = nullptr;
- ColumnPtr src_offsets;
- Columns src_columns; // to keep ownership
-
- const ColumnArray* first_column_array = nullptr;
+ ColumnUInt8::MutablePtr result_null_map;
+ ColumnUInt8::Container* result_null_map_data = nullptr;
+ if (block.get_by_position(result).type->is_nullable()) {
+ result_null_map = ColumnUInt8::create(input_rows_count, 0);
+ result_null_map_data = &result_null_map->get_data();
+ }
- for (size_t i = 0; i < arguments.size(); i++) {
- src_columns.emplace_back(
-
block.get_by_position(arguments[i]).column->convert_to_full_column_if_const());
- ColumnPtr& cur_column = src_columns[i];
- const ColumnArray* array =
-
check_and_get_column<ColumnArray>(remove_nullable(cur_column->get_ptr()).get());
+ std::vector<const ColumnArray*> array_columns(arguments.size());
Review Comment:
[P2] Check later all-NULL arguments before materializing earlier constants
This check happens only after each argument is converted in order. Thus
`array_enumerate_uniq(large_const_array, nullable_column, ...)` expands the
first constant across the entire block before a later all-NULL column triggers
the inevitable NULL return. The old default nullable wrapper scanned all
nullable inputs before `execute_impl`, so this is a new O(rows * array length)
memory/CPU path and can OOM for otherwise cheap all-NULL blocks. Please
pre-scan the original columns for `only_null()` before any conversion and add a
mixed constant/nonconstant regression.
##########
be/src/exprs/function/array/function_array_split.cpp:
##########
@@ -54,17 +57,34 @@ class FunctionArraySplit : public IFunction {
size_t get_number_of_arguments() const override { return 2; }
+ bool use_default_implementation_for_nulls() const override { return false;
}
+
DataTypePtr get_return_type_impl(const DataTypes& arguments) const
override {
- return std::make_shared<DataTypeArray>(make_nullable(arguments[0]));
+ auto result_type =
+
std::make_shared<DataTypeArray>(make_nullable(remove_nullable(arguments[0])));
+ return have_nullable(arguments) ? make_nullable(result_type) :
result_type;
};
Status execute_impl(FunctionContext* context, Block& block, const
ColumnNumbers& arguments,
uint32_t result, size_t input_rows_count) const
override {
- // <Nullable>(Array(<Nullable>(Int)))
- auto src_column =
-
block.get_by_position(arguments[0]).column->convert_to_full_column_if_const();
- auto spliter_column =
-
block.get_by_position(arguments[1]).column->convert_to_full_column_if_const();
+ ColumnUInt8::MutablePtr result_null_map;
+ ColumnUInt8::Container* result_null_map_data = nullptr;
+ if (block.get_by_position(result).type->is_nullable()) {
+ result_null_map = ColumnUInt8::create(input_rows_count, 0);
+ result_null_map_data = &result_null_map->get_data();
+ }
+ auto unwrap_nullable_array = [&](const ColumnPtr& input_column) {
Review Comment:
[P2] Preserve the all-NULL fast path before expanding constants
With the default nullable wrapper disabled, this materializes both inputs
before recognizing that one input makes the whole block NULL. If a nonconstant
nullable source or predicate is all NULL for a scan block and the other
argument is a large constant array, `ColumnConst` repeats the full nested
payload for every block row even though the result is inevitably NULL; the old
wrapper returned before this expansion. This creates avoidable O(rows * array
length) allocation/copy and can cause memory pressure or OOM. Please pre-scan
the original inputs for `only_null()` (as `array_zip` does), return the typed
NULL constant before materializing, and add a mixed constant/all-NULL test.
--
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]
