-
Notifications
You must be signed in to change notification settings - Fork 67
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
RFC: Using chunk format in coprocessor framework #43
Conversation
text/2020-03-07-copr-chunk.md
Outdated
```rust | ||
#[rpn_fn] | ||
pub fn md5(arg: Bytes, writer: Writer<Bytes>) -> Result<Guard> { | ||
writer.write(hex_digest(MessageDigest::md5(), arg)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that in this form, it is possible to first write something, then return an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If Err is returned, we should call discard in framework
text/2020-03-07-copr-chunk.md
Outdated
|
||
## Summary | ||
|
||
Using [TiDB Chunk format](https://github.com/pingcap/tidb/blob/9627132db25e5cb560ba6ed89c2e39c32edb267a/util/chunk/chunk.go) in the `coprocessor` framework instead of `VectorValue`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Links are not unified, and are duplicated.
text/2020-03-07-copr-chunk.md
Outdated
|
||
The definition of `Writer` and `PartialWriter` ensures some properties: | ||
|
||
1. `Guard` is zero-overhead, and the only way to achieve a `Guard` is calling `write` or `finish`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to mention that Guard
ensures write
or finish
has been called at least once in the implementation of the coprocessor. I think the name is confusing. Guard
does not guard any resources. It is proof that write
or finish
has been called. Maybe we can call it evidence
/witness
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@breeswish PTAL
text/2020-03-07-copr-chunk.md
Outdated
|
||
// To | ||
#[rpn_fn] | ||
pub fn logical_and(lhs: &i64, rhs: &i64) -> Result<Option<i64>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a question. For fixed sized type, is it better to use value type instead of ref? almost all known fixed type <= 8 byte.
PTAL, thanks! @TennyZhuang @breeswish |
e582fd2
to
14676e2
Compare
Signed-off-by: TennyZhuang <zty0826@gmail.com> Co-authored-by: Alex Chi <iskyzh@gmail.com> Co-authored-by: Wenxuan <hi@breeswish.org>
This RFC has been already implemented. cc @andylokandy could you please give an approval and get this merged? |
Signed-off-by: TennyZhuang zty0826@gmail.com