feat: implement shiftRight function push down #5156

Willendless · 2022-06-16T04:19:52Z

What problem does this PR solve?

Issue Number: close #5100
related pr: pingcap/tidb#35872

Problem Summary:

Implement rightShift function push down.

What is changed and how it works?

Register bitShiftRight func name in scalar_func_map.
Wrap builder with buildBitwiseFunction by updating DAGExpressionAnalyzerHelper::function_builder_map.
Update CH version's 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 shiftRight to TiFlash

ti-chi-bot · 2022-06-16T04:19:54Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

SeaRise
zanmato1984

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.

Willendless · 2022-07-06T23:19:58Z

/run-all-tests

sre-bot · 2022-07-06T23:37:56Z

Coverage for changed files

Filename                                              Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
DataTypes/NumberTraits.h                                    3                 3     0.00%           3                 3     0.00%           9                 9     0.00%           0                 0         -
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               155    55.33%          45                15    66.67%         638               294    53.92%         412               197    52.18%
Functions/bitShiftRight.cpp                                 5                 0   100.00%           2                 0   100.00%          10                 0   100.00%           2                 0   100.00%
Functions/tests/gtest_bitshiftright.cpp                  2120               430    79.72%           7                 0   100.00%         153                 0   100.00%         622               318    48.87%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                    2567               632    75.38%          71                21    70.42%        1054               399    62.14%        1104               554    49.82%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18430      9624             47.78%    207473  96549        53.46%

full coverage report (for internal network access only)

zanmato1984

LGTM

zanmato1984 · 2022-07-07T03:20:42Z

/merge

ti-chi-bot · 2022-07-07T03:20:43Z

@zanmato1984: 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-07-07T03:20:44Z

@zanmato1984: /merge in this pull request requires 2 approval(s).

In response to this:

/merge

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.

sre-bot · 2022-07-07T03:32:22Z

Coverage for changed files

Filename                                              Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
DataTypes/NumberTraits.h                                    3                 3     0.00%           3                 3     0.00%           9                 9     0.00%           0                 0         -
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               155    55.33%          45                15    66.67%         638               294    53.92%         412               197    52.18%
Functions/bitShiftRight.cpp                                 5                 0   100.00%           2                 0   100.00%          10                 0   100.00%           2                 0   100.00%
Functions/tests/gtest_bitshiftright.cpp                  2120               430    79.72%           7                 0   100.00%         153                 0   100.00%         622               318    48.87%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                    2567               632    75.38%          71                21    70.42%        1054               399    62.14%        1104               554    49.82%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18430      9624             47.78%    207473  96495        53.49%

full coverage report (for internal network access only)

SeaRise · 2022-07-07T03:36:17Z

/merge

ti-chi-bot · 2022-07-07T03:36:18Z

@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-07-07T03:36:19Z

@SeaRise: /merge in this pull request requires 2 approval(s).

In response to this:

/merge

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 · 2022-07-07T03:36:39Z

/merge

ti-chi-bot · 2022-07-07T03:36:41Z

@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-07-07T03:36:43Z

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

Commit hash: fda32fd

sre-bot · 2022-07-07T03:56:52Z

Coverage for changed files

Filename                                              Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
DataTypes/NumberTraits.h                                    3                 3     0.00%           3                 3     0.00%           9                 9     0.00%           0                 0         -
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               155    55.33%          45                15    66.67%         638               294    53.92%         412               197    52.18%
Functions/bitShiftRight.cpp                                 5                 0   100.00%           2                 0   100.00%          10                 0   100.00%           2                 0   100.00%
Functions/tests/gtest_bitshiftright.cpp                  2120               430    79.72%           7                 0   100.00%         153                 0   100.00%         622               318    48.87%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                    2567               632    75.38%          71                21    70.42%        1054               399    62.14%        1104               554    49.82%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18430      9624             47.78%    207473  96520        53.48%

full coverage report (for internal network access only)

ti-chi-bot · 2022-07-07T04:01:13Z

@Willendless: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

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.

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.

sre-bot · 2022-07-07T04:24:23Z

Coverage for changed files

Filename                                              Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
DataTypes/NumberTraits.h                                    3                 3     0.00%           3                 3     0.00%           9                 9     0.00%           0                 0         -
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               155    55.33%          45                15    66.67%         638               294    53.92%         412               197    52.18%
Functions/bitShiftRight.cpp                                 5                 0   100.00%           2                 0   100.00%          10                 0   100.00%           2                 0   100.00%
Functions/tests/gtest_bitshiftright.cpp                  2120               430    79.72%           7                 0   100.00%         153                 0   100.00%         622               318    48.87%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                    2567               632    75.38%          71                21    70.42%        1054               399    62.14%        1104               554    49.82%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18438      9624             47.80%    207600  96503        53.51%

full coverage report (for internal network access only)

…s in README (pingcap#5182) close pingcap#5172, ref pingcap#5178 Enhancement: add a integrated test on DDL module (pingcap#5130) ref pingcap#5129 Revert "Revise default background threads size" (pingcap#5176) close pingcap#5177 chore: remove extra dyn cast (pingcap#5186) close pingcap#5185 Add MPPReceiverSet, which includes ExchangeReceiver and CoprocessorReader (pingcap#5175) ref pingcap#5095 DDL: Use Column Name Instead of Offset to Find the common handle cluster index (pingcap#5166) close pingcap#5154 Add random failpoint in critical paths (pingcap#4876) close pingcap#4807 Segment test framework (pingcap#5150) close pingcap#5151 optimize ps v3 restore (pingcap#5163) ref pingcap#4914 Fix build failed (pingcap#5196) close pingcap#5195 feat: delta tree dispatching (pingcap#5199) close pingcap#5200 feat: introduce specialized API to write fixed length data rapidly (pingcap#5181) close pingcap#5183 Add gtest for Limit, TopN, Projection (pingcap#5187) (pingcap#5188) close pingcap#5187 add `MPPTask::handleError()` (pingcap#5202) ref pingcap#5095 Check result of starting grpc server (pingcap#5257) close pingcap#5255 feat: add optimized routines for aarch64 (pingcap#5231) close pingcap#5240 fix: aarch64-quick-fix (pingcap#5259) close pingcap#5260 Update client-c to support ipv6 (pingcap#5270) close pingcap#5247 upgrade prometheus-cpp to v1.0.1 (pingcap#5279) ref pingcap#2103, close pingcap#5278 Fix README type error (pingcap#5273) ref pingcap#5178 fix(cmake): make sure libc++ is utilized by tiflash-proxy (pingcap#5281) close pingcap#5282 fix the wrong order of execution summary for list based executors (pingcap#5242) close pingcap#5241 Schema: allow loading empty schema diff when the version grows up. (pingcap#5245) close pingcap#5244 Optimize apply speed under heavy write pressure (pingcap#4883) ref pingcap#4728 update proxy to raftstore-proxy-6.2 (pingcap#5287) ref pingcap#4982 Flush segment cache when doing the compaction (pingcap#5284) close pingcap#5179 metrics: Fix incorrect metrics for delta_merge tasks (pingcap#5061) close pingcap#5055 dep: upgrade jemalloc (pingcap#5197) close pingcap#5258 *: TiFlash pagectl/dttool use only-decryption mode (pingcap#5271) close pingcap#5122 suppresion false positive report from tsan (pingcap#5303) close pingcap#5088 Refine test framework code and tests (pingcap#5261) close pingcap#5262 feat: add logical cpu cores and memory into grafana (pingcap#5124) close pingcap#3821 Implement TimeToSec function push down (pingcap#5235) close pingcap#5116 feat: implement shiftRight function push down (pingcap#5156) close pingcap#5100 schema : make update to partition tables when 'set tiflash replica' (pingcap#5267) close pingcap#5266 Replace initializer_list with vector for planner test framework (pingcap#5307) close pingcap#5295 KVStore: decouple flush region and CompactLog with a new FFI fn_try_flush_data (pingcap#5283) ref pingcap#5170 refine error message in mpptask (pingcap#5304) ref pingcap#5095 Implement ReverseUTF8/Reverse function push down (pingcap#5233) close pingcap#5111 Optimize comparision for collation `UTF8_BIN` and `UTF8MB4_BIN` (pingcap#5299) ref pingcap#5294 feat : support set tiflash mode ddl action (pingcap#5256) ref pingcap#5252 Add non-blocking functions for MPMCQueue (pingcap#5311) close pingcap#5310 add random segment test for CI weekly (pingcap#5300) close pingcap#5301 *: tidy FunctionString.cpp (pingcap#5312) close pingcap#5313 ci: fix check-license github action (pingcap#5318) close pingcap#5317 update proxy to raftstore-proxy-6.2 (pingcap#5316) ref pingcap#4982 Change one `additional_input_at_end` to many streams in `ParallelInputsProcessor` (pingcap#5274) close pingcap#4856, close pingcap#5263 support fine grained shuffle for window function (pingcap#5048) close pingcap#5142 feat: pushdown get_format into TiFlash (pingcap#5269) close pingcap#5115 fix: format throw data truncated error (pingcap#5272) close pingcap#4891 Print content of columns for gtest (pingcap#5243) close pingcap#5203 *: also enable O3 for aarch64 (pingcap#5338) close pingcap#5342 Add debug image build target for CentOS7 (pingcap#5344) close pingcap#5343 *: mini refactor (pingcap#5326) close pingcap#4739 Refactor initialize of background pool (pingcap#5190) close pingcap#5189 delete copy/move ctor of MPMCQueue explicitly (pingcap#5328) close pingcap#5329 Introduce proxy_server and new-mock-engine-store (pingcap#5319) ref pingcap#5170 fix: incorrect uptime in grafana panel Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>

close pingcap#5100

feat: implement bitShiftRight function push down

777da37

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 16, 2022

Willendless added 5 commits June 16, 2022 00:20

update

f6a682b

update

b448b39

update

7f6d75c

fix: fix comparison condition

e6d320a

test: add unit test

1c0c874

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 16, 2022

test: add integration test

a5e9755

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 16, 2022

Merge branch 'pingcap:master' into rightshift-push-down

10f41db

Willendless changed the title ~~[WIP] feat: implement bitShiftRight function push down~~ feat: implement bitShiftRight function push down Jun 16, 2022

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 16, 2022

format

14590b6

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 16, 2022

Willendless added 2 commits June 16, 2022 15:13

chore: suppress sign comparing warning

edad4a0

chore: suppress sign comparing warning

e51840b

Willendless changed the title ~~feat: implement bitShiftRight function push down~~ [WIP] feat: implement bitShiftRight function push down Jun 16, 2022

ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 16, 2022

Willendless added 5 commits June 16, 2022 16:41

fix: update bit operation result type template

8416db3

fix: update bit operation result type template

187cd99

fix: update bit operation result type template

ac0e11c

fix: update bit operation result type template

828c3d8

fix: update bit operation result type template

a7c074c

chore: remove useless headers

fda32fd

zanmato1984 approved these changes Jul 7, 2022

View reviewed changes

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

SeaRise approved these changes Jul 7, 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 Jul 7, 2022

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

Merge branch 'master' into rightshift-push-down

8355ba7

Merge branch 'master' into rightshift-push-down

be5c9a8

ti-chi-bot merged commit cb69d5c into pingcap:master Jul 7, 2022

Willendless deleted the rightshift-push-down branch July 7, 2022 14:31

Lloyd-Pottiger pushed a commit to Lloyd-Pottiger/tiflash that referenced this pull request Jul 19, 2022

feat: implement shiftRight function push down (pingcap#5156)

25c2f82

close pingcap#5100

AnnieoftheStars mentioned this pull request Jul 28, 2022

feat: implement shiftLeft function push down #5495

Merged

12 tasks

SeaRise mentioned this pull request Aug 11, 2022

support CastTimeAsDuration in TiFlash #5548

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement shiftRight function push down #5156

feat: implement shiftRight function push down #5156

Willendless commented Jun 16, 2022 •

edited

Loading

ti-chi-bot commented Jun 16, 2022 •

edited

Loading

Willendless commented Jul 6, 2022

sre-bot commented Jul 6, 2022

zanmato1984 left a comment

zanmato1984 commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

sre-bot commented Jul 7, 2022

SeaRise commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

SeaRise commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

sre-bot commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

sre-bot commented Jul 7, 2022

feat: implement shiftRight function push down #5156

feat: implement shiftRight function push down #5156

Conversation

Willendless commented Jun 16, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jun 16, 2022 • edited Loading

Willendless commented Jul 6, 2022

sre-bot commented Jul 6, 2022

Coverage for changed files

Coverage summary

zanmato1984 left a comment

Choose a reason for hiding this comment

zanmato1984 commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

sre-bot commented Jul 7, 2022

Coverage for changed files

Coverage summary

SeaRise commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

SeaRise commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

sre-bot commented Jul 7, 2022

Coverage for changed files

Coverage summary

ti-chi-bot commented Jul 7, 2022

sre-bot commented Jul 7, 2022

Coverage for changed files

Coverage summary

Willendless commented Jun 16, 2022 •

edited

Loading

ti-chi-bot commented Jun 16, 2022 •

edited

Loading