HappenLee commented on code in PR #68359:
URL: https://github.com/apache/doris/pull/68359#discussion_r4236276438
##########
be/src/exprs/function/function_regexp.cpp:
##########
@@ -74,6 +76,60 @@ static bool advance_re2_search_position(const char* data,
size_t size,
return true;
}
+// The regexp functions below handle NULL rows themselves
(use_default_implementation_for_nulls()
+// returns false). The framework's default path runs a function over the
nested column of a
+// Nullable argument, and the bytes stored under a NULL slot are whatever the
producer left there;
+// compiling them as a pattern could fail and abort a query whose result for
that row is simply
+// NULL.
+//
+// Strips Nullable from every argument into `nested_block` (a ColumnConst
wrapper stays, so the
+// const/full handling of the functions applies unchanged) and ORs the
argument null maps into
+// `null_map`, which the functions skip while executing and hand back as the
result null map.
+// Returns false when an argument is a NULL constant: the whole result is NULL.
+bool unnest_regexp_arguments(const Block& block, const ColumnNumbers&
arguments,
+ Block& nested_block, ColumnNumbers&
nested_arguments,
+ NullMap& null_map) {
+ for (const auto argument : arguments) {
+ const auto& column = block.get_by_position(argument);
+ NullableColumnInfo info;
+ if (column.type->is_nullable()) {
+ info = column.get_nullable_column_info();
+ if (info.only_null) {
+ return false;
+ }
+ if (info.has_null) {
+ // A ColumnConst holding NULL is only_null, so this is a full
column.
+ DCHECK(!info.is_const);
+ VectorizedUtils::update_null_map(null_map,
+
column.get_nullable_null_map_column()->get_data());
+ }
+ }
+ nested_arguments.push_back(nested_block.columns());
+ nested_block.insert(column.unnest_nullable(info, false));
+ }
+ return true;
+}
+
+// Shared execute() prologue of the regexp functions: a NULL constant argument
makes the result
+// a NULL constant, otherwise `execute` runs over the Nullable-stripped
arguments and returns the
+// result column already wrapped with `null_map`.
+template <typename Execute>
+Status execute_regexp_with_nulls(Block& block, const ColumnNumbers& arguments,
uint32_t result,
+ size_t input_rows_count, Execute&& execute) {
+ auto& result_column = block.get_by_position(result);
+ auto null_map = ColumnUInt8::create(input_rows_count, 0);
Review Comment:
The `regexp_count` fast path in `554e1207632` addresses my previous
performance comment. Please apply the same principle through the shared regexp
executor, so the nullable/non-nullable dispatch is implemented once rather than
separately in each function.
`FunctionRegexpReplace::execute_impl()` and
`FunctionRegexpFunctionality::execute_impl()` still enter
`execute_regexp_with_nulls()` unconditionally. With entirely non-nullable
inputs, this builds `nested_block` and `nested_arguments` even though no
Nullable wrapper needs stripping, and the extract/replace loops still
read/check an all-zero input NULL map. These are metadata-wrapping and per-row
checking costs; I have not benchmarked their impact.
A focused approach would be:
- Make the shared executor select the path once per block using
`have_null_column(block, arguments)`.
- For non-nullable inputs, pass the original block/arguments directly to an
`execute<false>` specialization, with no input NULL map or temporary nested
block. Compile out input NULL checks with `if constexpr (CheckNull)`.
- For nullable inputs, reuse the current argument unnesting, NULL-map
merging, all-NULL short-circuit, and `execute<true>` path that skips NULL rows
before compiling patterns.
- Route count, extract, extract-all, and replace through this dispatch while
retaining their existing algorithms and result handlers.
Please keep **input NULL checking separate from output nullability**. The
extract/replace output NULL maps already existed before this PR; they are not
all new overhead. In particular, `regexp_extract_or_null` can produce NULL for
non-nullable inputs when there is no match, and a caller-declared nullable
count result still needs its output wrapper. `CheckNull=false` should only
remove input NULL processing, not these output semantics.
This can stay within the existing regexp helper; changing the default NULL
contract for all Doris functions is unnecessary for this fix. Please retain the
hidden-invalid-payload and count fast-path tests, and cover non-nullable
extract/replace inputs, including constant patterns and the no-match NULL
result of `regexp_extract_or_null`.
--
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]
