Skip to content
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

[fix #537] fix convert byte[] to long error #538

Merged
merged 8 commits into from
Mar 29, 2022

Conversation

shiyuhang0
Copy link
Collaborator

@shiyuhang0 shiyuhang0 commented Feb 25, 2022

What problem does this PR solve?

Issue Number: fix #537

Problem Description: convert byte[] to long error in TiChunkColumnVector#getLongFromBinary

What is changed and how it works?

When long and byte do | operation, I guess byte is converted to long, It make the wrong answer.
So, Use 0xff to intercept the last eight binary data

Check List for Tests

This PR has been tested by the at least one of the following methods:

  • Unit test
  • Integration test

Side effects

Related changes

  • Need to cherry-pick to other release

@zz-jason
Copy link
Member

@shiyuhang0 thanks for your contribution! please:

  1. fix the DCO. every commit in the pull request needs to be signed off.
  2. add a unit test for your bug fix.

@codecov
Copy link

codecov bot commented Feb 26, 2022

Codecov Report

Merging #538 (4f73759) into master (ef16787) will increase coverage by 0.83%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #538      +/-   ##
============================================
+ Coverage     33.23%   34.06%   +0.83%     
- Complexity     1338     1361      +23     
============================================
  Files           270      270              
  Lines         17131    17131              
  Branches       1950     1950              
============================================
+ Hits           5693     5836     +143     
+ Misses        10833    10681     -152     
- Partials        605      614       +9     
Impacted Files Coverage Δ
.../org/tikv/common/columnar/TiChunkColumnVector.java 19.04% <100.00%> (+19.04%) ⬆️
...va/org/tikv/common/region/StoreHealthyChecker.java 71.79% <0.00%> (-2.57%) ⬇️
...ty/handler/codec/http2/Http2ConnectionHandler.java 52.56% <0.00%> (+0.97%) ⬆️
...rc/main/java/io/grpc/netty/NettyClientHandler.java 63.36% <0.00%> (+7.32%) ⬆️
src/main/java/org/tikv/common/types/DataType.java 20.18% <0.00%> (+10.55%) ⬆️
.../java/org/tikv/common/columnar/TiColumnVector.java 11.76% <0.00%> (+11.76%) ⬆️
src/main/java/org/tikv/common/types/BitType.java 13.33% <0.00%> (+13.33%) ⬆️
...c/main/java/org/tikv/common/types/IntegerType.java 22.41% <0.00%> (+22.41%) ⬆️
src/main/java/org/tikv/common/types/MySQLType.java 84.31% <0.00%> (+84.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef16787...4f73759. Read the comment docs.

@shiyuhang0
Copy link
Collaborator Author

@shiyuhang0 thanks for your contribution! please:

  1. fix the DCO. every commit in the pull request needs to be signed off.
  2. add a unit test for your bug fix.

have done, PTAL

@shiyuhang0
Copy link
Collaborator Author

/run-all-tests

zz-jason
zz-jason previously approved these changes Feb 28, 2022
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zz-jason
Copy link
Member

@shiyuhang0 please fix license check and code format check failures.

shiyuhang0 and others added 6 commits March 1, 2022 11:05
Signed-off-by: shiyuhang <1136742008@qq.com>
Signed-off-by: shiyuhang <1136742008@qq.com>
Signed-off-by: shiyuhang <1136742008@qq.com>
Signed-off-by: shiyuhang <1136742008@qq.com>
Signed-off-by: shiyuhang <1136742008@qq.com>
Signed-off-by: shiyuhang <1136742008@qq.com>
@shiyuhang0
Copy link
Collaborator Author

@shiyuhang0 please fix license check and code format check failures.

done

Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shiyuhang0
Copy link
Collaborator Author

@marsishandsome @iosmanthus PTAL

@zz-jason zz-jason enabled auto-merge (squash) March 29, 2022 06:04
@iosmanthus
Copy link
Member

Rest LGTM. However, @marsishandsome is working on removing the coprocessor code out of the code base. This pull request might suffer from this change.

Copy link
Member

@iosmanthus iosmanthus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zz-jason zz-jason merged commit 56d64ef into tikv:master Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

convert byte[] to long error
5 participants