HappenLee commented on code in PR #68359:
URL: https://github.com/apache/doris/pull/68359#discussion_r4209900906
##########
be/src/exprs/function/function_regexp.cpp:
##########
@@ -288,17 +355,24 @@ class FunctionRegexpCount : public IFunction {
Status execute_impl(FunctionContext* context, Block& block, const
ColumnNumbers& arguments,
uint32_t result, size_t input_rows_count) const
override {
- auto result_data_column = ColumnInt32::create(input_rows_count);
- auto& result_data = result_data_column->get_data();
-
- ColumnPtr argument_columns[2];
-
- argument_columns[0] = block.get_by_position(arguments[0]).column;
- argument_columns[1] = block.get_by_position(arguments[1]).column;
- RegexpCountImpl::execute_impl(context, argument_columns,
input_rows_count, result_data);
-
- block.get_by_position(result).column = std::move(result_data_column);
- return Status::OK();
+ const bool result_nullable =
block.get_by_position(result).type->is_nullable();
+ return execute_regexp_with_nulls(
Review Comment:
Please preserve the non-nullable fast path of `regexp_count` before merging.
When both input columns and the result are non-nullable, this call still
enters `execute_regexp_with_nulls()`, which unconditionally allocates and
zero-fills `ColumnUInt8::create(input_rows_count, 0)`.
`RegexpCountImpl::execute_impl()` then reads/checks that all-zero map for every
row, and the `!result_nullable` branch ultimately discards it. Previously, the
framework skipped nullable handling for non-nullable inputs and the count loop
did not perform these checks.
This introduces an unnecessary per-block allocation/initialization
(approximately one byte per row, excluding overhead) and per-row null-map
checks on a path that cannot contain SQL NULLs. I have not benchmarked the
impact, so this is a request to remove the avoidable work rather than a claim
of a measured slowdown.
Suggested minimal change:
- Select the execution path once per block using the existing
`have_null_column(block, arguments)` helper.
- For non-nullable inputs/results, pass the original argument columns
directly to the count implementation and return the plain integer result,
without constructing the temporary nullable block or NULL map.
- Reuse the count loop with a compile-time flag, e.g.
`execute_impl<CheckNull>(..., const NullMap* null_map)`, and put the null check
behind `if constexpr (CheckNull)`. The non-nullable specialization can receive
`nullptr`, so it contains no per-row null-map check.
- Keep the current helper and NULL-row skipping for the nullable path.
Preserve the declared result type if a caller requests a nullable output, and
keep the existing constant-pattern cache and invalid-pattern error handling.
Please cover the non-nullable count path alongside the existing
mixed-NULL/invalid-hidden-payload cases. This can stay local to `regexp_count`;
there is no need to redesign the general function framework.
--
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]
