-
Notifications
You must be signed in to change notification settings - Fork 465
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
adapter/storage: Cast MySQL bit
columns to uint8
, add convience functions
#31097
Conversation
677e43e
to
f994fae
Compare
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.
LGTM, but I think we should add docs for the new functions.
src/adapter/BUILD.bazel
Outdated
@@ -24,7 +24,6 @@ rust_library( | |||
compile_data = [], | |||
crate_features = ["default"], | |||
data = [], | |||
disable_pipelining = True, |
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.
Did you mean to include this in this PR? It seems a bit unrelated.
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.
I didn't was rebased on the wrong branch by accident, removed!
.get(byte_index) | ||
.map(|b| (*b >> bit_index) & 1) | ||
.ok_or(err)?; | ||
Ok(Datum::from(i32::from(i))) |
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 I'm understanding correctly, the following might be useful: assert!(i == 0 || i == 1)
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.
Good call!
Good idea, I'll add them in a separate PR though so we can merge them when v0.131 goes out |
f2485a9
to
67c8301
Compare
bit
columns to bytea
, add convience functionsbit
columns to uint8
, add convience functions
67c8301
to
e6b3ef2
Compare
e6b3ef2
to
99fad97
Compare
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.
Thanks for adding tests!
Edit: I added platform checks scenario, green nightly run with it: https://buildkite.com/materialize/nightly/builds/10923
…unctions (#31097) This PR changes the MySQL source to support ingesting the `bit` type as `uint8`. It also adds two new Postgres functions, `bit_count(bytea)` and `get_bit(bytea, int32)` to making working with byte strings easier. ### Motivation Progress towards: MaterializeInc/database-issues#8891 ### Checklist - [x] This PR has adequate test coverage / QA involvement has been duly considered. ([trigger-ci for additional test/nightly runs](https://trigger-ci.dev.materialize.com/)) - [x] This PR has an associated up-to-date [design doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md), is a design doc ([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)), or is sufficiently small to not require a design. <!-- Reference the design in the description. --> - [x] If this PR evolves [an existing `$T ⇔ Proto$T` mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md) (possibly in a backwards-incompatible way), then it is tagged with a `T-proto` label. - [x] If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label ([example](MaterializeInc/cloud#5021)). <!-- Ask in #team-cloud on Slack if you need help preparing the cloud PR. --> - [x] If this PR includes major [user-facing behavior changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note), I have pinged the relevant PM to schedule a changelog post. --------- Co-authored-by: Dennis Felsing <[email protected]>
This PR changes the MySQL source to support ingesting the
bit
type asuint8
. It also adds two new Postgres functions,bit_count(bytea)
andget_bit(bytea, int32)
to making working with byte strings easier.Motivation
Progress towards: https://github.com/MaterializeInc/database-issues/issues/8891
Checklist
$T ⇔ Proto$T
mapping (possibly in a backwards-incompatible way), then it is tagged with aT-proto
label.