github-actions[bot] commented on code in PR #68551:
URL: https://github.com/apache/doris/pull/68551#discussion_r4119786784
##########
be/src/exprs/vectorized_fn_call.cpp:
##########
@@ -311,6 +311,13 @@ Status VectorizedFnCall::prepare(RuntimeState* state,
const RowDescriptor& desc,
_fn.name.function_name,
get_child_type_names(),
_data_type->get_name());
}
+ for (auto arg_num : _function->get_arguments_that_are_always_constant()) {
+ if (arg_num < _children.size() && !_children[arg_num]->is_constant()) {
+ return Status::InvalidArgument(
Review Comment:
This rejects a valid constant expression when a child uses custom constant
handling. For example, `array_apply([1,2,3], '>', length(from_base64('MQ==')) +
1)` passes FE's constant-argument check and stays as an expression because BE
folding skips `from_base64`. The BE `add` function sets
`use_default_implementation_for_constants() = false`, so its
`VectorizedFnCall::is_constant()` is false, but its constant/constant execution
returns `ColumnConst`; the old execution check accepted it. Please distinguish
a stable expression from whether a function uses the default constant
implementation, and cover this retained-expression case.
##########
be/src/exprs/vectorized_fn_call.cpp:
##########
@@ -311,6 +311,13 @@ Status VectorizedFnCall::prepare(RuntimeState* state,
const RowDescriptor& desc,
_fn.name.function_name,
get_child_type_names(),
_data_type->get_name());
}
+ for (auto arg_num : _function->get_arguments_that_are_always_constant()) {
+ if (arg_num < _children.size() && !_children[arg_num]->is_constant()) {
+ return Status::InvalidArgument(
+ "Argument at index {} for function {} must be a constant
expression", arg_num,
+ _function->get_name());
Review Comment:
The new message breaks existing negative regression expectations:
`test_mask_function.groovy` checks `Argument at index ... for function mask
must be constant` at three positions, and `test_unicode_normalize.groovy`
checks `must be constant`. These plans now fail in `prepare()` with `must be a
constant expression` before the old execute/open errors can run, so those
`exception` substring checks will fail. Please update the expectations with
this change (or preserve the existing diagnostic wording).
--
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]
