[BugFix] Fix regexp_replace hyperscan_vec processing multiple rows incorrectly (backport #67380) (#67470)

Signed-off-by: stdpain <34912776+stdpain@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: stdpain <34912776+stdpain@users.noreply.github.com>

Author: 3 people

be/src/exprs/string_functions.cpp

@@ -54,6 +54,8 @@
 namespace starrocks {
 // A regex to match any regex pattern is equivalent to a substring search.
 static const RE2 SUBSTRING_RE(R"((?:\.\*)*([^\.\^\{\[\(\|\)\]\}\+\*\?\$\\]+)(?:\.\*)*)", re2::RE2::Quiet);
+// A strict fixed literal regex contains only normal characters without any regex special characters.
+static const RE2 FIXED_LITERAL_RE(R"(^[^\.\^\{\[\(\|\)\]\}\+\*\?\$\\]+$)", re2::RE2::Quiet);
 
 #define THROW_RUNTIME_ERROR_IF_EXCEED_LIMIT(col, func_name)                        \
     if (UNLIKELY(col->capacity_limit_reached())) {                                 \
@@ -3114,8 +3116,10 @@ Status StringFunctions::regexp_replace_prepare(FunctionContext* context, Functio
     state->pattern = pattern_str;
 
     std::string search_string;
+
     if (pattern_str.size() && RE2::FullMatch(pattern_str, SUBSTRING_RE, &search_string)) {
         state->use_hyperscan = true;
+        state->use_hyperscan_vec = RE2::FullMatch(pattern_str, FIXED_LITERAL_RE, &search_string);
         state->size_of_pattern = pattern.size;
         std::string re_pattern(pattern.data, pattern.size);
         RETURN_IF_ERROR(hs_compile_and_alloc_scratch(re_pattern, state, context, pattern));
@@ -3586,35 +3590,36 @@ static StatusOr<ColumnPtr> hyperscan_vec_evaluate(const BinaryColumn* src, Strin
     raw::make_room(&dst_offsets, num_rows + 1);
     dst_bytes.reserve(data_count());
 
-    // copy data
+    // copy data row by row with replacements applied
     char* cursor = reinterpret_cast<char*>(dst_bytes.data());
-    size_t last_to = 0;
-    for (const auto& info : match_info_chain_in_one_row.info_chain) {
-        strings::memcpy_inlined(cursor, data + last_to, info.from - last_to);
-        cursor += info.from - last_to;
-        strings::memcpy_inlined(cursor, rpl_value.data(), rpl_value.size());
-        cursor += rpl_value.size();
-        last_to = info.to;
-    }
-    strings::memcpy_inlined(cursor, data + last_to, src_value_size - last_to);
-
-    // split offset
     size_t match_index = 0;
-    dst_offsets[0] = 0;
     size_t match_size = match_info_chain_in_one_row.info_chain.size();
+    dst_offsets[0] = 0;
+
     for (size_t i = 0; i < num_rows; i++) {
-        size_t from = src_offsets[i];
-        size_t to = src_offsets[i + 1];
-        size_t dis = to - from;
-        DCHECK(match_index == match_size || match_info_chain_in_one_row.info_chain[match_index].to > from);
-        while (match_index < match_size && match_info_chain_in_one_row.info_chain[match_index].from >= from &&
-               match_info_chain_in_one_row.info_chain[match_index].to <= to) {
-            dis -= match_info_chain_in_one_row.info_chain[match_index].to -
-                   match_info_chain_in_one_row.info_chain[match_index].from;
-            dis += rpl_value.size();
+        size_t row_start = src_offsets[i];
+        size_t row_end = src_offsets[i + 1];
+        size_t last_to = row_start;
+
+        // Process all matches in this row
+        while (match_index < match_size && match_info_chain_in_one_row.info_chain[match_index].from >= row_start &&
+               match_info_chain_in_one_row.info_chain[match_index].to <= row_end) {
+            const auto& info = match_info_chain_in_one_row.info_chain[match_index];
+            // Copy data before the match
+            strings::memcpy_inlined(cursor, data + last_to, info.from - last_to);
+            cursor += info.from - last_to;
+            // Copy replacement
+            strings::memcpy_inlined(cursor, rpl_value.data(), rpl_value.size());
+            cursor += rpl_value.size();
+            last_to = info.to;
             match_index++;
         }
-        dst_offsets[i + 1] = dst_offsets[i] + dis;
+        // Copy remaining data in this row
+        strings::memcpy_inlined(cursor, data + last_to, row_end - last_to);
+        cursor += row_end - last_to;
+
+        // Calculate offset for this row
+        dst_offsets[i + 1] = cursor - reinterpret_cast<char*>(dst_bytes.data());
     }
     DCHECK(match_index == match_size);
     DCHECK(dst_offsets.back() == data_count());
@@ -3723,7 +3728,7 @@ StatusOr<ColumnPtr> StringFunctions::regexp_replace(FunctionContext* context, co
 
     if (state->const_pattern) {
         if (state->use_hyperscan) {
-            if (columns[2]->is_constant() && context->state()->enable_hyperscan_vec()) {
+            if (columns[2]->is_constant() && context->state()->enable_hyperscan_vec() && state->use_hyperscan_vec) {
                 return regexp_replace_use_hyperscan_vec(state, columns);
             } else {
                 return regexp_replace_use_hyperscan(state, columns);

be/src/exprs/string_functions.h

@@ -62,6 +62,8 @@ struct StringFunctionsState {
     DriverMap driver_regex_map; // regex for each pipeline_driver, to make it driver-local
 
     bool use_hyperscan = false;
+    bool use_hyperscan_vec = false;
+
     int size_of_pattern = -1;
 
     // a pointer to the generated database that responsible for parsed expression.

be/test/exprs/string_fn_regexp_replace_test.cpp

@@ -25,6 +25,7 @@
 #include "common/statusor.h"
 #include "exprs/exprs_test_helper.h"
 #include "exprs/string_functions.h"
+#include "util/defer_op.h"
 
 namespace starrocks {
 
@@ -63,6 +64,7 @@ class StringFunctionRegexpReplaceTest : public ::testing::Test {
         std::string pattern = "-";
         _state->pattern = pattern;
         _state->use_hyperscan = true;
+        _state->use_hyperscan_vec = true;
         _state->size_of_pattern = int(pattern.size());
         if (hs_compile(pattern.c_str(), HS_FLAG_ALLOWEMPTY | HS_FLAG_DOTALL | HS_FLAG_UTF8 | HS_FLAG_SOM_LEFTMOST,
                        HS_MODE_BLOCK, nullptr, &_state->database, &_state->compile_err) != HS_SUCCESS) {
@@ -109,4 +111,4 @@ TEST_F(StringFunctionRegexpReplaceTest, testHyperscanVec) {
     }
 }
 
-} // namespace starrocks
+} // namespace starrocks

test/sql/test_function/R/test_regex

@@ -170,4 +170,63 @@ SELECT regexp_extract_all(str, regex, 3) from tsr;
 -- result:
 []
 []
--- !result
+-- !result
+-- name: test_regexp_replace_hyperscan_vec_multiple_rows
+CREATE TABLE `test_regexp_replace_multirow` (
+  `column_value` varchar(100) NULL COMMENT ""
+) ENGINE=OLAP 
+DUPLICATE KEY(`column_value`)
+COMMENT "OLAP"
+DISTRIBUTED BY HASH(`column_value`) BUCKETS 1 PROPERTIES ("replication_num" = "1");
+-- result:
+-- !result
+insert into test_regexp_replace_multirow values 
+('activity_60_package_2'),
+('activity_50_package_7'),
+('activity_70_package_1'),
+('nomatch_value'),
+('test_package_suffix');
+-- result:
+-- !result
+SELECT regexp_replace(column_value, '_package_.*', '') 
+FROM (VALUES 
+    ('activity_60_package_2'),
+    ('activity_50_package_7')
+) AS t(column_value);
+-- result:
+activity_60
+activity_50
+-- !result
+SELECT regexp_replace(column_value, '_package_.*', '') 
+FROM test_regexp_replace_multirow 
+ORDER BY column_value;
+-- result:
+activity_50
+activity_60
+activity_70
+nomatch_value
+test
+-- !result
+SELECT regexp_replace(column_value, '_[0-9]+', '_X') 
+FROM test_regexp_replace_multirow 
+ORDER BY column_value;
+-- result:
+activity_X_package_X
+activity_X_package_X
+activity_X_package_X
+nomatch_value
+test_package_suffix
+-- !result
+SELECT regexp_replace(column_value, '_package_', '_pkg_') 
+FROM test_regexp_replace_multirow 
+ORDER BY column_value;
+-- result:
+activity_50_pkg_7
+activity_60_pkg_2
+activity_70_pkg_1
+nomatch_value
+test_pkg_suffix
+-- !result
+DROP TABLE test_regexp_replace_multirow;
+-- result:
+-- !result

test/sql/test_function/T/test_regex

@@ -62,3 +62,48 @@ SELECT regexp_extract_all("AbCdExCeF", "([[:lower:]]+)C([[:lower:]]+)", 3);
 SELECT regexp_extract_all(str, "([[:lower:]]+)C([[:lower:]]+)", 3) from tsr;
 SELECT regexp_extract_all(str, regex, 3) from tsr;
 
+-- name: test_regexp_replace_hyperscan_vec_multiple_rows
+
+-- Test for issue #65936: regexp_replace with enable_hyperscan_vec=true 
+-- produces incorrect output when processing multiple rows
+-- This test ensures row boundaries are respected during data reconstruction
+
+CREATE TABLE `test_regexp_replace_multirow` (
+  `column_value` varchar(100) NULL COMMENT ""
+) ENGINE=OLAP 
+DUPLICATE KEY(`column_value`)
+COMMENT "OLAP"
+DISTRIBUTED BY HASH(`column_value`) BUCKETS 1 PROPERTIES ("replication_num" = "1");
+
+-- Insert test data with pattern that should be replaced
+insert into test_regexp_replace_multirow values 
+('activity_60_package_2'),
+('activity_50_package_7'),
+('activity_70_package_1'),
+('nomatch_value'),
+('test_package_suffix');
+
+-- Test with VALUES clause (original bug report scenario)
+SELECT regexp_replace(column_value, '_package_.*', '') 
+FROM (VALUES 
+    ('activity_60_package_2'),
+    ('activity_50_package_7')
+) AS t(column_value);
+
+-- Test with table data
+SELECT regexp_replace(column_value, '_package_.*', '') 
+FROM test_regexp_replace_multirow 
+ORDER BY column_value;
+
+-- Test with different patterns to ensure general correctness
+SELECT regexp_replace(column_value, '_[0-9]+', '_X') 
+FROM test_regexp_replace_multirow 
+ORDER BY column_value;
+
+-- Test with non-empty replacement
+SELECT regexp_replace(column_value, '_package_', '_pkg_') 
+FROM test_regexp_replace_multirow 
+ORDER BY column_value;
+
+-- Clean up
+DROP TABLE test_regexp_replace_multirow;