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

types/data: the bit data type should be treat as uint64 (#11951) #12424

Merged
merged 21 commits into from
Jan 9, 2020
Merged

types/data: the bit data type should be treat as uint64 (#11951) #12424

merged 21 commits into from
Jan 9, 2020

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented Sep 26, 2019

cherry-pick #11951 to release-2.1

What problem does this PR solve?

Fix #11895
the -1 can't be insert into a bit column.

What is changed and how it works?

  1. when convert to mysql bit, mapped it to unsigned int.
  2. the error is wrong (it should be data too long instead of out of range)

Check List

Tests

  • Unit test -- added test cases

@sre-bot
Copy link
Contributor Author

sre-bot commented Sep 26, 2019

/run-all-tests

@sre-bot sre-bot mentioned this pull request Sep 26, 2019
Merged
@sre-bot sre-bot added contribution This PR is from a community contributor. type/2.1 cherry-pick type/bugfix This PR fixes a bug. labels Sep 26, 2019
SunRunAway
SunRunAway reviewed Sep 26, 2019
View reviewed changes
Copy link
Contributor

@SunRunAway SunRunAway left a comment

Choose a reason for hiding this comment

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

LGTM

@qw4990
Copy link
Contributor

qw4990 commented Sep 27, 2019

/rebuild

qw4990
qw4990 reviewed Sep 27, 2019
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.

Please help us resolve CI problems. @gaoxingliang

@gaoxingliang
Copy link
Contributor

gaoxingliang commented Sep 29, 2019 •
edited
Loading

Please help us resolve CI problems. @gaoxingliang

Hi @qw4990 I know how to fix this. but I don't know the process flow of how to operate. Do I need to create a new PR base on sre-bot:release-2.1-2bba5654b456 or any other options?

@zz-jason zz-jason requested a review from qw4990 September 30, 2019 06:35
qw4990
qw4990 reviewed Sep 30, 2019
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.

@gaoxingliang Sorry for my late reply.
You can fetch the branch release-2.1 to local and git cherry-pick your PR into your local release-2.1 and then create a new PR to merge your local release-2.1 to TiDB release-2.1.

@zz-jason zz-jason removed their request for review October 9, 2019 09:00
@gaoxingliang gaoxingliang mentioned this pull request Oct 11, 2019
Closed
@zimulala zimulala added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 9, 2019
@bb7133 bb7133 added this to the v2.1.19 milestone Nov 22, 2019
@SunRunAway
Copy link
Contributor

I've resolved the conflict.

@SunRunAway SunRunAway requested a review from qw4990 November 29, 2019 07:27
@SunRunAway SunRunAway self-assigned this Dec 20, 2019
@SunRunAway
Copy link
Contributor

/run-unit-test

qw4990
qw4990 previously approved these changes Dec 23, 2019
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

@qw4990 qw4990 added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 23, 2019
@qw4990
Copy link
Contributor

qw4990 commented Dec 23, 2019

/run-unit-test

1 similar comment
@SunRunAway
Copy link
Contributor

/run-unit-test

@SunRunAway
Copy link
Contributor

I'm debugging this unit test failing, along with #13751

@zyxbest
Copy link
Contributor

zyxbest commented Dec 23, 2019

/run-unit-test

@SunRunAway
Copy link
Contributor

/run-unit-test

1 similar comment
@SunRunAway
Copy link
Contributor

/run-unit-test

@SunRunAway SunRunAway force-pushed the release-2.1-2bba5654b456 branch from b774695 to af96a3a Compare December 25, 2019 10:52
@SunRunAway
Copy link
Contributor

/run-all-tests

@SunRunAway
Copy link
Contributor

/run-all-tests

zz-jason
zz-jason approved these changes Jan 9, 2020
View reviewed changes
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 zz-jason merged commit 2841779 into pingcap:release-2.1 Jan 9, 2020
@bb7133 bb7133 added this to the v2.1.20 milestone Jan 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SunRunAway SunRunAway SunRunAway left review comments

@zz-jason zz-jason zz-jason approved these changes

@qw4990 qw4990 qw4990 left review comments

Assignees

@SunRunAway SunRunAway

Labels
contribution This PR is from a community contributor. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
v2.1.20
Development

Successfully merging this pull request may close these issues.
8 participants
@sre-bot @qw4990 @gaoxingliang @SunRunAway @zyxbest @zz-jason @bb7133 @zimulala