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.

Sign up for GitHub

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

Jump to bottom

expression: check ErrTruncate/Overflow for CastRealAsDecimalSig #18961

Merged
merged 3 commits into from
Aug 4, 2020
Merged

expression: check ErrTruncate/Overflow for CastRealAsDecimalSig #18961

merged 3 commits into from
Aug 4, 2020

Conversation

XuHuaiyu
Copy link
Contributor

@XuHuaiyu XuHuaiyu commented Aug 4, 2020

What problem does this PR solve?

Issue Number: close #16147

Problem Summary:
Before this commit, the truncate/overflow error of children of builtinArithmeticIntDivideDecimalSig is checked in builtinArithmeticIntDivideDecimalSig.vecEvalInt. But when the error happens, the buf[0] or buf[1] maybe not initialized, and thus cause the should ensure all columns have the same length panic in MergeNulls.

What is changed and how it works?

Proposal: xxx

What's Changed:
Remove the check for children's truncate/overflow error in builtinArithmeticIntDivideDecimalSig

How it Works:

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

exists test

Side effects

N/A

Release note

  • check ErrTruncate/Overflow locally for builtinCastRealAsDecimalSig to fix the "should ensure all columns have the same length" error

@XuHuaiyu XuHuaiyu requested a review from a team as a code owner August 4, 2020 04:01
@XuHuaiyu XuHuaiyu requested review from wshwsh12 and removed request for a team August 4, 2020 04:01
@XuHuaiyu
Copy link
Contributor Author

XuHuaiyu commented Aug 4, 2020

/run-unit-test

@XuHuaiyu
Copy link
Contributor Author

XuHuaiyu commented Aug 4, 2020

/run-all-tests

@qw4990 qw4990 self-requested a review August 4, 2020 06:16
wshwsh12
wshwsh12 reviewed Aug 4, 2020
View reviewed changes
Copy link
Contributor

@wshwsh12 wshwsh12 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 4, 2020
@codecov
Copy link

codecov bot commented Aug 4, 2020 •
edited
Loading

Codecov Report

Merging #18961 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #18961   +/-   ##
===========================================
  Coverage   79.5524%   79.5524%           
===========================================
  Files           546        546           
  Lines        148551     148551           
===========================================
  Hits         118176     118176           
  Misses        20907      20907           
  Partials       9468       9468

qw4990
qw4990 approved these changes Aug 4, 2020
View reviewed changes
Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 4, 2020
@qw4990
Copy link
Contributor

qw4990 commented Aug 4, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 4, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@XuHuaiyu merge failed.

@XuHuaiyu
Copy link
Contributor Author

XuHuaiyu commented Aug 4, 2020

/run-check_dev

@XuHuaiyu XuHuaiyu changed the title expression: check ErrTruncate/Overflow locally for builtinCastRealAsDecimalSig expression: check ErrTruncate/Overflow for CastRealAsDecimalSig Aug 4, 2020
@XuHuaiyu XuHuaiyu merged commit 4645c65 into pingcap:master Aug 4, 2020
@XuHuaiyu XuHuaiyu deleted the int_div branch August 4, 2020 06:37
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Aug 4, 2020
@XuHuaiyu @ti-srebot
cherry pick pingcap#18961 to release-4.0
32b5a56 
Signed-off-by: ti-srebot <[email protected]>
@ti-srebot ti-srebot mentioned this pull request Aug 4, 2020
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #18967

jebter pushed a commit that referenced this pull request Aug 4, 2020
@ti-srebot @XuHuaiyu
cherry pick #18961 to release-4.0 (#18967)
df16c56 
Signed-off-by: ti-srebot <[email protected]>

Co-authored-by: HuaiyuXu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wshwsh12 wshwsh12 wshwsh12 left review comments

@qw4990 qw4990 qw4990 approved these changes

@ti-srebot ti-srebot ti-srebot approved these changes

Assignees
No one assigned
Labels
component/expression status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error 1105: should ensure all columns have the same length
4 participants
@XuHuaiyu @qw4990 @ti-srebot @wshwsh12