-
Notifications
You must be signed in to change notification settings - Fork 109
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
Serialization refactor: macros for value/row serialization #851
Conversation
d076162
to
bcd3b16
Compare
bcd3b16
to
0b95f92
Compare
v2:
|
0b95f92
to
4ea5a69
Compare
v2.1: address clippy's complaint |
4ea5a69
to
dc58365
Compare
dc58365
to
571b31c
Compare
v2.2:
|
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 left a few questions - they are about commits belonging to #862 which I (mistakenly) thought I was reviewing, sorry. I'll review 4 commits of this PR shortly
No problem, my bad - the description of the other PR was misleading. |
571b31c
to
cf63068
Compare
v3:
|
480b5e2
to
938ed26
Compare
v4:
@Lorak-mmk - I manually checked that the test you commented out in your PR now compiles, so you can rebase. |
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.
Looks good - but maybe add a test for generics so that we have an actual test for that beside the code in examples/
?
Introduce a derive macro which serializes a struct into a UDT. Unlike the previous IntoUserType, the new macro takes care to match the struct fields to UDT fields by their names. It does not assume that the order of the fields in the Rust struct is the same as in the UDT.
Introduce a derive macro which serializes a struct into bind markers of a statement. Unlike the previous ValueList, the new macro takes care to match the struct fields to bind markers/columns by their names.
Some users might not need the additional robustness of `SerializeCql` that comes from sorting the fields before serializing, as they are used to the current behavior of `Value` and properly set the order of the fields in their Rust struct. In order to give them some performance boost, add an additional mode to `SerializeCql` called "enforce_order" which expects that the order of the fields in the struct is kept in sync with the DB definition of the UDT. It's still safe to use because, as the struct fields are serialized, their names are compared with the fields in the UDT definition order and serialization fails if the field name on some position is mismatched.
Like in the case of `SerializeRow`, some people might be used to working with the old `ValueList` and already order their Rust struct fields with accordance to the queries they are used with and don't need the overhead associated with looking up columns by name. The `enforce_order` mode is added to `SerializeRow` which works analogously as in `SerializeCql` - expects the columns to be in the correct order and verifies that this is the case when serializing, but just fails instead of reordering if that expectation is broken.
938ed26
to
a255d81
Compare
Done. v4.1: added some minimal tests for UDTs/rows with generics in them, mostly to see whether they compile or not |
Introduce derive macros for both of the new serialization traits:
SerializeCql
andSerializeRow
. They allow to serialize given Rust struct either into a UDT value or a list of values for a query.The most significant difference from the previous
IntoUserType
andValueList
is that the new macros match the Rust struct fields to UDT fields or columns by name. This makes the macros less error-prone to use, especially in the UDT case.For users which are already used to the old macros and are willing to guarantee the field order but do not want to pay performance cost associated with sorting the fields, a separate "flavor" is introduced. With that flavor enabled, the generated code expects that the field order is correct. The field names are still checked - if the expectation about the matching fields is broken serialization will just fail.
Depends on #862
Refs: #802
Pre-review checklist
./docs/source/
.Fixes:
annotations to PR description.