From 16cef4419e434b25d13161811daacdf0da08c2c1 Mon Sep 17 00:00:00 2001 From: ccl125 <1253846621@qq.com> Date: Sun, 26 Jul 2026 02:33:37 +0800 Subject: [PATCH 1/5] [Feature](func) Support the third argument for regexp_extract_all and regexp_extract_all_array Add an optional group-index argument (Spark semantics, default 1) to regexp_extract_all and regexp_extract_all_array. Index 0 extracts the whole match, a positive index extracts the corresponding capturing group, an out-of-range index yields an empty result, and a negative index yields NULL. Two-argument calls are normalized in the FE by padding the default index, keeping existing behavior unchanged. --- be/src/exprs/function/function_regexp.cpp | 54 +++++--- be/test/exprs/function/function_like_test.cpp | 117 +++++++++++++----- .../functions/scalar/RegexpExtractAll.java | 23 +++- .../scalar/RegexpExtractAllArray.java | 23 +++- .../test_string_function_regexp.out | 18 +++ .../test_string_function_regexp.groovy | 7 ++ 6 files changed, 181 insertions(+), 61 deletions(-) diff --git a/be/src/exprs/function/function_regexp.cpp b/be/src/exprs/function/function_regexp.cpp index 206f51ce0e7d46..f21fe5537ac0c7 100644 --- a/be/src/exprs/function/function_regexp.cpp +++ b/be/src/exprs/function/function_regexp.cpp @@ -136,13 +136,17 @@ struct RegexpExtractEngine { return false; } - // Match all occurrences and extract the first capturing group - void match_all_and_extract(const char* data, size_t size, + // Match all occurrences and extract the capturing group with the given index. + // index 0 means the whole match, 1 means the first capturing group (the default), and so on. + void match_all_and_extract(const char* data, size_t size, int index, std::vector& results) const { + if (index < 0) { + return; + } if (is_re2()) { int max_matches = 1 + re2_regex->NumberOfCapturingGroups(); - if (max_matches < 2) { - return; // No capturing groups + if (index >= max_matches) { + return; } size_t pos = 0; @@ -159,9 +163,9 @@ struct RegexpExtractEngine { pos += 1; continue; } - // Extract first capturing group - if (matches.size() > 1 && !matches[1].empty()) { - results.emplace_back(matches[1].data(), matches[1].size()); + // Extract the capturing group with the given index + if (static_cast(index) < matches.size() && !matches[index].empty()) { + results.emplace_back(matches[index].data(), matches[index].size()); } // Move position forward auto offset = std::string(str_pos, str_size) @@ -174,8 +178,8 @@ struct RegexpExtractEngine { boost::match_results matches; while (boost::regex_search(search_start, search_end, matches, *boost_regex)) { - if (matches.size() > 1 && matches[1].matched) { - results.emplace_back(matches[1].str()); + if (static_cast(index) < matches.size() && matches[index].matched) { + results.emplace_back(matches[index].str()); } if (matches[0].length() == 0) { if (search_start == search_end) { @@ -716,16 +720,16 @@ struct RegexpExtractAllArrayOutput { template struct RegexpExtractAllImpl { static constexpr auto name = Handler::func_name; - static constexpr size_t num_args = 2; + static constexpr size_t num_args = 3; static constexpr size_t PATTERN_ARG_IDX = 1; static DataTypePtr return_type() { return Handler::return_type(); } static Status execute(FunctionContext* context, Block& block, const ColumnNumbers& arguments, uint32_t result, size_t input_rows_count) { - bool col_const[2]; - ColumnPtr argument_columns[2]; - for (int i = 0; i < 2; ++i) { + bool col_const[3]; + ColumnPtr argument_columns[3]; + for (int i = 0; i < 3; ++i) { col_const[i] = is_column_const(*block.get_by_position(arguments[i]).column); } argument_columns[0] = col_const[0] ? static_cast( @@ -733,10 +737,11 @@ struct RegexpExtractAllImpl { .convert_to_full_column() : block.get_by_position(arguments[0]).column; - default_preprocess_parameter_columns(argument_columns, col_const, {1}, block, arguments); + default_preprocess_parameter_columns(argument_columns, col_const, {1, 2}, block, arguments); const auto* str_col = check_and_get_column(argument_columns[0].get()); const auto* pattern_col = check_and_get_column(argument_columns[1].get()); + const auto* index_col = check_and_get_column(argument_columns[2].get()); auto outer_null_map = ColumnUInt8::create(input_rows_count, 0); auto& null_map_data = outer_null_map->get_data(); @@ -751,11 +756,13 @@ struct RegexpExtractAllImpl { handler.push_null(i, null_map_data); continue; } + const auto index_data = index_col->get_int(index_check_const(i, is_const)); regexp_extract_all_inner_loop(context, str_col, pattern_col, - handler, null_map_data, i); + index_data, handler, null_map_data, + i); } }, - make_bool_variant(col_const[1])); + make_bool_variant(col_const[1] && col_const[2])); block.get_by_position(result).column = state.finalize(std::move(outer_null_map)); return Status::OK(); @@ -764,8 +771,14 @@ struct RegexpExtractAllImpl { private: template static void regexp_extract_all_inner_loop(FunctionContext* context, const ColumnString* str_col, - const ColumnString* pattern_col, Handler& handler, + const ColumnString* pattern_col, + const Int64 index_data, Handler& handler, NullMap& null_map, const size_t index_now) { + if (index_data < 0) { + handler.push_null(index_now, null_map); + return; + } + auto* engine = reinterpret_cast( context->get_function_state(FunctionContext::THREAD_LOCAL)); std::unique_ptr scoped_engine; @@ -784,13 +797,16 @@ struct RegexpExtractAllImpl { engine = scoped_engine.get(); } - if (engine->number_of_capturing_groups() == 0) { + // index 0 extracts the whole match, so patterns without capturing groups are + // only rejected when a positive group index is requested. + if (index_data > engine->number_of_capturing_groups()) { handler.push_empty(index_now); return; } const auto& str = str_col->get_data_at(index_now); std::vector res_matches; - engine->match_all_and_extract(str.data, str.size, res_matches); + engine->match_all_and_extract(str.data, str.size, static_cast(index_data), + res_matches); if (res_matches.empty()) { handler.push_empty(index_now); diff --git a/be/test/exprs/function/function_like_test.cpp b/be/test/exprs/function/function_like_test.cpp index 82618a790e99e7..16033cc2bf6ee1 100644 --- a/be/test/exprs/function/function_like_test.cpp +++ b/be/test/exprs/function/function_like_test.cpp @@ -221,27 +221,53 @@ TEST(FunctionLikeTest, regexp_extract_all) { std::string func_name = "regexp_extract_all"; DataSet data_set = { - {{std::string("x=a3&x=18abc&x=2&y=3&x=4&x=17bcd"), std::string("x=([0-9]+)([a-z]+)")}, + {{std::string("x=a3&x=18abc&x=2&y=3&x=4&x=17bcd"), std::string("x=([0-9]+)([a-z]+)"), + (int64_t)1}, std::string("['18','17']")}, - {{std::string("x=a3&x=18abc&x=2&y=3&x=4"), std::string("^x=([a-z]+)([0-9]+)")}, + {{std::string("x=a3&x=18abc&x=2&y=3&x=4"), std::string("^x=([a-z]+)([0-9]+)"), (int64_t)1}, std::string("['a']")}, - {{std::string("http://a.m.baidu.com/i41915173660.htm"), std::string("i([0-9]+)")}, + {{std::string("http://a.m.baidu.com/i41915173660.htm"), std::string("i([0-9]+)"), + (int64_t)1}, std::string("['41915173660']")}, - {{std::string("http://a.m.baidu.com/i41915i73660.htm"), std::string("i([0-9]+)")}, + {{std::string("http://a.m.baidu.com/i41915i73660.htm"), std::string("i([0-9]+)"), + (int64_t)1}, std::string("['41915','73660']")}, - {{std::string("hitdecisiondlist"), std::string("(i)(.*?)(e)")}, std::string("['i']")}, - {{std::string("hitdecisioendlist"), std::string("(i)(.*?)(e)")}, + {{std::string("hitdecisiondlist"), std::string("(i)(.*?)(e)"), (int64_t)1}, + std::string("['i']")}, + {{std::string("hitdecisioendlist"), std::string("(i)(.*?)(e)"), (int64_t)1}, std::string("['i','i']")}, - {{std::string("hitdecisioendliset"), std::string("(i)(.*?)(e)")}, + {{std::string("hitdecisioendliset"), std::string("(i)(.*?)(e)"), (int64_t)1}, std::string("['i','i','i']")}, + + // group index 0 extracts the whole match + {{std::string("x=a3&x=18abc&x=2&y=3&x=4&x=17bcd"), std::string("x=([0-9]+)([a-z]+)"), + (int64_t)0}, + std::string("['x=18abc','x=17bcd']")}, + {{std::string("hitdecisiondlist"), std::string("(i)(.*?)(e)"), (int64_t)0}, + std::string("['itde']")}, + // other capturing groups + {{std::string("x=a3&x=18abc&x=2&y=3&x=4&x=17bcd"), std::string("x=([0-9]+)([a-z]+)"), + (int64_t)2}, + std::string("['abc','bcd']")}, + {{std::string("hitdecisiondlist"), std::string("(i)(.*?)(e)"), (int64_t)2}, + std::string("['td']")}, + // group index out of range returns an empty result + {{std::string("hitdecisiondlist"), std::string("(i)(.*?)(e)"), (int64_t)3}, + std::string("")}, + // group index 0 also works for patterns without capturing groups + {{std::string("ab1cd22"), std::string("[0-9]+"), (int64_t)0}, + std::string("['1','22']")}, + // negative group index returns null + {{std::string("hitdecisiondlist"), std::string("(i)(.*?)(e)"), (int64_t)-1}, Null()}, // null - {{std::string("abc"), Null()}, Null()}, - {{Null(), std::string("i([0-9]+)")}, Null()}}; + {{std::string("abc"), Null(), (int64_t)1}, Null()}, + {{Null(), std::string("i([0-9]+)"), (int64_t)1}, Null()}}; // pattern is constant value InputTypeSet const_pattern_input_types = {PrimitiveType::TYPE_VARCHAR, - Consted {PrimitiveType::TYPE_VARCHAR}}; + Consted {PrimitiveType::TYPE_VARCHAR}, + PrimitiveType::TYPE_BIGINT}; for (const auto& line : data_set) { DataSet const_pattern_dataset = {line}; static_cast(check_function(func_name, const_pattern_input_types, @@ -255,34 +281,40 @@ TEST(FunctionLikeTest, regexp_extract_all_array) { auto return_type = make_nullable( std::make_shared(make_nullable(std::make_shared()))); - auto run_case = [&](const std::string& str, const std::string& pattern, + auto bigint_type = std::make_shared(); + auto run_case = [&](const std::string& str, const std::string& pattern, int64_t idx, const std::string& expected, bool expect_null = false) { auto col_str = ColumnString::create(); col_str->insert_data(str.data(), str.size()); auto col_pattern = ColumnString::create(); col_pattern->insert_data(pattern.data(), pattern.size()); + auto col_idx = ColumnInt64::create(); + col_idx->insert_value(idx); Block block; block.insert({std::move(col_str), str_type, "str"}); block.insert({ColumnConst::create(std::move(col_pattern), 1), str_type, "pattern"}); + block.insert({ColumnConst::create(std::move(col_idx), 1), bigint_type, "idx"}); block.insert({nullptr, return_type, "result"}); - ColumnsWithTypeAndName arg_cols = {block.get_by_position(0), block.get_by_position(1)}; + ColumnsWithTypeAndName arg_cols = {block.get_by_position(0), block.get_by_position(1), + block.get_by_position(2)}; auto func = SimpleFunctionFactory::instance().get_function(func_name, arg_cols, return_type); ASSERT_TRUE(func != nullptr); - std::vector arg_types = {str_type, str_type}; + std::vector arg_types = {str_type, str_type, bigint_type}; FunctionUtils fn_utils({}, arg_types, false); auto* fn_ctx = fn_utils.get_fn_ctx(); fn_ctx->set_constant_cols( - {nullptr, std::make_shared(block.get_by_position(1).column)}); + {nullptr, std::make_shared(block.get_by_position(1).column), + std::make_shared(block.get_by_position(2).column)}); ASSERT_EQ(Status::OK(), func->open(fn_ctx, FunctionContext::FRAGMENT_LOCAL)); ASSERT_EQ(Status::OK(), func->open(fn_ctx, FunctionContext::THREAD_LOCAL)); - ASSERT_EQ(Status::OK(), func->execute(fn_ctx, block, {0, 1}, 2, 1)); + ASSERT_EQ(Status::OK(), func->execute(fn_ctx, block, {0, 1, 2}, 3, 1)); - auto result_col = block.get_by_position(2).column; + auto result_col = block.get_by_position(3).column; ASSERT_TRUE(result_col.get() != nullptr); if (expect_null) { EXPECT_TRUE(result_col->is_null_at(0)); @@ -290,20 +322,29 @@ TEST(FunctionLikeTest, regexp_extract_all_array) { ASSERT_FALSE(result_col->is_null_at(0)); auto result_str = return_type->to_string(*result_col, 0); EXPECT_EQ(expected, result_str) - << "input: '" << str << "', pattern: '" << pattern << "'"; + << "input: '" << str << "', pattern: '" << pattern << "', idx: " << idx; } static_cast(func->close(fn_ctx, FunctionContext::THREAD_LOCAL)); static_cast(func->close(fn_ctx, FunctionContext::FRAGMENT_LOCAL)); }; - run_case("x=a3&x=18abc&x=2&y=3&x=4&x=17bcd", "x=([0-9]+)([a-z]+)", "[\"18\", \"17\"]"); - run_case("x=a3&x=18abc&x=2&y=3&x=4", "^x=([a-z]+)([0-9]+)", "[\"a\"]"); - run_case("http://a.m.baidu.com/i41915173660.htm", "i([0-9]+)", "[\"41915173660\"]"); - run_case("http://a.m.baidu.com/i41915i73660.htm", "i([0-9]+)", "[\"41915\", \"73660\"]"); - run_case("hitdecisiondlist", "(i)(.*?)(e)", "[\"i\"]"); - run_case("no_match_here", "x=([0-9]+)", "[]"); - run_case("abc", "([a-z]+)", "[\"abc\"]"); + run_case("x=a3&x=18abc&x=2&y=3&x=4&x=17bcd", "x=([0-9]+)([a-z]+)", 1, "[\"18\", \"17\"]"); + run_case("x=a3&x=18abc&x=2&y=3&x=4", "^x=([a-z]+)([0-9]+)", 1, "[\"a\"]"); + run_case("http://a.m.baidu.com/i41915173660.htm", "i([0-9]+)", 1, "[\"41915173660\"]"); + run_case("http://a.m.baidu.com/i41915i73660.htm", "i([0-9]+)", 1, "[\"41915\", \"73660\"]"); + run_case("hitdecisiondlist", "(i)(.*?)(e)", 1, "[\"i\"]"); + run_case("no_match_here", "x=([0-9]+)", 1, "[]"); + run_case("abc", "([a-z]+)", 1, "[\"abc\"]"); + // group index 0 extracts the whole match + run_case("x=a3&x=18abc&x=2&y=3&x=4&x=17bcd", "x=([0-9]+)([a-z]+)", 0, + "[\"x=18abc\", \"x=17bcd\"]"); + // other capturing groups + run_case("x=a3&x=18abc&x=2&y=3&x=4&x=17bcd", "x=([0-9]+)([a-z]+)", 2, "[\"abc\", \"bcd\"]"); + // group index out of range returns an empty array + run_case("hitdecisiondlist", "(i)(.*?)(e)", 3, "[]"); + // negative group index returns null + run_case("hitdecisiondlist", "(i)(.*?)(e)", -1, "", true); // Helper for testing null input propagation auto nullable_str_type = make_nullable(str_type); @@ -339,24 +380,31 @@ TEST(FunctionLikeTest, regexp_extract_all_array) { Block block; block.insert({col_str, str_col_type, "str"}); block.insert({col_pattern, pattern_col_type, "pattern"}); + auto col_idx = ColumnInt64::create(); + col_idx->insert_value(1); + block.insert({ColumnConst::create(std::move(col_idx), 1), + std::make_shared(), "idx"}); block.insert({nullptr, return_type, "result"}); - ColumnsWithTypeAndName arg_cols = {block.get_by_position(0), block.get_by_position(1)}; + ColumnsWithTypeAndName arg_cols = {block.get_by_position(0), block.get_by_position(1), + block.get_by_position(2)}; auto func = SimpleFunctionFactory::instance().get_function(func_name, arg_cols, return_type); ASSERT_TRUE(func != nullptr); - std::vector arg_types = {str_col_type, pattern_col_type}; + std::vector arg_types = {str_col_type, pattern_col_type, + std::make_shared()}; FunctionUtils fn_utils({}, arg_types, false); auto* fn_ctx = fn_utils.get_fn_ctx(); fn_ctx->set_constant_cols( - {nullptr, std::make_shared(block.get_by_position(1).column)}); + {nullptr, std::make_shared(block.get_by_position(1).column), + std::make_shared(block.get_by_position(2).column)}); ASSERT_EQ(Status::OK(), func->open(fn_ctx, FunctionContext::FRAGMENT_LOCAL)); ASSERT_EQ(Status::OK(), func->open(fn_ctx, FunctionContext::THREAD_LOCAL)); - ASSERT_EQ(Status::OK(), func->execute(fn_ctx, block, {0, 1}, 2, 1)); + ASSERT_EQ(Status::OK(), func->execute(fn_ctx, block, {0, 1, 2}, 3, 1)); - EXPECT_TRUE(block.get_by_position(2).column->is_null_at(0)) + EXPECT_TRUE(block.get_by_position(3).column->is_null_at(0)) << "Expected null for null_str=" << null_str << " null_pattern=" << null_pattern; static_cast(func->close(fn_ctx, FunctionContext::THREAD_LOCAL)); @@ -374,21 +422,26 @@ TEST(FunctionLikeTest, regexp_extract_all_array) { col_str->insert_data("abc", 3); auto col_pattern = ColumnString::create(); col_pattern->insert_data("(", 1); + auto col_idx = ColumnInt64::create(); + col_idx->insert_value(1); Block block; block.insert({std::move(col_str), str_type, "str"}); block.insert({ColumnConst::create(std::move(col_pattern), 1), str_type, "pattern"}); + block.insert({ColumnConst::create(std::move(col_idx), 1), bigint_type, "idx"}); block.insert({nullptr, return_type, "result"}); - ColumnsWithTypeAndName arg_cols = {block.get_by_position(0), block.get_by_position(1)}; + ColumnsWithTypeAndName arg_cols = {block.get_by_position(0), block.get_by_position(1), + block.get_by_position(2)}; auto func = SimpleFunctionFactory::instance().get_function(func_name, arg_cols, return_type); ASSERT_TRUE(func != nullptr); - std::vector arg_types = {str_type, str_type}; + std::vector arg_types = {str_type, str_type, bigint_type}; FunctionUtils fn_utils({}, arg_types, false); auto* fn_ctx = fn_utils.get_fn_ctx(); fn_ctx->set_constant_cols( - {nullptr, std::make_shared(block.get_by_position(1).column)}); + {nullptr, std::make_shared(block.get_by_position(1).column), + std::make_shared(block.get_by_position(2).column)}); ASSERT_EQ(Status::OK(), func->open(fn_ctx, FunctionContext::FRAGMENT_LOCAL)); // Invalid pattern should cause open() to fail for THREAD_LOCAL scope diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java index 9492e34e5f1ab2..ab3d417df173d3 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java @@ -22,8 +22,9 @@ import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable; import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; import org.apache.doris.nereids.trees.expressions.functions.PropagateNullLiteral; -import org.apache.doris.nereids.trees.expressions.shape.BinaryExpression; +import org.apache.doris.nereids.trees.expressions.literal.BigIntLiteral; import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; +import org.apache.doris.nereids.types.BigIntType; import org.apache.doris.nereids.types.StringType; import org.apache.doris.nereids.types.VarcharType; @@ -36,19 +37,31 @@ * ScalarFunction 'regexp_extract_all'. This class is generated by GenerateFunction. */ public class RegexpExtractAll extends ScalarFunction - implements BinaryExpression, ExplicitlyCastableSignature, AlwaysNullable, PropagateNullLiteral { + implements ExplicitlyCastableSignature, AlwaysNullable, PropagateNullLiteral { public static final List SIGNATURES = ImmutableList.of( FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT) .args(VarcharType.SYSTEM_DEFAULT, VarcharType.SYSTEM_DEFAULT), - FunctionSignature.ret(StringType.INSTANCE).args(StringType.INSTANCE, StringType.INSTANCE) + FunctionSignature.ret(StringType.INSTANCE).args(StringType.INSTANCE, StringType.INSTANCE), + FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT) + .args(VarcharType.SYSTEM_DEFAULT, VarcharType.SYSTEM_DEFAULT, BigIntType.INSTANCE), + FunctionSignature.ret(StringType.INSTANCE) + .args(StringType.INSTANCE, StringType.INSTANCE, BigIntType.INSTANCE) ); /** * constructor with 2 arguments. + * The optional group index follows Spark semantics and defaults to 1 (the first capturing group). */ public RegexpExtractAll(Expression arg0, Expression arg1) { - super("regexp_extract_all", arg0, arg1); + super("regexp_extract_all", arg0, arg1, new BigIntLiteral(1)); + } + + /** + * constructor with 3 arguments. + */ + public RegexpExtractAll(Expression arg0, Expression arg1, Expression arg2) { + super("regexp_extract_all", arg0, arg1, arg2); } /** constructor for withChildren and reuse signature */ @@ -61,7 +74,7 @@ private RegexpExtractAll(ScalarFunctionParams functionParams) { */ @Override public RegexpExtractAll withChildren(List children) { - Preconditions.checkArgument(children.size() == 2); + Preconditions.checkArgument(children.size() == 2 || children.size() == 3); return new RegexpExtractAll(getFunctionParams(children)); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAllArray.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAllArray.java index 58f395bb71652f..773a684499cb18 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAllArray.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAllArray.java @@ -22,9 +22,10 @@ import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable; import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; import org.apache.doris.nereids.trees.expressions.functions.PropagateNullLiteral; -import org.apache.doris.nereids.trees.expressions.shape.BinaryExpression; +import org.apache.doris.nereids.trees.expressions.literal.BigIntLiteral; import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; import org.apache.doris.nereids.types.ArrayType; +import org.apache.doris.nereids.types.BigIntType; import org.apache.doris.nereids.types.StringType; import org.apache.doris.nereids.types.VarcharType; @@ -38,20 +39,32 @@ * Returns all matches of a regex pattern as an Array<String> instead of a string-formatted array. */ public class RegexpExtractAllArray extends ScalarFunction - implements BinaryExpression, ExplicitlyCastableSignature, AlwaysNullable, PropagateNullLiteral { + implements ExplicitlyCastableSignature, AlwaysNullable, PropagateNullLiteral { public static final List SIGNATURES = ImmutableList.of( FunctionSignature.ret(ArrayType.of(VarcharType.SYSTEM_DEFAULT)) .args(VarcharType.SYSTEM_DEFAULT, VarcharType.SYSTEM_DEFAULT), FunctionSignature.ret(ArrayType.of(StringType.INSTANCE)) - .args(StringType.INSTANCE, StringType.INSTANCE) + .args(StringType.INSTANCE, StringType.INSTANCE), + FunctionSignature.ret(ArrayType.of(VarcharType.SYSTEM_DEFAULT)) + .args(VarcharType.SYSTEM_DEFAULT, VarcharType.SYSTEM_DEFAULT, BigIntType.INSTANCE), + FunctionSignature.ret(ArrayType.of(StringType.INSTANCE)) + .args(StringType.INSTANCE, StringType.INSTANCE, BigIntType.INSTANCE) ); /** * constructor with 2 arguments. + * The optional group index follows Spark semantics and defaults to 1 (the first capturing group). */ public RegexpExtractAllArray(Expression arg0, Expression arg1) { - super("regexp_extract_all_array", arg0, arg1); + super("regexp_extract_all_array", arg0, arg1, new BigIntLiteral(1)); + } + + /** + * constructor with 3 arguments. + */ + public RegexpExtractAllArray(Expression arg0, Expression arg1, Expression arg2) { + super("regexp_extract_all_array", arg0, arg1, arg2); } /** constructor for withChildren and reuse signature */ @@ -64,7 +77,7 @@ private RegexpExtractAllArray(ScalarFunctionParams functionParams) { */ @Override public RegexpExtractAllArray withChildren(List children) { - Preconditions.checkArgument(children.size() == 2); + Preconditions.checkArgument(children.size() == 2 || children.size() == 3); return new RegexpExtractAllArray(getFunctionParams(children)); } diff --git a/regression-test/data/query_p0/sql_functions/string_functions/test_string_function_regexp.out b/regression-test/data/query_p0/sql_functions/string_functions/test_string_function_regexp.out index 3a2754b72e1390..a56484f9b8711d 100644 --- a/regression-test/data/query_p0/sql_functions/string_functions/test_string_function_regexp.out +++ b/regression-test/data/query_p0/sql_functions/string_functions/test_string_function_regexp.out @@ -511,3 +511,21 @@ Ben -- !sql_field5 -- 2 +-- !sql_regexp_extract_all_group0 -- +['x=18abc','x=17bcd'] + +-- !sql_regexp_extract_all_group2 -- +['abc','bcd'] + +-- !sql_regexp_extract_all_group_negative -- +\N + +-- !sql_regexp_extract_all_array_group0 -- +["x=18abc", "x=17bcd"] + +-- !sql_regexp_extract_all_array_group2 -- +["abc", "bcd"] + +-- !sql_regexp_extract_all_array_group_out_of_range -- +[] + diff --git a/regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy b/regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy index 09a5a2f7e58014..18406aee0cff96 100644 --- a/regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy +++ b/regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy @@ -255,4 +255,11 @@ suite("test_string_function_regexp") { qt_sql_field4 "SELECT FIELD('21','2130', '2131', '21');" qt_sql_field5 "SELECT FIELD(21, 2130, 21, 2131);" + + qt_sql_regexp_extract_all_group0 "select regexp_extract_all('x=a3&x=18abc&x=2&y=3&x=4&x=17bcd', 'x=([0-9]+)([a-z]+)', 0);" + qt_sql_regexp_extract_all_group2 "select regexp_extract_all('x=a3&x=18abc&x=2&y=3&x=4&x=17bcd', 'x=([0-9]+)([a-z]+)', 2);" + qt_sql_regexp_extract_all_group_negative "select regexp_extract_all('abc', '(b)', -1);" + qt_sql_regexp_extract_all_array_group0 "select regexp_extract_all_array('x=a3&x=18abc&x=2&y=3&x=4&x=17bcd', 'x=([0-9]+)([a-z]+)', 0);" + qt_sql_regexp_extract_all_array_group2 "select regexp_extract_all_array('x=a3&x=18abc&x=2&y=3&x=4&x=17bcd', 'x=([0-9]+)([a-z]+)', 2);" + qt_sql_regexp_extract_all_array_group_out_of_range "select regexp_extract_all_array('hitdecisiondlist', '(i)(.*?)(e)', 3);" } From aa701aa5c48745e6ee45a99b9736dfd1c70fa12c Mon Sep 17 00:00:00 2001 From: ccl125 <1253846621@qq.com> Date: Mon, 27 Jul 2026 14:04:46 +0800 Subject: [PATCH 2/5] [Fix](func) Address review: withChildren should require 3 children The 2-arg constructor always pads the default group index, so every regexp_extract_all / regexp_extract_all_array node physically has 3 children. Tighten the withChildren assertion accordingly. --- .../trees/expressions/functions/scalar/RegexpExtractAll.java | 2 +- .../expressions/functions/scalar/RegexpExtractAllArray.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java index ab3d417df173d3..994f6206cd0d7d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java @@ -74,7 +74,7 @@ private RegexpExtractAll(ScalarFunctionParams functionParams) { */ @Override public RegexpExtractAll withChildren(List children) { - Preconditions.checkArgument(children.size() == 2 || children.size() == 3); + Preconditions.checkArgument(children.size() == 3); return new RegexpExtractAll(getFunctionParams(children)); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAllArray.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAllArray.java index 773a684499cb18..9a2d8d5e48c670 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAllArray.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAllArray.java @@ -77,7 +77,7 @@ private RegexpExtractAllArray(ScalarFunctionParams functionParams) { */ @Override public RegexpExtractAllArray withChildren(List children) { - Preconditions.checkArgument(children.size() == 2 || children.size() == 3); + Preconditions.checkArgument(children.size() == 3); return new RegexpExtractAllArray(getFunctionParams(children)); } From d387667321b1ef9852fc3eefc3a89c04ba7c9778 Mon Sep 17 00:00:00 2001 From: ccl125 <1253846621@qq.com> Date: Thu, 30 Jul 2026 15:03:41 +0800 Subject: [PATCH 3/5] [Fix](func) Address review: variadic arity, Spark-consistent group index semantics - Support both 2-arg and 3-arg forms at the BE (separate variadic registrations) instead of FE-side default padding, so old FE/new BE and new FE/old BE combinations keep working during rolling upgrades. - Align the group index contract with Spark: an index outside [0, number_of_capturing_groups] is an error instead of NULL/empty. - Keep empty strings for non-participating groups instead of skipping them, consistent with Spark. - Fix test group counting ((i)(.*?)(e) has three groups, index 3 is valid) and expand regression coverage: column input, NULL literals, illegal indexes, empty groups. --- be/src/exprs/function/function_regexp.cpp | 139 +++++++++++++----- be/test/exprs/function/function_like_test.cpp | 84 ++++++++++- .../functions/scalar/RegexpExtractAll.java | 9 +- .../scalar/RegexpExtractAllArray.java | 9 +- .../test_string_function_regexp.out | 44 +++++- .../test_string_function_regexp.groovy | 32 +++- 6 files changed, 258 insertions(+), 59 deletions(-) diff --git a/be/src/exprs/function/function_regexp.cpp b/be/src/exprs/function/function_regexp.cpp index f21fe5537ac0c7..603f9d75c8cc60 100644 --- a/be/src/exprs/function/function_regexp.cpp +++ b/be/src/exprs/function/function_regexp.cpp @@ -138,6 +138,8 @@ struct RegexpExtractEngine { // Match all occurrences and extract the capturing group with the given index. // index 0 means the whole match, 1 means the first capturing group (the default), and so on. + // Groups that did not participate in a match (or matched an empty string) contribute an + // empty string to the result, consistent with Spark. void match_all_and_extract(const char* data, size_t size, int index, std::vector& results) const { if (index < 0) { @@ -164,8 +166,13 @@ struct RegexpExtractEngine { continue; } // Extract the capturing group with the given index - if (static_cast(index) < matches.size() && !matches[index].empty()) { - results.emplace_back(matches[index].data(), matches[index].size()); + if (static_cast(index) < matches.size()) { + const re2::StringPiece& group = matches[index]; + if (group.data() != nullptr) { + results.emplace_back(group.data(), group.size()); + } else { + results.emplace_back(); + } } // Move position forward auto offset = std::string(str_pos, str_size) @@ -178,7 +185,7 @@ struct RegexpExtractEngine { boost::match_results matches; while (boost::regex_search(search_start, search_end, matches, *boost_regex)) { - if (static_cast(index) < matches.size() && matches[index].matched) { + if (static_cast(index) < matches.size()) { results.emplace_back(matches[index].str()); } if (matches[0].length() == 0) { @@ -716,20 +723,40 @@ struct RegexpExtractAllArrayOutput { }; }; +// Two-parameter form of regexp_extract_all/regexp_extract_all_array, kept so that an +// old FE (which knows nothing about the group index) can run on a new BE during +// rolling upgrades. +struct RegexpExtractAllTwoParams { + static DataTypes get_variadic_argument_types() { + return {std::make_shared(), std::make_shared()}; + } +}; + +// Three-parameter form with the explicit group index. +struct RegexpExtractAllThreeParams { + static DataTypes get_variadic_argument_types() { + return {std::make_shared(), std::make_shared(), + std::make_shared()}; + } +}; + // Handler controls return type & column layout -template +template struct RegexpExtractAllImpl { static constexpr auto name = Handler::func_name; - static constexpr size_t num_args = 3; static constexpr size_t PATTERN_ARG_IDX = 1; + static DataTypes variadic_argument_types() { return ParamTypes::get_variadic_argument_types(); } + static DataTypePtr return_type() { return Handler::return_type(); } static Status execute(FunctionContext* context, Block& block, const ColumnNumbers& arguments, uint32_t result, size_t input_rows_count) { - bool col_const[3]; + const bool has_index = arguments.size() == 3; + + bool col_const[3] = {false, false, false}; ColumnPtr argument_columns[3]; - for (int i = 0; i < 3; ++i) { + for (size_t i = 0; i < arguments.size(); ++i) { col_const[i] = is_column_const(*block.get_by_position(arguments[i]).column); } argument_columns[0] = col_const[0] ? static_cast( @@ -737,11 +764,19 @@ struct RegexpExtractAllImpl { .convert_to_full_column() : block.get_by_position(arguments[0]).column; - default_preprocess_parameter_columns(argument_columns, col_const, {1, 2}, block, arguments); + if (has_index) { + default_preprocess_parameter_columns(argument_columns, col_const, {1, 2}, block, + arguments); + } else { + default_preprocess_parameter_columns(argument_columns, col_const, {1}, block, + arguments); + } const auto* str_col = check_and_get_column(argument_columns[0].get()); const auto* pattern_col = check_and_get_column(argument_columns[1].get()); - const auto* index_col = check_and_get_column(argument_columns[2].get()); + const auto* index_col = has_index + ? check_and_get_column(argument_columns[2].get()) + : nullptr; auto outer_null_map = ColumnUInt8::create(input_rows_count, 0); auto& null_map_data = outer_null_map->get_data(); @@ -749,6 +784,7 @@ struct RegexpExtractAllImpl { typename Handler::State state(input_rows_count); auto handler = state.create_handler(); + Status status = Status::OK(); std::visit( [&](auto is_const) { for (size_t i = 0; i < input_rows_count; ++i) { @@ -756,29 +792,32 @@ struct RegexpExtractAllImpl { handler.push_null(i, null_map_data); continue; } - const auto index_data = index_col->get_int(index_check_const(i, is_const)); - regexp_extract_all_inner_loop(context, str_col, pattern_col, - index_data, handler, null_map_data, - i); + const Int64 index_data = + has_index ? index_col->get_int(index_check_const(i, is_const)) : 1; + status = regexp_extract_all_inner_loop( + context, str_col, pattern_col, index_data, handler, null_map_data, + i); + if (!status.ok()) { + return; + } } }, - make_bool_variant(col_const[1] && col_const[2])); + make_bool_variant(col_const[1] && (!has_index || col_const[2]))); + if (!status.ok()) { + return status; + } block.get_by_position(result).column = state.finalize(std::move(outer_null_map)); return Status::OK(); } private: template - static void regexp_extract_all_inner_loop(FunctionContext* context, const ColumnString* str_col, - const ColumnString* pattern_col, - const Int64 index_data, Handler& handler, - NullMap& null_map, const size_t index_now) { - if (index_data < 0) { - handler.push_null(index_now, null_map); - return; - } - + static Status regexp_extract_all_inner_loop(FunctionContext* context, + const ColumnString* str_col, + const ColumnString* pattern_col, + const Int64 index_data, Handler& handler, + NullMap& null_map, const size_t index_now) { auto* engine = reinterpret_cast( context->get_function_state(FunctionContext::THREAD_LOCAL)); std::unique_ptr scoped_engine; @@ -792,17 +831,21 @@ struct RegexpExtractAllImpl { if (!st) { context->add_warning(error_str.c_str()); handler.push_null(index_now, null_map); - return; + return Status::OK(); } engine = scoped_engine.get(); } - // index 0 extracts the whole match, so patterns without capturing groups are - // only rejected when a positive group index is requested. - if (index_data > engine->number_of_capturing_groups()) { - handler.push_empty(index_now); - return; + // Same contract as Spark: the group index must be within + // [0, number_of_capturing_groups], anything else is an error. + const int num_groups = engine->number_of_capturing_groups(); + if (index_data < 0 || index_data > num_groups) { + return Status::InvalidArgument( + "The value of parameter(s) `idx` in `{}` is invalid: Expects group index " + "between 0 and {}, but got {}.", + name, num_groups, index_data); } + const auto& str = str_col->get_data_at(index_now); std::vector res_matches; engine->match_all_and_extract(str.data, str.size, static_cast(index_data), @@ -810,15 +853,21 @@ struct RegexpExtractAllImpl { if (res_matches.empty()) { handler.push_empty(index_now); - return; + return Status::OK(); } handler.push_matches(index_now, res_matches); + return Status::OK(); } }; // template FunctionRegexpFunctionality is used for regexp_xxxx series functions, not for regexp match. template class FunctionRegexpFunctionality : public IFunction { + // Impls that also expose variadic_argument_types() (e.g. RegexpExtractAllImpl) are + // registered once per supported arity with the same function name, mirroring + // FunctionRegexpReplace's ThreeParamTypes/FourParamTypes pattern. + static constexpr bool kVariadic = requires { Impl::variadic_argument_types(); }; + public: static constexpr auto name = Impl::name; @@ -826,7 +875,23 @@ class FunctionRegexpFunctionality : public IFunction { String get_name() const override { return name; } - size_t get_number_of_arguments() const override { return Impl::num_args; } + size_t get_number_of_arguments() const override { + if constexpr (kVariadic) { + return Impl::variadic_argument_types().size(); + } else { + return Impl::num_args; + } + } + + bool is_variadic() const override { return kVariadic; } + + DataTypes get_variadic_argument_types_impl() const override { + if constexpr (kVariadic) { + return Impl::variadic_argument_types(); + } else { + return {}; + } + } DataTypePtr get_return_type_impl(const DataTypes& arguments) const override { return Impl::return_type(); @@ -871,10 +936,14 @@ void register_function_regexp_extract(SimpleFunctionFactory& factory) { factory.register_function, FourParamTypes>>(); factory.register_function>>(); factory.register_function>>(); - factory.register_function< - FunctionRegexpFunctionality>>(); - factory.register_function< - FunctionRegexpFunctionality>>(); + factory.register_function>>(); + factory.register_function>>(); + factory.register_function>>(); + factory.register_function>>(); factory.register_function(); } diff --git a/be/test/exprs/function/function_like_test.cpp b/be/test/exprs/function/function_like_test.cpp index 16033cc2bf6ee1..800470b41036fa 100644 --- a/be/test/exprs/function/function_like_test.cpp +++ b/be/test/exprs/function/function_like_test.cpp @@ -252,14 +252,13 @@ TEST(FunctionLikeTest, regexp_extract_all) { std::string("['abc','bcd']")}, {{std::string("hitdecisiondlist"), std::string("(i)(.*?)(e)"), (int64_t)2}, std::string("['td']")}, - // group index out of range returns an empty result {{std::string("hitdecisiondlist"), std::string("(i)(.*?)(e)"), (int64_t)3}, - std::string("")}, + std::string("['e']")}, + // non-participating groups contribute empty strings, consistent with Spark + {{std::string("a b"), std::string("(a)|(b)"), (int64_t)2}, std::string("['','b']")}, // group index 0 also works for patterns without capturing groups {{std::string("ab1cd22"), std::string("[0-9]+"), (int64_t)0}, std::string("['1','22']")}, - // negative group index returns null - {{std::string("hitdecisiondlist"), std::string("(i)(.*?)(e)"), (int64_t)-1}, Null()}, // null {{std::string("abc"), Null(), (int64_t)1}, Null()}, {{Null(), std::string("i([0-9]+)"), (int64_t)1}, Null()}}; @@ -273,6 +272,21 @@ TEST(FunctionLikeTest, regexp_extract_all) { static_cast(check_function(func_name, const_pattern_input_types, const_pattern_dataset)); } + + // the two-argument form must keep working (old FE on new BE during rolling upgrades) + DataSet two_arg_data_set = { + {{std::string("x=a3&x=18abc&x=2&y=3&x=4&x=17bcd"), std::string("x=([0-9]+)([a-z]+)")}, + std::string("['18','17']")}, + {{std::string("hitdecisiondlist"), std::string("(i)(.*?)(e)")}, std::string("['i']")}, + {{std::string("abc"), Null()}, Null()}, + {{Null(), std::string("i([0-9]+)")}, Null()}}; + InputTypeSet two_arg_input_types = {PrimitiveType::TYPE_VARCHAR, + Consted {PrimitiveType::TYPE_VARCHAR}}; + for (const auto& line : two_arg_data_set) { + DataSet two_arg_dataset = {line}; + static_cast(check_function(func_name, two_arg_input_types, + two_arg_dataset)); + } } TEST(FunctionLikeTest, regexp_extract_all_array) { @@ -341,10 +355,9 @@ TEST(FunctionLikeTest, regexp_extract_all_array) { "[\"x=18abc\", \"x=17bcd\"]"); // other capturing groups run_case("x=a3&x=18abc&x=2&y=3&x=4&x=17bcd", "x=([0-9]+)([a-z]+)", 2, "[\"abc\", \"bcd\"]"); - // group index out of range returns an empty array - run_case("hitdecisiondlist", "(i)(.*?)(e)", 3, "[]"); - // negative group index returns null - run_case("hitdecisiondlist", "(i)(.*?)(e)", -1, "", true); + run_case("hitdecisiondlist", "(i)(.*?)(e)", 3, "[\"e\"]"); + // non-participating groups contribute empty strings, consistent with Spark + run_case("a b", "(a)|(b)", 2, "[\"\", \"b\"]"); // Helper for testing null input propagation auto nullable_str_type = make_nullable(str_type); @@ -452,6 +465,61 @@ TEST(FunctionLikeTest, regexp_extract_all_array) { } } +TEST(FunctionLikeTest, regexp_extract_all_invalid_group_index) { + // Same contract as Spark: a group index outside [0, number_of_capturing_groups] + // is an error, for both regexp_extract_all and regexp_extract_all_array. + auto str_type = std::make_shared(); + auto bigint_type = std::make_shared(); + + auto run_error_case = [&](const std::string& func_name, DataTypePtr return_type, int64_t idx) { + auto col_str = ColumnString::create(); + col_str->insert_data("hitdecisiondlist", 16); + auto col_pattern = ColumnString::create(); + col_pattern->insert_data("(i)(.*?)(e)", 10); + auto col_idx = ColumnInt64::create(); + col_idx->insert_value(idx); + + Block block; + block.insert({std::move(col_str), str_type, "str"}); + block.insert({ColumnConst::create(std::move(col_pattern), 1), str_type, "pattern"}); + block.insert({ColumnConst::create(std::move(col_idx), 1), bigint_type, "idx"}); + block.insert({nullptr, return_type, "result"}); + + ColumnsWithTypeAndName arg_cols = {block.get_by_position(0), block.get_by_position(1), + block.get_by_position(2)}; + auto func = + SimpleFunctionFactory::instance().get_function(func_name, arg_cols, return_type); + ASSERT_TRUE(func != nullptr); + + std::vector arg_types = {str_type, str_type, bigint_type}; + FunctionUtils fn_utils({}, arg_types, false); + auto* fn_ctx = fn_utils.get_fn_ctx(); + fn_ctx->set_constant_cols( + {nullptr, std::make_shared(block.get_by_position(1).column), + std::make_shared(block.get_by_position(2).column)}); + + ASSERT_EQ(Status::OK(), func->open(fn_ctx, FunctionContext::FRAGMENT_LOCAL)); + ASSERT_EQ(Status::OK(), func->open(fn_ctx, FunctionContext::THREAD_LOCAL)); + auto status = func->execute(fn_ctx, block, {0, 1, 2}, 3, 1); + EXPECT_FALSE(status.ok()) << "func: " << func_name << ", idx: " << idx; + EXPECT_NE(status.to_string().find("invalid"), std::string::npos); + + static_cast(func->close(fn_ctx, FunctionContext::THREAD_LOCAL)); + static_cast(func->close(fn_ctx, FunctionContext::FRAGMENT_LOCAL)); + }; + + auto string_return_type = make_nullable(std::make_shared()); + auto array_return_type = make_nullable( + std::make_shared(make_nullable(std::make_shared()))); + + // negative group index + run_error_case("regexp_extract_all", string_return_type, -1); + run_error_case("regexp_extract_all_array", array_return_type, -1); + // group index beyond the number of capturing groups + run_error_case("regexp_extract_all", string_return_type, 4); + run_error_case("regexp_extract_all_array", array_return_type, 4); +} + TEST(FunctionLikeTest, regexp_replace) { std::string func_name = "regexp_replace"; diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java index 994f6206cd0d7d..6f8904e3c5fa08 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java @@ -22,7 +22,6 @@ import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable; import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; import org.apache.doris.nereids.trees.expressions.functions.PropagateNullLiteral; -import org.apache.doris.nereids.trees.expressions.literal.BigIntLiteral; import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; import org.apache.doris.nereids.types.BigIntType; import org.apache.doris.nereids.types.StringType; @@ -50,11 +49,11 @@ public class RegexpExtractAll extends ScalarFunction ); /** - * constructor with 2 arguments. - * The optional group index follows Spark semantics and defaults to 1 (the first capturing group). + * constructor with 2 arguments. The optional group index follows Spark semantics + * and defaults to 1 (the first capturing group) in the BE. */ public RegexpExtractAll(Expression arg0, Expression arg1) { - super("regexp_extract_all", arg0, arg1, new BigIntLiteral(1)); + super("regexp_extract_all", arg0, arg1); } /** @@ -74,7 +73,7 @@ private RegexpExtractAll(ScalarFunctionParams functionParams) { */ @Override public RegexpExtractAll withChildren(List children) { - Preconditions.checkArgument(children.size() == 3); + Preconditions.checkArgument(children.size() == 2 || children.size() == 3); return new RegexpExtractAll(getFunctionParams(children)); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAllArray.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAllArray.java index 9a2d8d5e48c670..e2f58ce83f6c5e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAllArray.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAllArray.java @@ -22,7 +22,6 @@ import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable; import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; import org.apache.doris.nereids.trees.expressions.functions.PropagateNullLiteral; -import org.apache.doris.nereids.trees.expressions.literal.BigIntLiteral; import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; import org.apache.doris.nereids.types.ArrayType; import org.apache.doris.nereids.types.BigIntType; @@ -53,11 +52,11 @@ public class RegexpExtractAllArray extends ScalarFunction ); /** - * constructor with 2 arguments. - * The optional group index follows Spark semantics and defaults to 1 (the first capturing group). + * constructor with 2 arguments. The optional group index follows Spark semantics + * and defaults to 1 (the first capturing group) in the BE. */ public RegexpExtractAllArray(Expression arg0, Expression arg1) { - super("regexp_extract_all_array", arg0, arg1, new BigIntLiteral(1)); + super("regexp_extract_all_array", arg0, arg1); } /** @@ -77,7 +76,7 @@ private RegexpExtractAllArray(ScalarFunctionParams functionParams) { */ @Override public RegexpExtractAllArray withChildren(List children) { - Preconditions.checkArgument(children.size() == 3); + Preconditions.checkArgument(children.size() == 2 || children.size() == 3); return new RegexpExtractAllArray(getFunctionParams(children)); } diff --git a/regression-test/data/query_p0/sql_functions/string_functions/test_string_function_regexp.out b/regression-test/data/query_p0/sql_functions/string_functions/test_string_function_regexp.out index a56484f9b8711d..dad7d8639b76f0 100644 --- a/regression-test/data/query_p0/sql_functions/string_functions/test_string_function_regexp.out +++ b/regression-test/data/query_p0/sql_functions/string_functions/test_string_function_regexp.out @@ -517,8 +517,8 @@ Ben -- !sql_regexp_extract_all_group2 -- ['abc','bcd'] --- !sql_regexp_extract_all_group_negative -- -\N +-- !sql_regexp_extract_all_empty_group -- +['','b'] -- !sql_regexp_extract_all_array_group0 -- ["x=18abc", "x=17bcd"] @@ -526,6 +526,42 @@ Ben -- !sql_regexp_extract_all_array_group2 -- ["abc", "bcd"] --- !sql_regexp_extract_all_array_group_out_of_range -- -[] +-- !sql_regexp_extract_all_array_group3 -- +["e"] + +-- !sql_regexp_extract_all_col_group0 -- +\N + +['lli'] + + +['lli','lli'] +['lli','lli'] + +-- !sql_regexp_extract_all_col_group1 -- +\N + +['ll'] + + +['ll','ll'] +['ll','ll'] + +-- !sql_regexp_extract_all_col_group2 -- +\N + +['i'] + + +['i','i'] +['i','i'] + +-- !sql_regexp_extract_all_null_str -- +\N + +-- !sql_regexp_extract_all_null_pattern -- +\N + +-- !sql_regexp_extract_all_null_idx -- +\N diff --git a/regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy b/regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy index 18406aee0cff96..4979a087cc20a6 100644 --- a/regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy +++ b/regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy @@ -258,8 +258,36 @@ suite("test_string_function_regexp") { qt_sql_regexp_extract_all_group0 "select regexp_extract_all('x=a3&x=18abc&x=2&y=3&x=4&x=17bcd', 'x=([0-9]+)([a-z]+)', 0);" qt_sql_regexp_extract_all_group2 "select regexp_extract_all('x=a3&x=18abc&x=2&y=3&x=4&x=17bcd', 'x=([0-9]+)([a-z]+)', 2);" - qt_sql_regexp_extract_all_group_negative "select regexp_extract_all('abc', '(b)', -1);" + qt_sql_regexp_extract_all_empty_group "select regexp_extract_all('a b', '(a)|(b)', 2);" qt_sql_regexp_extract_all_array_group0 "select regexp_extract_all_array('x=a3&x=18abc&x=2&y=3&x=4&x=17bcd', 'x=([0-9]+)([a-z]+)', 0);" qt_sql_regexp_extract_all_array_group2 "select regexp_extract_all_array('x=a3&x=18abc&x=2&y=3&x=4&x=17bcd', 'x=([0-9]+)([a-z]+)', 2);" - qt_sql_regexp_extract_all_array_group_out_of_range "select regexp_extract_all_array('hitdecisiondlist', '(i)(.*?)(e)', 3);" + qt_sql_regexp_extract_all_array_group3 "select regexp_extract_all_array('hitdecisiondlist', '(i)(.*?)(e)', 3);" + + // column input + qt_sql_regexp_extract_all_col_group0 "SELECT regexp_extract_all(k, '(ll)(i)', 0) from test_string_function_regexp ORDER BY k;" + qt_sql_regexp_extract_all_col_group1 "SELECT regexp_extract_all(k, '(ll)(i)', 1) from test_string_function_regexp ORDER BY k;" + qt_sql_regexp_extract_all_col_group2 "SELECT regexp_extract_all(k, '(ll)(i)', 2) from test_string_function_regexp ORDER BY k;" + + // null literal input + qt_sql_regexp_extract_all_null_str "SELECT regexp_extract_all(null, '(b)', 1);" + qt_sql_regexp_extract_all_null_pattern "SELECT regexp_extract_all('abc', null, 1);" + qt_sql_regexp_extract_all_null_idx "SELECT regexp_extract_all('abc', '(b)', null);" + + // illegal group index: negative or beyond the number of capturing groups + test { + sql "SELECT regexp_extract_all('abc', '(b)', -1);" + exception "invalid" + } + test { + sql "SELECT regexp_extract_all('abc', '(b)', 2);" + exception "invalid" + } + test { + sql "SELECT regexp_extract_all_array('abc', '(b)', -1);" + exception "invalid" + } + test { + sql "SELECT regexp_extract_all_array('abc', '(b)', 2);" + exception "invalid" + } } From 1ca8dcfd949917d2f7da8c6004ec188f971b8abc Mon Sep 17 00:00:00 2001 From: ccl125 <1253846621@qq.com> Date: Thu, 30 Jul 2026 17:26:40 +0800 Subject: [PATCH 4/5] [Fix](func) Address review: keep 2-arg legacy behavior and fix match advancing - Only enforce the out-of-range group index error for the explicit three-argument form; two-argument calls on patterns without capturing groups keep returning an empty result. - Advance past re2 matches via the match pointer instead of a textual find, which could select an earlier identical substring. - Advance past boost zero-width matches from the match position, not from the old search origin. - Fix a truncated pattern literal in the invalid-index unit test and recreate the test table for the new column-input regression cases. --- be/src/exprs/function/function_regexp.cpp | 43 +++++++++++-------- be/test/exprs/function/function_like_test.cpp | 6 ++- .../test_string_function_regexp.groovy | 19 ++++++++ 3 files changed, 50 insertions(+), 18 deletions(-) diff --git a/be/src/exprs/function/function_regexp.cpp b/be/src/exprs/function/function_regexp.cpp index 603f9d75c8cc60..56403cb628bbdd 100644 --- a/be/src/exprs/function/function_regexp.cpp +++ b/be/src/exprs/function/function_regexp.cpp @@ -174,10 +174,9 @@ struct RegexpExtractEngine { results.emplace_back(); } } - // Move position forward - auto offset = std::string(str_pos, str_size) - .find(std::string(matches[0].data(), matches[0].size())); - pos += offset + matches[0].size(); + // Move position forward. matches[0] points into the searched string, + // so its address gives the exact match offset — no textual find needed. + pos += (matches[0].data() - str_pos) + matches[0].size(); } } else if (is_boost()) { const char* search_start = data; @@ -189,10 +188,12 @@ struct RegexpExtractEngine { results.emplace_back(matches[index].str()); } if (matches[0].length() == 0) { - if (search_start == search_end) { + // Advance past the zero-width match itself, not from the old origin, + // otherwise a match found after the origin would be emitted twice. + if (matches[0].first == search_end) { break; } - search_start += 1; + search_start = matches[0].first + 1; } else { search_start = matches[0].second; } @@ -795,8 +796,8 @@ struct RegexpExtractAllImpl { const Int64 index_data = has_index ? index_col->get_int(index_check_const(i, is_const)) : 1; status = regexp_extract_all_inner_loop( - context, str_col, pattern_col, index_data, handler, null_map_data, - i); + context, str_col, pattern_col, index_data, has_index, handler, + null_map_data, i); if (!status.ok()) { return; } @@ -816,8 +817,9 @@ struct RegexpExtractAllImpl { static Status regexp_extract_all_inner_loop(FunctionContext* context, const ColumnString* str_col, const ColumnString* pattern_col, - const Int64 index_data, Handler& handler, - NullMap& null_map, const size_t index_now) { + const Int64 index_data, const bool has_index, + Handler& handler, NullMap& null_map, + const size_t index_now) { auto* engine = reinterpret_cast( context->get_function_state(FunctionContext::THREAD_LOCAL)); std::unique_ptr scoped_engine; @@ -836,14 +838,21 @@ struct RegexpExtractAllImpl { engine = scoped_engine.get(); } - // Same contract as Spark: the group index must be within - // [0, number_of_capturing_groups], anything else is an error. const int num_groups = engine->number_of_capturing_groups(); - if (index_data < 0 || index_data > num_groups) { - return Status::InvalidArgument( - "The value of parameter(s) `idx` in `{}` is invalid: Expects group index " - "between 0 and {}, but got {}.", - name, num_groups, index_data); + if (has_index) { + // Same contract as Spark: an explicit group index must be within + // [0, number_of_capturing_groups], anything else is an error. + if (index_data < 0 || index_data > num_groups) { + return Status::InvalidArgument( + "The value of parameter(s) `idx` in `{}` is invalid: Expects group index " + "between 0 and {}, but got {}.", + name, num_groups, index_data); + } + } else if (num_groups == 0) { + // Legacy two-argument behavior: patterns without capturing groups + // yield an empty result instead of an error. + handler.push_empty(index_now); + return Status::OK(); } const auto& str = str_col->get_data_at(index_now); diff --git a/be/test/exprs/function/function_like_test.cpp b/be/test/exprs/function/function_like_test.cpp index 800470b41036fa..6f68ef5d451748 100644 --- a/be/test/exprs/function/function_like_test.cpp +++ b/be/test/exprs/function/function_like_test.cpp @@ -259,6 +259,8 @@ TEST(FunctionLikeTest, regexp_extract_all) { // group index 0 also works for patterns without capturing groups {{std::string("ab1cd22"), std::string("[0-9]+"), (int64_t)0}, std::string("['1','22']")}, + // advancing past a match must use the match offset, not a textual find + {{std::string("ab b"), std::string("\\bb"), (int64_t)0}, std::string("['b']")}, // null {{std::string("abc"), Null(), (int64_t)1}, Null()}, {{Null(), std::string("i([0-9]+)"), (int64_t)1}, Null()}}; @@ -278,6 +280,8 @@ TEST(FunctionLikeTest, regexp_extract_all) { {{std::string("x=a3&x=18abc&x=2&y=3&x=4&x=17bcd"), std::string("x=([0-9]+)([a-z]+)")}, std::string("['18','17']")}, {{std::string("hitdecisiondlist"), std::string("(i)(.*?)(e)")}, std::string("['i']")}, + // legacy behavior: patterns without capturing groups yield an empty result + {{std::string("xxfs"), std::string("f")}, std::string("")}, {{std::string("abc"), Null()}, Null()}, {{Null(), std::string("i([0-9]+)")}, Null()}}; InputTypeSet two_arg_input_types = {PrimitiveType::TYPE_VARCHAR, @@ -475,7 +479,7 @@ TEST(FunctionLikeTest, regexp_extract_all_invalid_group_index) { auto col_str = ColumnString::create(); col_str->insert_data("hitdecisiondlist", 16); auto col_pattern = ColumnString::create(); - col_pattern->insert_data("(i)(.*?)(e)", 10); + col_pattern->insert_data("(i)(.*?)(e)", 11); auto col_idx = ColumnInt64::create(); col_idx->insert_value(idx); diff --git a/regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy b/regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy index 4979a087cc20a6..2f87f1a0a711ca 100644 --- a/regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy +++ b/regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy @@ -264,6 +264,25 @@ suite("test_string_function_regexp") { qt_sql_regexp_extract_all_array_group3 "select regexp_extract_all_array('hitdecisiondlist', '(i)(.*?)(e)', 3);" // column input + // column input: the table was dropped earlier in this suite, recreate it first + sql "DROP TABLE IF EXISTS test_string_function_regexp" + sql """ + CREATE TABLE IF NOT EXISTS test_string_function_regexp ( + k varchar(32), + v int, + ) + DISTRIBUTED BY HASH(k) BUCKETS 1 properties("replication_num" = "1"); + """ + sql """ + INSERT INTO test_string_function_regexp VALUES + ("billie eillish",1), + ("It's ok",2), + ("Emmy eillish",3), + ("It's true",4), + (null,5), + ("",6), + ("billie eillish",null) + """ qt_sql_regexp_extract_all_col_group0 "SELECT regexp_extract_all(k, '(ll)(i)', 0) from test_string_function_regexp ORDER BY k;" qt_sql_regexp_extract_all_col_group1 "SELECT regexp_extract_all(k, '(ll)(i)', 1) from test_string_function_regexp ORDER BY k;" qt_sql_regexp_extract_all_col_group2 "SELECT regexp_extract_all(k, '(ll)(i)', 2) from test_string_function_regexp ORDER BY k;" From cf47ba19e4de62b5f1f78008ad318cf9075c34ed Mon Sep 17 00:00:00 2001 From: ccl125 <1253846621@qq.com> Date: Fri, 31 Jul 2026 10:39:59 +0800 Subject: [PATCH 5/5] [Fix](func) Preserve the original regex subject between matches re2 now searches the original subject with an advancing start position instead of a fresh substring, so `^` stays anchored to the beginning of the original string; the boost path keeps the original begin reachable via match_prev_avail and suppresses bol at the advanced origin via match_not_bol. --- be/src/exprs/function/function_regexp.cpp | 19 ++++++++++++------- be/test/exprs/function/function_like_test.cpp | 2 ++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/be/src/exprs/function/function_regexp.cpp b/be/src/exprs/function/function_regexp.cpp index 56403cb628bbdd..6740c9e404657c 100644 --- a/be/src/exprs/function/function_regexp.cpp +++ b/be/src/exprs/function/function_regexp.cpp @@ -153,10 +153,10 @@ struct RegexpExtractEngine { size_t pos = 0; while (pos < size) { - const char* str_pos = data + pos; - size_t str_size = size - pos; std::vector matches(max_matches); - bool success = re2_regex->Match(re2::StringPiece(str_pos, str_size), 0, str_size, + // Search within the original subject starting from pos, so `^` stays + // anchored to the beginning of the original string. + bool success = re2_regex->Match(re2::StringPiece(data, size), pos, size, re2::RE2::UNANCHORED, matches.data(), max_matches); if (!success) { break; @@ -174,16 +174,21 @@ struct RegexpExtractEngine { results.emplace_back(); } } - // Move position forward. matches[0] points into the searched string, - // so its address gives the exact match offset — no textual find needed. - pos += (matches[0].data() - str_pos) + matches[0].size(); + // Advance past the match via its pointer into the original subject. + pos = (matches[0].data() - data) + matches[0].size(); } } else if (is_boost()) { const char* search_start = data; const char* search_end = data + size; boost::match_results matches; - while (boost::regex_search(search_start, search_end, matches, *boost_regex)) { + // Keep the original subject start reachable: match_prev_avail lets + // look-behind assertions see characters before search_start, and + // match_not_bol keeps `^` anchored to the original string. + while (boost::regex_search(search_start, search_end, matches, *boost_regex, + search_start == data + ? boost::match_default + : boost::match_prev_avail | boost::match_not_bol)) { if (static_cast(index) < matches.size()) { results.emplace_back(matches[index].str()); } diff --git a/be/test/exprs/function/function_like_test.cpp b/be/test/exprs/function/function_like_test.cpp index 6f68ef5d451748..d2de9e9d1e85ff 100644 --- a/be/test/exprs/function/function_like_test.cpp +++ b/be/test/exprs/function/function_like_test.cpp @@ -261,6 +261,8 @@ TEST(FunctionLikeTest, regexp_extract_all) { std::string("['1','22']")}, // advancing past a match must use the match offset, not a textual find {{std::string("ab b"), std::string("\\bb"), (int64_t)0}, std::string("['b']")}, + // `^` stays anchored to the original subject between matches + {{std::string("aa"), std::string("^a"), (int64_t)0}, std::string("['a']")}, // null {{std::string("abc"), Null(), (int64_t)1}, Null()}, {{Null(), std::string("i([0-9]+)"), (int64_t)1}, Null()}};