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

Store Uuid as Bytes instead of String #95

Open
bottiger opened this issue Feb 20, 2020 · 10 comments
Open

Store Uuid as Bytes instead of String #95

bottiger opened this issue Feb 20, 2020 · 10 comments
Labels
help wanted Extra attention is needed

Comments

@bottiger
Copy link

bottiger commented Feb 20, 2020

💡 Store Uuid as bytes

The current support for Uuid's store the Uuid's as a hyphenated string. However, this encoding is obviously a bit verbose since a string is a little more than twice as large as the 128 bit value the Uuid represents - without any clear benefit. For a database with large documents this will be a non issue, but for a large collection of small documents this might introduce a significant overhead.

Backwards compatibility for existing users is an issue though.

💻 Basic example

Just save the Uuid as a binary blob (base64 encoded in the API) instead of a string.

// A `String` type for `Uuids`, represented by the `S` AttributeValue type
#[cfg(feature = "uuid")]
impl Attribute for Uuid {
    fn into_attr(self: Self) -> AttributeValue {
        Bytes::copy_from_slice(self.as_bytes()).into_attr()
    }

PS: This code snippet requires Bytes 0.5 in contrast to Bytes 0.4 which dynomite currently uses.

@softprops
Copy link
Owner

I like where you're going with this. What would be the suggested migration path for those already using a string attribute type for their keys?

@bottiger
Copy link
Author

Unfortunately it is unclear to me what the best way forward is. It is not obvious to me how you upgrade an existing database (assuming you are using the Uuid as the partition key), so I think the best way would simply be to support both. Perhaps is can be implemented using a cargo feature where users can opt in to using the legacy string representation.

@softprops
Copy link
Owner

Perhaps is can be implemented using a cargo feature where users can opt in to using the legacy string representation.

That's an excellent idea. Would you be up for submitting or pr on this. I'd be happy to merge and publish a new version.

@bottiger
Copy link
Author

@softprops I can try that. But my time is currently very limited, so it will be whenever I have a small gap

@phrohdoh
Copy link
Contributor

phrohdoh commented May 22, 2020

Perhaps is can be implemented using a cargo feature where users can opt in to using the legacy string representation.

Definitely don't force a particular storage representation on users. Some may not want bytes for whatever reason.

@softprops
Copy link
Owner

I'd imagine the way to do this if someone wants to pick this up is with a cargo feature flag defaulting to the current format

@softprops softprops added the help wanted Extra attention is needed label May 22, 2020
@rimutaka
Copy link

rimutaka commented Jul 1, 2020

@softprops , I'm learning Rust and would give it a try with some guidance from you. I do use UUIDs a lot and store them as UUID type in Postgres, which is bytes.
Is this the right place to make the change?

impl Attribute for Uuid {

I'll see if I can wrap my head around it. No pressure, though - you asked for help, not an apprentice :)

@softprops
Copy link
Owner

Thanks!

You are off to a great start already. That's where I would be looking.

Ideally we'd want to do with this with a cargo feature flag with the default the current behavior to avoid breaking existing applications. Let me know if you get stuck or need more direction. Happy to help

@rimutaka
Copy link

rimutaka commented Jul 1, 2020

This feature may cause more problems than it solves. The same document may have multiple UUID properties. Some can be stored as bytes to save space, some must be stored as strings, for example for front end scripts in JS. Having a "feature" to choose one or the other will not let us discriminate between the two within the same document.

There must be some other way, e.g. #[dynomite(as_bytes)].

@softprops
Copy link
Owner

The feature can be implemented inside the Attribute impl for Uuid since it's the one place uuids are serialized and deserialized to dynamodb attribute values for any given application so a single item having multiple uuid fields is not a problem.

If you are using serde to serialize a struct to JavaScript you can use serde to serialize in the format your front end expects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants