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

UUID does not match #26

Closed
18601673727 opened this issue Dec 1, 2021 · 5 comments
Closed

UUID does not match #26

18601673727 opened this issue Dec 1, 2021 · 5 comments
Labels
bug Something isn't working

Comments

@18601673727
Copy link

use serde::{Deserialize, Serialize};

#[derive(clickhouse::Row, Debug, Serialize, Deserialize)]
pub struct Test {
    #[serde(with = "uuid::serde::compact")]
    pub id: uuid::Uuid,
}

let mut inserter = db.insert("tests")?;
inserter.write(&Test { id: Uuid::new_v4() }).await?; // id: cb5f628b-0f76-4e3e-a310-9b15cb9d29ee
inserter.end().await?; // success

// later in clickhouse-client do:
// select * from tests;

┌─id───────────────────────────────────┐
│ 3e4e760f-8b62-5fcb-ee29-9dcb159b10a3 │
└──────────────────────────────────────┘
@zryambus
Copy link

zryambus commented Nov 8, 2022

I have a some tricky solution, but it can be improved:

#[derive(Debug)]
pub struct UUID(pub uuid::Uuid);

impl From<uuid::Uuid> for UUID {
    fn from(value: uuid::Uuid) -> Self {
        Self { 0: value }
    }
}

fn uuid_to_bytes_be(id: &uuid::Uuid) -> [u8; 16] {
    let bytes = id.as_bytes();
    let mut result: [u8; 16] = [0; 16];
    for i in 0..16 {
        let multiplier = i / 8;
        let j = i % 8;
        let index = 8 * multiplier + 7 - j;
        result[index] = bytes[i];
    }
    result
}

impl Serialize for UUID {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: serde::Serializer
    {
        let bytes = uuid_to_bytes_be(&self.0);
        serde::Serialize::serialize(&bytes, serializer)
    }
}

The idea is to move the bytes in the next order

Before  0 1 2 3 4 5 6 7   8 9 10 a b c  d e f
After   7 6 5 4 3 2 1 0   f e  d c b a 10 9 8

After this you can pass UUID object to write method of Inserter and this should work as expected

@18601673727
Copy link
Author

Thanks @zryambus !

@loyd loyd reopened this Nov 9, 2022
@loyd
Copy link
Collaborator

loyd commented Nov 9, 2022

It obviously must be handled in this library, so I've reopened the issue.
Variants:

  • Move to RowBinaryWithNamesAndTypes (Use RowBinaryWithNamesAndTypes #10) and detect UUID types by CH. However, it works well for deserialization, but serialization requires fetching an actual schema of a table.
  • Check a struct name in the (de)serializer. It hurts the performance of all newtypes and has false positives.
  • Provide the uuid feature and the wrapper around, as above.
  • Provide a custom Uuid type and conversions to uuid::Uuid under the uuid feature.
  • Provide the clickhouse::serde::uuid module with ser/de impl under the uuid feature to use with serde(with = ..).

What do you think about variants?

@zryambus
Copy link

zryambus commented Nov 9, 2022

I think that last two variants are pretty good.
Before #26 will be resolved, it will be great to add to readme the note, that there are some issues with serialization of uuid::Uuid type

@loyd loyd closed this as completed in 06e5616 Nov 10, 2022
@loyd
Copy link
Collaborator

loyd commented Nov 10, 2022

Published in v0.11.0

@loyd loyd added the bug Something isn't working label Aug 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants