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::fmt::Hyphenated as sqlx::Type<MySql>::compatible() is wrong? #3409

Open
AlphaKeks opened this issue Aug 6, 2024 · 5 comments
Open
Labels
bug db:mysql Related to MySQL

Comments

@AlphaKeks
Copy link
Contributor

Bug Description

I'm running MariaDB and have a table with a UUID column. When trying to fetch a row from this table and decoding the column into uuid::Uuid, it complains saying it expected 16 bytes, but got 36. This sort of makes sense, as I inserted the row using a string literal of a hyphenated UUID. When I try to use uuid::fmt::Hyphenated instead, it says the types aren't compatible. When using the query!() macro, it infers the type to be Vec<u8>, and decoding that as UTF-8 yields the expected string value I originally inserted. Maybe I'm just using these types incorrectly?

From what I can see in sqlx-mysql, uuid::Uuid expects 16 bytes, and uuid::fmt::Hyphenated just forwards to str? But str says it isn't compatible with anything that has a BINARY flag set, which the returned column does in my case...

Minimal Reproduction

https://github.com/AlphaKeks/sqlx-uuid-repro

What I would expect to happen is that either of the two queries succeed, but they both fail with the errors mentioned above.

Info

  • SQLx version: 0.8.0
  • SQLx features enabled: ["macros", "mysql", "runtime-tokio-rustls", "uuid"]
  • Database server and version: MariaDB 11.0
  • Operating system: Linux
  • rustc --version: rustc 1.80.0 (051478957 2024-07-21)
@AlphaKeks AlphaKeks added the bug label Aug 6, 2024
@abonander
Copy link
Collaborator

Likely related to #3387.

@DrewMcArthur
Copy link

DrewMcArthur commented Sep 16, 2024

Interesting, so running your example @AlphaKeks, I get

Failed to fetch `FooUuid`: error occurred while decoding column "bar": invalid length: expected 16 bytes, found 36
Failed to fetch `FooHyphenated`: error occurred while decoding column "bar": mismatched types; Rust type `uuid::fmt::Hyphenated` (as SQL type `VARCHAR`) is not compatible with SQL type `BINARY`

then when I point the sqlx dependency at my branch, I get:

Failed to fetch `FooUuid`: error occurred while decoding column "bar": invalid length: expected 16 bytes, found 36
Fetched `FooHyphenated`: FooHyphenated { bar: Hyphenated(a107320d-ad7e-40f5-98e5-aa0e15171bc0) }

so #3400 fixes the second test case, but not the first. I wonder if it's something related to how you're inserting the UUID?

you can reproduce by removing the sqlx row in your Cargo.toml dependencies section and adding this to the end:

[dependencies.sqlx]
version = "0.8"
features = [
    "macros",
    "mysql",
    "runtime-tokio-rustls",
    "uuid",
]
git = "https://github.com/DrewMcArthur/sqlx"
branch="3387-bin-collation-compatibility"

I'll look into this a bit further later, but if you get a chance before I do, let me know if that branch fixes your issue!

@DrewMcArthur
Copy link

DrewMcArthur commented Sep 18, 2024

so #3400 fixes the second test case (Hyphenated), but not the first. Looks like an issue with the impl Decode for Uuid block.

changing it to this fixes the test case (and a case I added, which empties the table, adds a fresh row, SELECTs and compares, to make sure it isn't an issue with the insertion)

impl Decode<'_, MySql> for Uuid {
    fn decode(value: MySqlValueRef<'_>) -> Result<Self, BoxDynError> {
        // delegate to the &[u8] type to decode from MySQL
        let slice = <&[u8] as Decode<MySql>>::decode(value)?;
        let bytes = String::from_utf8(Vec::from(slice))?;

        // construct a Uuid from the returned bytes
        Uuid::from_str(&bytes).map_err(Into::into)
    }
}

now, obviously this isn't ideal since it's pulling a byte slice from the DB, converting that to a Vec, converting that to a String, then finally going to a Uuid.

@abonander is this right, does the current impl Decode for Uuid block not actually work? the test cases I found for this is the codebase look like they don't cover this (create a Uuid, insert it, select, compare), only a smaller surface of the Uuid::parse_str from what I can tell (which is eventually being deprecated for try_parse anyways).

seems to me like we should be storing Uuids as a blob of 16 bytes, but fetching from the DB yields 36 bytes. i don't yet understand what's going on under the hood, but it does seem to be like these encode and decode methods could be redone?

here's some more verbose output:

encoded to buf [16, 19, 55, 28, 146, 146, 22, 69, 107, 139, 164, 40, 97, 69, 0, 164, 32] len: 17
✓ Inserted new `Foo`: 13371c92-9216-456b-8ba4-28614500a420
decoding value: MySqlValueRef { value: Some([49, 51, 51, 55, 49, 99, 57, 50, 45, 57, 50, 49, 54, 45, 52, 53, 54, 98, 45, 56, 98, 97, 52, 45, 50, 56, 54, 49, 52, 53, 48, 48, 97, 52, 50, 48]), row: Some(b"\0$13371c92-9216-456b-8ba4-28614500a420"), type_info: MySqlTypeInfo { type: String, flags: ColumnFlags(NOT_NULL | UNSIGNED | BINARY | NO_DEFAULT_VALUE), collation: utf8mb4_unicode_ci, max_size: Some(144) }, format: Binary }
found 36 bytes: [49, 51, 51, 55, 49, 99, 57, 50, 45, 57, 50, 49, 54, 45, 52, 53, 54, 98, 45, 56, 98, 97, 52, 45, 50, 56, 54, 49, 52, 53, 48, 48, 97, 52, 50, 48]
✓ Fetched `FooUuid`: FooUuid { bar: 13371c92-9216-456b-8ba4-28614500a420 }

maybe this is more of the collation issue after all... i'll keep digging 😄

@DrewMcArthur
Copy link

Here is a slightly cleaner impl block; seems like the value is being stored as a VARCHAR(32) rather than BINARY(16), which yields the wrong array of u8s with the previous code.

impl Decode<'_, MySql> for Uuid {
    fn decode(value: MySqlValueRef<'_>) -> Result<Self, BoxDynError> {
        // delegate to the &str type to decode from MySQL
        let slice = <&str as Decode<MySql>>::decode(value)?;

        // construct a Uuid from the returned bytes
        Uuid::from_str(slice).map_err(Into::into)
    }
}

@DrewMcArthur
Copy link

what I still don't understand is why encode puts the BINARY(16) representation into the buf, but the value we get in decode isn't that same buf, but a utf8-encoded (decoded?) 32 byte array (hence the error @AlphaKeks got originally)...

<Uuid as Type<MySql>>::type_info() returns <&[u8] as Type<MySql>>::type_info(), which returns MySqlTypeInfo::binary(ColumnType::Blob). hm...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug db:mysql Related to MySQL
Projects
None yet
Development

No branches or pull requests

3 participants