-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[fix](function) Result type of regexp_extract_all should be Array #50367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
run buildall |
.../main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the return type of the regexp_extract_all function so that it returns an array instead of a string. Key changes include updating the function registration in the Python builtins script, modifying the Java expression builder to support a three-argument signature with an array return type, and updating the vectorized function implementation and unit tests accordingly.
Reviewed Changes
Copilot reviewed 5 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
gensrc/script/doris_builtins_functions.py | Updated function signature to register regexp_extract_all with ARRAY_VARCHAR return type instead of VARCHAR. |
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/RegexpExtractAll.java | Revised the function implementation to support a 2- or 3-argument signature and return an array type, and removed legacy nullable interfaces. |
be/test/vec/function/function_test_util.h | Adjusted test utility template parameters to support the new array result type. |
be/test/vec/function/function_like_test.cpp | Modified test cases to validate the new array output from regexp_extract_all. |
be/src/vec/functions/function_regexp.cpp | Updated the implementation of the vectorized regexp_extract_all, including changes to argument handling and result construction for array outputs. |
Files not reviewed (4)
- regression-test/data/nereids_function_p0/scalar_function/R.out: Language not supported
- regression-test/data/nereids_syntax_p0/function.out: Language not supported
- regression-test/data/query_p0/sql_functions/string_functions/test_string_function_regexp.out: Language not supported
- regression-test/suites/query_p0/sql_functions/string_functions/test_string_function_regexp.groovy: Language not supported
Comments suppressed due to low confidence (1)
be/src/vec/functions/function_regexp.cpp:407
- [nitpick] Returning 0 in get_number_of_arguments() for RegexpExtractAllImpl may be non-obvious; consider adding a comment to clarify that this function is variadic and that the return value is not used conventionally.
return 0;
run buildall |
TeamCity cloud ut coverage result: |
TPC-H: Total hot run time: 34071 ms
|
TPC-DS: Total hot run time: 185479 ms
|
ClickBench: Total hot run time: 29.07 s
|
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression P0 && UT Coverage ReportIncrement line coverage Increment coverage report
|
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
Function
regexp_extract_all
should return a array instead of string.Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)