github-actions[bot] commented on code in PR #67218:
URL: https://github.com/apache/doris/pull/67218#discussion_r3871903106
##########
be/src/exprs/function/function_string_misc.cpp:
##########
@@ -100,20 +100,23 @@ class FunctionAutoPartitionName : public IFunction {
size_t get_number_of_arguments() const override { return 0; }
bool is_variadic() const override { return true; }
bool use_default_implementation_for_nulls() const override { return false;
}
+ bool use_default_implementation_for_constants() const override { return
false; }
Review Comment:
[P1] Preserve the constant-expression contract here
This override is also what `VectorizedFnCall::is_constant()` consults, so it
does more than keep the control columns wrapped during `execute_impl()`. With
folding skipped, `auto_partition_name('list', 'x')` now produces an ordinary
`ColumnString` instead of a cached `ColumnConst`. For example, `SELECT
/*+SET_VAR(debug_skip_fold_constant=true)*/ trim('abc',
auto_partition_name('list', 'x'))` reaches
`FunctionTrim::get_arguments_that_are_always_constant() == {1}` and is rejected
by the generic constant-argument check, although the same deterministic nested
expression was constant before this change. Literal-only projections also
recompute and allocate the name once per input row (load planning is one path
that explicitly skips folding). Please preserve `VectorizedFnCall` constness
while validating the original control arguments through `FunctionContext`
constant-column metadata or another decoupled mechanism, and add a no-fold
nested regression.
##########
be/src/exprs/function/function_string_misc.cpp:
##########
@@ -122,40 +125,49 @@ class FunctionAutoPartitionName : public IFunction {
if (const auto* nullable =
check_and_get_column<const
ColumnNullable>(*argument_columns[i])) {
null_list[i] = &nullable->get_null_map_data();
Review Comment:
[P1] Keep the materialized null map alive
For a `ColumnConst(ColumnNullable(...))`,
`convert_to_full_column_if_const()` creates a new full `ColumnNullable` with a
freshly allocated null map. `null_list[i]` keeps only a raw pointer into that
map, and the next assignment retains the nested string but releases the sole
owner of the nullable parent and its null map. The later checks at lines
151/223/275 therefore read freed storage. The deleted `argument_null_columns[i]
= nullable->get_null_map_column_ptr()` was the lifetime guard; the changed
`Consted` tests build this exact const-nullable shape, as do supported LIST
calls with NULL values. Please retain either the full materialized nullable
column or its null-map `ColumnPtr` for the duration of execution, and cover
both NULL and non-NULL const-nullable inputs under ASAN.
--
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]
