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

coprocessor, dag: add arithmetic operations. #2189

Merged
merged 34 commits into from Aug 24, 2017

Conversation

Projects
None yet
4 participants
@hicqu
Contributor

hicqu commented Aug 18, 2017

No description provided.

@hicqu hicqu requested review from andelf, AndreMouche, BusyJay and disksing Aug 18, 2017

hicqu added some commits Aug 18, 2017

@AndreMouche

This comment has been minimized.

Show comment
Hide comment
@AndreMouche

AndreMouche Aug 19, 2017

Member

Please fix the CI

Member

AndreMouche commented Aug 19, 2017

Please fix the CI

Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated
Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated
Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated
Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated
Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated
Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated
@hicqu

This comment has been minimized.

Show comment
Hide comment
@hicqu

hicqu Aug 22, 2017

Contributor

@BusyJay , PTAL, thanks.

Contributor

hicqu commented Aug 22, 2017

@BusyJay , PTAL, thanks.

hicqu added some commits Aug 22, 2017

Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated
Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated
Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated
Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated

hicqu added some commits Aug 22, 2017

Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated
Show outdated Hide outdated tests/coprocessor/test_select.rs Outdated
Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated

hicqu added some commits Aug 23, 2017

fix
@hicqu

This comment has been minimized.

Show comment
Hide comment
@hicqu

hicqu Aug 23, 2017

Contributor

@BusyJay , PTAL, thanks.

Contributor

hicqu commented Aug 23, 2017

@BusyJay , PTAL, thanks.

Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated
result.map(Cow::Owned).map(Some)
}
pub fn multiply_int(&self, ctx: &StatementContext, row: &[Datum]) -> Result<Option<i64>> {

This comment has been minimized.

@BusyJay

BusyJay Aug 23, 2017

Contributor

Should follow the same pattern as plus_int or minus_int.

@BusyJay

BusyJay Aug 23, 2017

Contributor

Should follow the same pattern as plus_int or minus_int.

This comment has been minimized.

@hicqu

hicqu Aug 23, 2017

Contributor

fixed.

@hicqu

hicqu Aug 23, 2017

Contributor

fixed.

Show outdated Hide outdated src/coprocessor/dag/expr/arithmetic.rs Outdated

hicqu added some commits Aug 23, 2017

@BusyJay

This comment has been minimized.

Show comment
Hide comment
@BusyJay

BusyJay Aug 23, 2017

Contributor

I think u64_mul_i64 is better.

Contributor

BusyJay commented on src/coprocessor/dag/expr/arithmetic.rs in 7f3ef85 Aug 23, 2017

I think u64_mul_i64 is better.

hicqu added some commits Aug 23, 2017

@AndreMouche

Please fix conflicts, rest LGTM

@hicqu hicqu merged commit 4169386 into master Aug 24, 2017

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@hicqu hicqu deleted the qupeng/expr-math branch Aug 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment