Support `regexp` in TiFlash by windtalker · Pull Request #3957 · pingcap/tiflash

windtalker · 2022-01-28T07:44:45Z

What problem does this PR solve?

Issue Number: close #3956

Problem Summary:
Support regexp in TiFlash

What is changed and how it works?

Support regexp/regexp_replace according to MySQL documents. Note TiDB only support regexp with two arguments(data regexp pattern), so some of the code path like regexp_replace and match_type in regexp/regexp_replace is not used currently.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Support `regexp` in TiFlash

ti-chi-bot · 2022-01-28T07:44:46Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

fzhedu

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

windtalker · 2022-01-28T09:51:10Z

/run-all-tests

fzhedu · 2022-02-25T03:23:02Z

tests/fullstack-test/expr/regexp.test

+mysql> insert into test.t values ('aaaa', 'AAAA', '^a.*', '^A.*'), ('abcd', 'abcd', null, '^a..d$'), (null, 'bbb', 'bb$', 'bb$'),('中文测试','中文测试','中文','^....$'),('中English混合','中English混合','^中English','^..nglish..$');
+mysql> alter table test.t set tiflash replica 1


add some empty string cases

fzhedu · 2022-02-25T03:42:57Z

dbms/src/Flash/Coprocessor/DAGExpressionAnalyzerHelper.cpp

+        String name = analyzer->getActions(child, actions);
+        argument_names.push_back(name);


would names be replicated?

replicated name should point to the same column, and it is ok that the arguments of the functions comes from the same column.

fzhedu · 2022-02-25T04:31:41Z

dbms/src/Functions/FunctionsStringSearch.cpp

+            const UInt8 * begin = &data[0];
+            memcpy(&res_data[0], begin, data.size());
+            memcpy(&res_offsets[0], &offsets[0], size * sizeof(UInt64));


are xx[0] of xx not empty?

I think res_data/res_offsets/data/offsets are not empty, it is possible that the data.size()/offsets.size() could be zero, but this is ok.

dbms/src/Functions/FunctionsStringSearch.h

fzhedu

/LGTM

fzhedu · 2022-02-28T02:38:10Z

/merge

ti-chi-bot · 2022-02-28T02:38:11Z

@fzhedu: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2022-02-28T02:38:17Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 06483f3

sre-bot · 2022-02-28T03:10:01Z

Coverage for changed files

Filename                                              Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Common/OptimizedRegularExpression.h                         5                 2    60.00%           5                 2    60.00%          13                 4    69.23%           0                 0         -
Common/OptimizedRegularExpression.inl.h                   196               103    47.45%           4                 1    75.00%         317               187    41.01%         194               114    41.24%
Flash/Coprocessor/DAGExpressionAnalyzerHelper.cpp          91                91     0.00%          13                13     0.00%         237               237     0.00%          68                68     0.00%
Flash/Coprocessor/DAGUtils.cpp                            285               228    20.00%          35                23    34.29%         491               390    20.57%         320               226    29.38%
Functions/FunctionsStringArray.h                          150               150     0.00%          40                40     0.00%         301               301     0.00%          84                84     0.00%
Functions/FunctionsStringSearch.cpp                       645               333    48.37%          56                29    48.21%        1312               670    48.93%         410               215    47.56%
Functions/FunctionsStringSearch.h                          85                33    61.18%          15                 9    40.00%         166                64    61.45%          42                20    52.38%
Functions/FunctionsVisitParam.h                            96                96     0.00%          10                10     0.00%         150               150     0.00%          78                78     0.00%
Functions/Regexps.h                                         6                 0   100.00%           4                 0   100.00%          21                 0   100.00%           0                 0         -
Functions/tests/gtest_regexp.cpp                        11158              1958    82.45%          13                 0   100.00%        1509                 0   100.00%        3966              1949    50.86%
Storages/Transaction/Collator.h                            14                 2    85.71%           8                 2    75.00%          14                 5    64.29%           8                 3    62.50%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                   12731              2996    76.47%         203               129    36.45%        4531              2008    55.68%        5170              2757    46.67%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16715      9483             43.27%    186682  95638        48.77%

full coverage report (for internal network access only)

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 28, 2022

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 7, 2022

windtalker force-pushed the regexp branch from 721fa27 to 1ac1a86 Compare February 8, 2022 01:37

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 8, 2022

windtalker force-pushed the regexp branch 3 times, most recently from b1dc1a4 to 403c306 Compare February 15, 2022 05:00

windtalker force-pushed the regexp branch from 403c306 to 5ccb7e7 Compare February 22, 2022 01:13

fzhedu reviewed Feb 25, 2022

View reviewed changes

dbms/src/Functions/FunctionsStringSearch.h Show resolved Hide resolved

windtalker added 4 commits February 25, 2022 15:42

add support for regexp

f26066e

refine code

01a923b

fix build

6e9082a

address comments

06483f3

windtalker force-pushed the regexp branch from 5ccb7e7 to 06483f3 Compare February 25, 2022 07:42

fuzhe1989 self-requested a review February 27, 2022 12:30

fzhedu approved these changes Feb 28, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 28, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 28, 2022

Merge branch 'master' into regexp

eafb605

ti-chi-bot merged commit 860405e into pingcap:master Feb 28, 2022

		mysql> insert into test.t values ('aaaa', 'AAAA', '^a.', '^A.'), ('abcd', 'abcd', null, '^a..d$'), (null, 'bbb', 'bb$', 'bb$'),('中文测试','中文测试','中文','^....$'),('中English混合','中English混合','^中English','^..nglish..$');
		mysql> alter table test.t set tiflash replica 1

		String name = analyzer->getActions(child, actions);
		argument_names.push_back(name);

Conversation

windtalker commented Jan 28, 2022

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

ti-chi-bot commented Jan 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

windtalker commented Jan 28, 2022

Uh oh!

fzhedu Feb 25, 2022

Choose a reason for hiding this comment

Uh oh!

windtalker Feb 25, 2022

Choose a reason for hiding this comment

Uh oh!

fzhedu Feb 25, 2022

Choose a reason for hiding this comment

Uh oh!

windtalker Feb 25, 2022

Choose a reason for hiding this comment

Uh oh!

fzhedu Feb 25, 2022

Choose a reason for hiding this comment

Uh oh!

windtalker Feb 28, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fzhedu left a comment

Choose a reason for hiding this comment

Uh oh!

fzhedu commented Feb 28, 2022

Uh oh!

ti-chi-bot commented Feb 28, 2022

Uh oh!

ti-chi-bot commented Feb 28, 2022

Uh oh!

sre-bot commented Feb 28, 2022

Coverage for changed files

Coverage summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ti-chi-bot commented Jan 28, 2022 •

edited

Loading