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

coprocessor: support builtin aggregation function bit_and, 'bit_or, bit_xor. #2510

Merged
merged 28 commits into from
Dec 29, 2017
Merged

coprocessor: support builtin aggregation function bit_and, 'bit_or, bit_xor. #2510

merged 28 commits into from
Dec 29, 2017

Conversation

spongedu
Copy link
Collaborator

The related modification in tipb is : pingcap/tipb#67

@sre-bot
Copy link
Contributor

sre-bot commented Nov 26, 2017

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

@CLAassistant
Copy link

CLAassistant commented Nov 26, 2017 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@AndreMouche
Copy link
Member

Please fix the CI

@AndreMouche
Copy link
Member

@breeswish @winkyao PTAL

@spongedu
Copy link
Collaborator Author

@AndreMouche to make ci work, pingcap/tipb#67 must be checked in first.

@AndreMouche
Copy link
Member

@hanfei1991 PTAL

@spongedu
Copy link
Collaborator Author

Should be merge after #2503 because some other tipb changes works with it.

@spongedu
Copy link
Collaborator Author

@AndreMouche @hanfei1991 @winkyao PTAL

AndreMouche
AndreMouche reviewed Dec 5, 2017
View reviewed changes
Copy link
Member

@AndreMouche AndreMouche left a comment

Choose a reason for hiding this comment

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

It seems tidb haven't implemented, @hanfei1991 PTAL

Cargo.lock Outdated
@@ -1207,7 +1207,7 @@ dependencies = [
"checksum thread-id 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "a9539db560102d1cef46b8b78ce737ff0bb64e7e18d35b2a5688f7d097d0ff03"
"checksum thread_local 0.2.7 (registry+https://github.com/rust-lang/crates.io-index)" = "8576dbbfcaef9641452d5cf0df9b0e7eeab7694956dd33bb61515fb8f18cfdd5"
"checksum time 0.1.38 (registry+https://github.com/rust-lang/crates.io-index)" = "d5d788d3aa77bc0ef3e9621256885555368b47bd495c13dd2e7413c89f845520"
"checksum tipb 0.0.1 (git+https://github.com/pingcap/tipb.git)" = "<none>"
"checksum tipb 0.0.1 (git+https://github.com/spongedu/tipb.git)" = "<none>"
Copy link
Member

Choose a reason for hiding this comment

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

Please make a pull request to tipb first

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@AndreMouche the tipb part has done in pingcap/tipb#66 with some other modifications. Thus this pr should be done after #2503 . I'll correct the cargo dependency after #2503 's checkin. The modification here is just to make tests happy.

@spongedu
Copy link
Collaborator Author

spongedu commented Dec 5, 2017

@AndreMouche I'm also working on the tidb part. It's in progress.

@AndreMouche
Copy link
Member

AndreMouche commented Dec 6, 2017 •
edited
Loading

Thanks, I've got it. We would like to review this PR after related PRs merged @spongedu

@spongedu
Copy link
Collaborator Author

@AndreMouche the related PRs have been merged. I've revert the changes in Cargo.lock to fix dependency for tipb. PTAL.
BTW: make test and make works well on my laptop, but ci/circleci fails when build rocksDB. Do you know the reason?

@AndreMouche
Copy link
Member

AndreMouche commented Dec 19, 2017 •
edited
Loading

@hanfei1991 @breeswish PTAL

AndreMouche
AndreMouche reviewed Dec 22, 2017
View reviewed changes
Copy link
Member

@AndreMouche AndreMouche left a comment

Choose a reason for hiding this comment

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

LGTM @hanfei1991 @breeswish PTAL

tests/coprocessor/test_select.rs Outdated
fn max(self, col: Column) -> Select<'a> {
self.aggr_col(col, ExprType::Max)
}


Copy link
Member

Choose a reason for hiding this comment

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

Please remove the redundant empty line

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

AndreMouche
AndreMouche reviewed Dec 25, 2017
View reviewed changes
Copy link
Member

@AndreMouche AndreMouche left a comment

Choose a reason for hiding this comment

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

LGTM @hanfei1991 @breeswish @XuHuaiyu PTAL

@AndreMouche
Copy link
Member

Please fix the conflicts @spongedu

winoros
winoros reviewed Dec 29, 2017
View reviewed changes
Copy link
Contributor

@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

@winoros
Copy link
Contributor

winoros commented Dec 29, 2017

/ok-to-test

@winoros
Copy link
Contributor

winoros commented Dec 29, 2017

/run-all-tests

@AndreMouche AndreMouche merged commit 2249bdd into tikv:master Dec 29, 2017
@spongedu spongedu mentioned this pull request Jan 1, 2018
Merged
sticnarf pushed a commit to sticnarf/tikv that referenced this pull request Oct 27, 2019
@spongedu @AndreMouche
coprocessor: support builtin aggregation function bit_and, `'bit_or…
b856ed3 
…`, `bit_xor`. (tikv#2510)
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndreMouche AndreMouche AndreMouche left review comments

@winoros winoros winoros approved these changes

Assignees
No one assigned
Labels
contribution This PR is from a community contributor.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@spongedu @sre-bot @CLAassistant @AndreMouche @winoros @breezewish