feat: implement shiftLeft function push down #5495

AnnieoftheStars · 2022-07-28T12:01:27Z

What problem does this PR solve?

Issue Number: close #5099

related pr: pingcap/tidb#36677

Problem Summary:

implement shiftLeft function push down

What is changed and how it works?

Register bitShiftLeft func name in scalar_func_map.
update DAGExpressionAnalyzerHelper::function_builder_map, wrap bitShiftLeft builder with buildBitwiseFunction.
Referring to shiftRight implementation and cppRef, update original BitShiftRightImpl to handle a C++ undefined behavior of shift operation.

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 to pushdown shiftLeft to TiFlash

ti-chi-bot · 2022-07-28T12:01:29Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

SeaRise
wshwsh12

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.

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

sre-bot · 2022-07-28T12:01:35Z

All committers have signed the CLA.

ti-chi-bot · 2022-07-28T12:01:39Z

Welcome @AnnieoftheStars!

It looks like this is your first PR to pingcap/tiflash 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to pingcap/tiflash. 😃

AnnieoftheStars · 2022-07-28T12:08:11Z

/cc @Willendless Please take a look

ti-chi-bot · 2022-07-28T12:08:14Z

@AnnieoftheStars: GitHub didn't allow me to request PR reviews from the following users: Please, take, a, look.

Note that only pingcap members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @Willendless Please take a look

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 kubernetes/test-infra repository.

AnnieoftheStars · 2022-07-28T12:40:09Z

/cc @zanmato1984 @guo-shaoge

AnnieoftheStars · 2022-07-28T12:50:16Z

/retest

Willendless · 2022-07-28T13:01:52Z

Thanks for your contribution, excellent job! 🥳

Willendless · 2022-07-28T13:05:22Z

According to https://github.com/pingcap/tiflash#contributing, you can try

cd $WORKSPACE/tiflash
python3 format-diff.py --diff_from `git merge-base ${TARGET_REMOTE_BRANCH} HEAD`

to format your code :)

Willendless · 2022-07-28T13:25:44Z

/run-all-tests

sre-bot · 2022-07-28T13:43:16Z

Coverage for changed files

Filename                                              Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/Coprocessor/DAGExpressionAnalyzerHelper.cpp          92                44    52.17%          14                 3    78.57%         244                96    60.66%          68                39    42.65%
Flash/Coprocessor/DAGUtils.cpp                            347               141    59.37%          45                11    75.56%         638               271    57.52%         412               190    53.88%
Functions/bitShiftLeft.cpp                                  5                 0   100.00%           2                 0   100.00%          10                 0   100.00%           2                 0   100.00%
Functions/tests/gtest_bitshiftleft.cpp                    357                49    86.27%           7                 0   100.00%         153                 0   100.00%          28                28     0.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                     801               234    70.79%          68                14    79.41%        1045               367    64.88%         510               257    49.61%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18875      9437             50.00%    215184  95374        55.68%

full coverage report (for internal network access only)

… into issue-5009

AnnieoftheStars · 2022-07-29T15:51:04Z

According to https://github.com/pingcap/tiflash#contributing, you can try
cd $WORKSPACE/tiflash
python3 format-diff.py --diff_from `git merge-base ${TARGET_REMOTE_BRANCH} HEAD`
to format your code :)

okay.

Willendless · 2022-07-29T15:53:45Z

/run-all-tests

sre-bot · 2022-07-29T16:10:14Z

Coverage for changed files

Filename                                              Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/Coprocessor/DAGExpressionAnalyzerHelper.cpp          92                44    52.17%          14                 3    78.57%         244                96    60.66%          68                39    42.65%
Flash/Coprocessor/DAGUtils.cpp                            347               141    59.37%          45                11    75.56%         638               271    57.52%         412               190    53.88%
Functions/bitShiftLeft.cpp                                  5                 0   100.00%           2                 0   100.00%          10                 0   100.00%           2                 0   100.00%
Functions/tests/gtest_bitshiftleft.cpp                    357                49    86.27%           7                 0   100.00%         153                 0   100.00%          28                28     0.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                     801               234    70.79%          68                14    79.41%        1045               367    64.88%         510               257    49.61%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18875      9438             50.00%    215184  95395        55.67%

full coverage report (for internal network access only)

Willendless · 2022-07-29T17:20:41Z

LGTM

ti-chi-bot · 2022-07-29T17:22:29Z

@Willendless: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

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.

SeaRise

LGTM

SeaRise · 2022-08-04T07:14:38Z

/merge

ti-chi-bot · 2022-08-04T07:14:39Z

@SeaRise: 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.

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-08-04T07:14:40Z

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

Commit hash: 32f827c

sre-bot · 2022-08-04T07:34:00Z

Coverage for changed files

Filename                                              Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/Coprocessor/DAGExpressionAnalyzerHelper.cpp          92                44    52.17%          14                 3    78.57%         244                96    60.66%          68                39    42.65%
Flash/Coprocessor/DAGUtils.cpp                            347               135    61.10%          45                 7    84.44%         638               257    59.72%         412               188    54.37%
Functions/bitShiftLeft.cpp                                  5                 0   100.00%           2                 0   100.00%          10                 0   100.00%           2                 0   100.00%
Functions/tests/gtest_bitshiftleft.cpp                    357                49    86.27%           7                 0   100.00%         153                 0   100.00%          28                28     0.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                     801               228    71.54%          68                10    85.29%        1045               353    66.22%         510               255    50.00%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
19022      9231             51.47%    215506  93988        56.39%

full coverage report (for internal network access only)

AnnieoftheStars added 2 commits July 26, 2022 23:55

impl shiftLeft function pushdown

d21d217

shiftLeft intergation test

bfde110

ti-chi-bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. labels Jul 28, 2022

ti-chi-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 28, 2022

Merge branch 'master' into issue-5009

980d685

ti-chi-bot requested a review from Willendless July 28, 2022 12:08

AnnieoftheStars changed the title ~~feat: implement shiftRight function push down~~ feat: implement shiftLeft function push down Jul 28, 2022

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. labels Jul 28, 2022

ti-chi-bot requested review from guo-shaoge and zanmato1984 July 28, 2022 12:40

Willendless mentioned this pull request Jul 28, 2022

Call For Participation: implement function push down #5092

Open

AnnieoftheStars added 2 commits July 29, 2022 23:05

code format

4671076

Merge branch 'issue-5009' of https://github.com/AnnieoftheStars/tiflash…

32f827c

… into issue-5009

Willendless approved these changes Jul 29, 2022

View reviewed changes

wshwsh12 approved these changes Aug 1, 2022

View reviewed changes

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

SeaRise approved these changes Aug 3, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 3, 2022

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

Merge branch 'master' into issue-5009

ab8ed62

ti-chi-bot merged commit e17ef39 into pingcap:master Aug 4, 2022

AnnieoftheStars deleted the issue-5009 branch August 5, 2022 09:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement shiftLeft function push down #5495

feat: implement shiftLeft function push down #5495

AnnieoftheStars commented Jul 28, 2022 •

edited

Loading

ti-chi-bot commented Jul 28, 2022 •

edited

Loading

sre-bot commented Jul 28, 2022 •

edited

Loading

ti-chi-bot commented Jul 28, 2022

AnnieoftheStars commented Jul 28, 2022

ti-chi-bot commented Jul 28, 2022

AnnieoftheStars commented Jul 28, 2022

AnnieoftheStars commented Jul 28, 2022

Willendless commented Jul 28, 2022 •

edited

Loading

Willendless commented Jul 28, 2022

Willendless commented Jul 28, 2022

sre-bot commented Jul 28, 2022

AnnieoftheStars commented Jul 29, 2022

Willendless commented Jul 29, 2022

sre-bot commented Jul 29, 2022

Willendless commented Jul 29, 2022 •

edited

Loading

ti-chi-bot commented Jul 29, 2022

SeaRise left a comment

SeaRise commented Aug 4, 2022

ti-chi-bot commented Aug 4, 2022

ti-chi-bot commented Aug 4, 2022

sre-bot commented Aug 4, 2022

feat: implement shiftLeft function push down #5495

feat: implement shiftLeft function push down #5495

Conversation

AnnieoftheStars commented Jul 28, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jul 28, 2022 • edited Loading

sre-bot commented Jul 28, 2022 • edited Loading

ti-chi-bot commented Jul 28, 2022

AnnieoftheStars commented Jul 28, 2022

ti-chi-bot commented Jul 28, 2022

AnnieoftheStars commented Jul 28, 2022

AnnieoftheStars commented Jul 28, 2022

Willendless commented Jul 28, 2022 • edited Loading

Willendless commented Jul 28, 2022

Willendless commented Jul 28, 2022

sre-bot commented Jul 28, 2022

Coverage for changed files

Coverage summary

AnnieoftheStars commented Jul 29, 2022

Willendless commented Jul 29, 2022

sre-bot commented Jul 29, 2022

Coverage for changed files

Coverage summary

Willendless commented Jul 29, 2022 • edited Loading

ti-chi-bot commented Jul 29, 2022

SeaRise left a comment

Choose a reason for hiding this comment

SeaRise commented Aug 4, 2022

ti-chi-bot commented Aug 4, 2022

ti-chi-bot commented Aug 4, 2022

sre-bot commented Aug 4, 2022

Coverage for changed files

Coverage summary

AnnieoftheStars commented Jul 28, 2022 •

edited

Loading

ti-chi-bot commented Jul 28, 2022 •

edited

Loading

sre-bot commented Jul 28, 2022 •

edited

Loading

Willendless commented Jul 28, 2022 •

edited

Loading

Willendless commented Jul 29, 2022 •

edited

Loading