github-actions[bot] commented on code in PR #63391:
URL: https://github.com/apache/doris/pull/63391#discussion_r3285579301
##########
be/src/exprs/function/function.cpp:
##########
@@ -400,4 +400,55 @@ bool
FunctionBuilderImpl::is_nested_type_date_or_datetime_or_decimal(
}
}
+Status IFunctionBase::mock_const_execute(FunctionContext* context, Block&
block,
+ const ColumnNumbers& arguments,
uint32_t result,
+ size_t input_rows_count) const {
+ if (!is_udf_function()) {
+ try {
+ Block const_block;
+ for (size_t col = 0; col < block.columns(); ++col) {
+ auto& col_data = block.get_by_position(col);
+ if (col_data.column) {
+ auto one_row = col_data.column->cut(0, 1);
+ auto const_col = ColumnConst::create(std::move(one_row),
input_rows_count);
+ const_block.insert({std::move(const_col), col_data.type,
col_data.name});
+ } else {
+ const_block.insert(col_data);
+ }
+ }
+ RETURN_IF_ERROR(
+ prepare(context, const_block, arguments, result)
+ ->execute(context, const_block, arguments, result,
input_rows_count));
+ } catch (const Exception&) {
Review Comment:
This `try` covers both building const columns and executing the prepared
function, so any `doris::Exception` thrown by the function under all-const
inputs is silently ignored. That hides the exact class of const-column bugs
this debug check is supposed to expose; only `Status` failures are propagated.
Please narrow the catch to the `cut`/const-block construction that may be
unsupported, or otherwise distinguish construction failures from exceptions
raised by `execute` and propagate the latter.
##########
be/src/exprs/function/function.cpp:
##########
@@ -400,4 +400,55 @@ bool
FunctionBuilderImpl::is_nested_type_date_or_datetime_or_decimal(
}
}
+Status IFunctionBase::mock_const_execute(FunctionContext* context, Block&
block,
+ const ColumnNumbers& arguments,
uint32_t result,
+ size_t input_rows_count) const {
+ if (!is_udf_function()) {
+ try {
+ Block const_block;
+ for (size_t col = 0; col < block.columns(); ++col) {
+ auto& col_data = block.get_by_position(col);
+ if (col_data.column) {
+ auto one_row = col_data.column->cut(0, 1);
+ auto const_col = ColumnConst::create(std::move(one_row),
input_rows_count);
+ const_block.insert({std::move(const_col), col_data.type,
col_data.name});
+ } else {
+ const_block.insert(col_data);
+ }
+ }
+ RETURN_IF_ERROR(
+ prepare(context, const_block, arguments, result)
+ ->execute(context, const_block, arguments, result,
input_rows_count));
+ } catch (const Exception&) {
+ // some column not support cut, just ignore and return OK.
+ }
+ }
+ return Status::OK();
+}
+
+Status IFunctionBase::execute(FunctionContext* context, Block& block,
+ const ColumnNumbers& arguments, uint32_t result,
+ size_t input_rows_count) const {
+ // Some function implementations may not handle the case where
input_rows_count is 0
+ // (e.g., some functions access the 0th row of input columns during
execution).
+ // Additionally, some UDF functions may hang if they write 0 rows and then
try to read.
+ // Therefore, before executing the function, we first check if
input_rows_count is 0.
+ // If it is 0, we directly return an empty result column to avoid
executing the function body.
+ if (input_rows_count == 0) {
+ block.get_by_position(result).column =
block.get_by_position(result).type->create_column();
+ return Status::OK();
+ }
+
+#ifndef NDEBUG
+ RETURN_IF_ERROR(mock_const_execute(context, block, arguments, result,
input_rows_count));
+#endif
+
+ try {
Review Comment:
Blocking: this runs the function implementation once on `const_block` and
then immediately runs it again on the real block in every non-`NDEBUG` build.
That is not side-effect free for existing non-UDF functions: `random(seed)`
stores its `std::mt19937_64` in `FunctionContext` and `_execute_float` advances
it, so debug/ASAN builds now return the second block of random values (the PR
had to skip `random_test`). RPC scalar functions (`FunctionRPC::prepare` ->
`RPCPreparedFunction::execute` -> `RPCFnImpl::vec_call`) are also not
`is_udf_function()`, so this doubles external RPC calls. The generic execute
path cannot speculatively execute arbitrary functions against the live
`FunctionContext`; restrict this to functions that explicitly opt in to a pure
const-mock check, or use isolated state, and keep behavior-preserving tests
enabled.
--
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]
