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

aggregation: handle non-integer input for bit related aggregate functions #6994

Merged
merged 4 commits into from
Jul 5, 2018
Merged

aggregation: handle non-integer input for bit related aggregate functions #6994

merged 4 commits into from
Jul 5, 2018

Conversation

crazycs520
Copy link
Contributor

What have you changed? (mandatory)

fix issue#6987
bit_or doesn't handle decimal

What are the type of the changes (mandatory)?

Bug fix

How has this PR been tested (mandatory)?

Does this PR affect documentation (docs/docs-cn) update? (optional)

No.

XuHuaiyu
XuHuaiyu reviewed Jul 5, 2018
View reviewed changes
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

add a test case for this.

@zz-jason zz-jason added this to the 2.1 milestone Jul 5, 2018
@zz-jason
Copy link
Member

zz-jason commented Jul 5, 2018

@crazycs520 You can add some tests in executor/aggregate_test.go

@crazycs520
Copy link
Contributor Author

@zz-jason @XuHuaiyu Already add a test. PTAL.
bit_and, bit_xor also have the same problem too, I'll fix it.

zz-jason
zz-jason reviewed Jul 5, 2018
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

@XuHuaiyu
Copy link
Contributor

XuHuaiyu commented Jul 5, 2018

/run-all-tests

@crazycs520
Copy link
Contributor Author

@XuHuaiyu @zz-jason I fix the same bug of bit_and, bit_xor in this pr, PTAL.

@XuHuaiyu
Copy link
Contributor

XuHuaiyu commented Jul 5, 2018

/run-all-tests

zz-jason
zz-jason reviewed Jul 5, 2018
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 changed the title aggregation: fix bit_or handle decimal bugs aggregation: handle non-integer input for bit related aggregate functions Jul 5, 2018
@crazycs520
Copy link
Contributor Author

/run-all-tests

@winoros winoros added status/LGT1 Indicates that a PR has LGTM 1. contribution This PR is from a community contributor. labels Jul 5, 2018
winoros
winoros approved these changes Jul 5, 2018
View reviewed changes
Copy link
Member

@winoros winoros 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 73dc86c into pingcap:master Jul 5, 2018
@zz-jason zz-jason added status/LGT2 Indicates that a PR has LGTM 2. status/all tests passed and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 5, 2018
@crazycs520 crazycs520 deleted the dev/fix-bit-or branch July 5, 2018 16:28
@AndreMouche AndreMouche mentioned this pull request Jul 17, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zz-jason zz-jason zz-jason left review comments

@XuHuaiyu XuHuaiyu XuHuaiyu left review comments

@winoros winoros winoros approved these changes

Assignees
No one assigned
Labels
component/expression 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
2.1
Development

Successfully merging this pull request may close these issues.

result of bit_or is different from mysql
4 participants
@crazycs520 @zz-jason @XuHuaiyu @winoros