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.

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: add batch aggregate function BitAnd/BitOr/BitXor #4824

Merged
merged 11 commits into from Jun 6, 2019

Conversation

Projects
None yet
4 participants
@lonng
Copy link
Contributor

commented Jun 3, 2019

Signed-off-by: Lonng heng@lonng.org

What have you changed? (mandatory)

Add batch aggregate function BitAnd/BitOr/BitXor. All related CAST functions don't contain in this PR.

What are the type of the changes? (mandatory)

The currently defined types are listed below, please pick one of the types for this PR by removing the others:

  • New feature (change which adds functionality)

How has this PR been tested? (mandatory)

  • unit tests

@lonng lonng added the C: Copr label Jun 3, 2019

@lonng lonng force-pushed the lonng:batch/bit-and branch from 3ace745 to 68f6b17 Jun 3, 2019

@breeswish breeswish referenced this pull request Jun 3, 2019

Merged

Batch Top N Layered Benchmarks #4827

1 of 1 task complete
@lonng

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2019

/run-all-tests

lonng added some commits Jun 3, 2019

coprocessor: add batch aggregate function BitAnd/BitOr/BitXor
Signed-off-by: Lonng <heng@lonng.org>
coprocessor: add filed type check for child expression
Signed-off-by: Lonng <heng@lonng.org>

@lonng lonng force-pushed the lonng:batch/bit-and branch from 2db87f6 to f0297a3 Jun 5, 2019

coprocessor: refactor
Signed-off-by: Lonng <heng@lonng.org>

@lonng lonng force-pushed the lonng:batch/bit-and branch from f0297a3 to f69334b Jun 5, 2019

@lonng

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2019

Show resolved Hide resolved src/coprocessor/dag/aggr_fn/impl_bit_op.rs Outdated
Show resolved Hide resolved src/coprocessor/dag/aggr_fn/impl_bit_op.rs
out_schema.push(
FieldTypeBuilder::new()
.tp(FieldTypeTp::LongLong)
.flag(FieldTypeFlag::UNSIGNED)

This comment has been minimized.

Copy link
@breeswish

breeswish Jun 5, 2019

Member

I guess this schema can be built from aggr_def.field_type

This comment has been minimized.

This comment has been minimized.

Copy link
@breeswish

breeswish Jun 5, 2019

Member

I mean you can directly use aggr_def.field_type instead of build a new one

Show resolved Hide resolved src/coprocessor/dag/aggr_fn/parser.rs Outdated
address comment
Signed-off-by: Lonng <heng@lonng.org>
state.push_result(&mut ctx, &mut result).unwrap();
assert_eq!(
result[0].as_int_slice(),
&[Some(0xffffffffffffffff as u64 as i64)]

This comment has been minimized.

Copy link
@sticnarf

sticnarf Jun 5, 2019

Contributor

You may write 0xffffffffffffffffu64 as i64.
(Same below)

This comment has been minimized.

Copy link
@kennytm

kennytm Jun 5, 2019

Contributor

😕 Then why not just write -1_i64

This comment has been minimized.

Copy link
@sticnarf

sticnarf Jun 5, 2019

Contributor

Maybe it's to emphasize that the output result of bit ops is actually an unsigned int? I think just -1 is also ok...

This comment has been minimized.

Copy link
@lonng

lonng Jun 5, 2019

Author Contributor

@kennytm 0xffffffffffffffff is more clear.

This comment has been minimized.

Copy link
@kennytm

kennytm Jun 5, 2019

Contributor

@lonng in that case I'd suggest 0xffff_ffff_ffff_ffff_u64 to clearly show that there are 16 fs (IIRC Clippy will complain if you don't insert those _ anyway).

// 13 | 2 == 15
state.update(&mut ctx, &Some(2i64)).unwrap();
state
.update_vector(&mut ctx, &[Some(2i64), None, Some(1i64)])

This comment has been minimized.

Copy link
@sticnarf

sticnarf Jun 5, 2019

Contributor

It does not do the same thing as the comment ("13 | 2 == 15") says...

This comment has been minimized.

Copy link
@lonng

lonng Jun 5, 2019

Author Contributor

@sticnarf I think there is no need to comment above update_vector.

lonng added some commits Jun 5, 2019

address comment
Signed-off-by: Lonng <heng@lonng.org>
address comment
Signed-off-by: Lonng <heng@lonng.org>
EvalType::Int => {
return Ok(());
}
_ => FieldTypeBuilder::new()

This comment has been minimized.

Copy link
@breeswish

breeswish Jun 5, 2019

Member

Looks like we will meet some duplicate code here. We build a field type when it is used in schema, and build another one when it is used to cast. We can refine this in future PRs, for all aggregate functions.

This comment has been minimized.

Copy link
@lonng

lonng Jun 6, 2019

Author Contributor

OK

address comment
Signed-off-by: Lonng <heng@lonng.org>
@breeswish

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

Looks like CI failed because of warnings. I will re-approve once it is fixed.

breeswish and others added some commits Jun 6, 2019

address warning
Signed-off-by: Lonng <heng@lonng.org>
@breeswish

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

/run-integration-tests

@lonng

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2019

/run-integration-common-test

@lonng lonng merged commit fc838a2 into tikv:master Jun 6, 2019

2 checks passed

DCO All commits are signed off!
Details
idc-jenkins-ci/test Jenkins job succeeded.
Details

@lonng lonng deleted the lonng:batch/bit-and branch Jun 6, 2019

sticnarf added a commit to sticnarf/tikv that referenced this pull request Jun 10, 2019

Remove all Box of RPN functions
Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Makefile: make sure gdb is installed before sse4.2 check (tikv#4832)

Signed-off-by: Kaige Ye <ye@kaige.org>

Upgrade sys-info (tikv#4760)

* *: upgrade sys-info crate

This fixes a problem with the next toolchain upgrade
where rust fails to link the native components of the crate.

Signed-off-by: Brian Anderson <andersrb@gmail.com>

* *: bump sys-info to 0.5.7

Signed-off-by: Brian Anderson <andersrb@gmail.com>

Batch Top N Executor (tikv#4825)

Signed-off-by: Breezewish <breezewish@pingcap.com>

Add help message in doc:go-client-api.md (tikv#4763)

* add help message in doc:go-client-api.md

Signed-off-by: yy <cacheyy@qq.com>

* update go-client-api.md

Signed-off-by: yy <cacheyy@qq.com>

Modify Makefile to distinguish between developer and packaging use cases (tikv#4687)

* make: Add new "dist_release" rules

To make the optimized build faster the existing "release" rules are going to
changed such that they are not identical to the actual releases. Primarily they
will not have debuginfo by default and will use thinLTO instead of LTO.

This adds new "dist_release", etc rules for the CI/CD system to use.

For now they are identical to the existing rules. After CI is updated
the "release" rules will be changed.

Signed-off-by: Brian Anderson <andersrb@gmail.com>

* make: Document release rules

Signed-off-by: Brian Anderson <andersrb@gmail.com>

* Makefile: indicate use of fail_release

Signed-off-by: Brian Anderson <andersrb@gmail.com>

* Clarify the distinction in instruction set for release targets

Signed-off-by: Brian Anderson <andersrb@gmail.com>

Makefile: fix gdb check (tikv#4840)

Signed-off-by: Kaige Ye <ye@kaige.org>

pessimistic-txn: solve non-pessimistic-lock conflict (tikv#4801)

* txn: replace is_pessimistic_lock to for_update_ts in Lock

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

* pessimistic-txn: overwrite optimistic lock in pessimistic_prewrite if
request's for_update_ts is greater than lock's for_update_ts

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

* modify comment

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

* address comment

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

* address comment

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

* address comment

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

* address comment

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

* add comment

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

* return Error let TiDB to resolve lock

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

* address comment

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

* address comment

Signed-off-by: youjiali1995 <zlwgx1023@gmail.com>

coprocessor: add batch aggregate function BitAnd/BitOr/BitXor (tikv#4824)

Batch Top N Layered Benchmarks (tikv#4827)

* Add Top N benchmarks

Signed-off-by: Breezewish <breezewish@pingcap.com>

* Address some comments in previous PRs

Signed-off-by: Breezewish <breezewish@pingcap.com>

coprocessor: add batch aggregate function Max/Min (tikv#4837)

Implement RpnFunction MultiplyDecimal (tikv#4849)

Signed-off-by: Breezewish <breezewish@pingcap.com>

Add missing fsync calls in the snapshot module (tikv#4850)

Signed-off-by: Ben Pig Chu <benpichu@gmail.com>

use HTTP to enable jemalloc profile (tikv#4600)

use HTTP to enable jemalloc profile

Signed-off-by: Yang Keao <keao.yang@yahoo.com>

coprocessor: use servo_arc in BatchTopNExecutor (tikv#4854)

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Fix clippy warnings

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Fix test

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Add docs to function.rs

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

Add example output of the macro in the test of the macro.

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

fix broken url for configuration options (tikv#4856)

Signed-off-by: Yukang <moorekang@gmail.com>

shrink the latch waiting list (tikv#4844)

Signed-off-by: zhangjinpeng1987 <zhangjinpeng@pingcap.com>

Fix clippy

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

scheduler use spin::Mutex (tikv#4829)

* scheduler use spinlock

Signed-off-by: zhangjinpeng1987 <zhangjinpeng@pingcap.com>

Better panic info

Signed-off-by: Yilin Chen <sticnarf@gmail.com>

breeswish added a commit to breeswish/tikv that referenced this pull request Jun 12, 2019

breeswish added a commit to breeswish/tikv that referenced this pull request Jun 12, 2019

coprocessor: add batch aggregate function BitAnd/BitOr/BitXor (tikv#4824
)

Signed-off-by: Breezewish <breezewish@pingcap.com>

breeswish added a commit to breeswish/tikv that referenced this pull request Jun 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.