Uuid type does not work correctly with MariaDB

[tgross35]

Bug Description

SQLx seems to fail to decode to Uuid on MariaDB. I think this may be because a binary value is expected, but the select type for a UUID column on MariaDB is a hyphenated string.

sqlx/sqlx-mysql/src/types/uuid.rs

Line 37 in 84d5760

Uuid::from_slice(bytes).map_err(Into::into)

Could the column type or length be checked to determine whether to decode a string or byte slice?

Minimal Reproduction

use sqlx::{Connection, Row};

#[tokio::main]
async fn main() {
    let mut conn = sqlx::MySqlConnection::connect(env!("DATABASE_URL"))
        .await
        .unwrap();
    sqlx::query("create table t1 (a uuid)")
        .execute(&mut conn)
        .await
        .unwrap();
    sqlx::query("insert into t1 (a) values (uuid())")
        .execute(&mut conn)
        .await
        .unwrap();

    let row = sqlx::query("select a from t1")
        .fetch_one(&mut conn)
        .await
        .unwrap();
    dbg!(&row);

    // This fails
    let val: uuid::Uuid = row.get(0);

    let row = sqlx::query("select uuid()")
        .fetch_one(&mut conn)
        .await
        .unwrap();
    dbg!(&row);
    // This also fails if the above is removed
    let val: uuid::Uuid = row.get(0);
}

Output:

[foo/src/main.rs:20:5] &row = MySqlRow {
    row: Row {
        storage: b"\0$9bb557bd-c4ba-11ee-8149-0242ac140002",
        values: [
            Some(
                2..38,
            ),
        ],
    },
    format: Binary,
    columns: [
        MySqlColumn {
            ordinal: 0,
            name: a,
            type_info: MySqlTypeInfo {
                type: String,
                flags: ColumnFlags(
                    UNSIGNED | BINARY,
                ),
                char_set: 224,
                max_size: Some(
                    144,
                ),
            },
            flags: Some(
                ColumnFlags(
                    UNSIGNED | BINARY,
                ),
            ),
        },
    ],
    column_names: {
        a: 0,
    },
}
thread 'main' panicked at .cargo/registry/src/index.crates.io-6f17d22bba15001f/sqlx-core-0.7.3/src/row.rs:72:37:
called `Result::unwrap()` on an `Err` value: ColumnDecode { index: "0", source: Error(ByteLength { len: 36 }) }

Also tested with mysql which can't create the table but does have a uuid() function. This gets similar failures.

Info

• SQLx version:
• SQLx features enabled:
  sqlx = { version = "0.7.3", features = ["mysql", "runtime-tokio", "uuid"] }
  tokio = { version = "1.36.0", features = ["full"] }
  uuid = "1.7.0"
• Database server and version: Mariadb 11.2
• Operating system: MacOS
• rustc --version: rustc 1.77.0-nightly (11f32b73e 2024-01-31)

[acidindustries]

Similar situation here, I managed to get around the issue by using #[sqlx(try_from = "Hyphenated"] above the struct member with the Uuid type.

[LawsOfScience]

Hi, I'm encountering a related issue and I'm not sure how to work around it.

Using MariaDB 11.5.2.
My code looks like the following:

#[derive(Debug, Deserialize, Serialize, sqlx::FromRow)]
#[allow(non_snake_case)]
pub struct UserObjectBase {
    #[sqlx(try_from = "Hyphenated")]
    pub UUID: uuid::Uuid,
    // ...
}

I'm using MariaDB's native UUID datatype in the database.

As you can see, I've tried doing #[sqlx(try_from = "Hyphenated")] as suggested above and also tried pulling the changes from #3580, but I'm still getting an error that looks like this:

ColumnDecode { index: \"\\\"UUID\\\"\", source: \"mismatched types; Rust type `uuid::fmt::Hyphenated` (as SQL type `VARCHAR`) is not compatible with SQL type `BINARY`\" }

Removing the #[sqlx(...)] macro gives the error that was added in #3580, and using just uuid::fmt::Hyphenated (I had to remove Serialize and Deserialize to make it compile) gives the same error as above. I'm afraid I didn't read how to solve this correctly and would like to ask how.

[koeninger]

As former committer on a large Apache project, I get the desire for consistency.

But I think it's actively user-hostile to return an error in a situation where it's very obvious that you've been handed a string representation of UUID, when you could instead just parse it.

[LawsOfScience]

where it's very obvious that you've been handed a string representation of UUID, when you could instead just parse it

I think the issue with this isn't that it isn't obvious that there's a string, but that it isn't obvious how it's stored. uuid::fmt supports 4 formats for string-ifying a UUID, and sqlx can't just know which format it is in.
It seems that there is already some mechanism to tell sqlx which string to parse it as (#[sqlx(try_from = ... )]), but that it doesn't work quite right since it still complains about a type mismatch between VARCHAR and BINARY.

If @abonander or another maintainer could comment on this and if the type mismatch I was encountering is intentional or not (and if intentional, how to fix it), that would be great.

[koeninger]

By the time sqlx is in impl Decode<'_, MySql> for Uuid and it got passed something other than 16 bytes, it's obvious that's not a binary UUID. Uuid::parse_str already handles multiple formats.

[abonander]

@LawsOfScience please open a new issue. That's a different bug. It's likely because UUID() returns a text value with a collation/charset that SQLx doesn't recognize as UTF-8 compatible. We've had a ton of headaches with this recently because the only way MySQL differentiates between VARCHAR and BINARY values on the wire is the collation that it uses, and it's a game of whack-a-mole trying to list all of the ones that should work. This is likely caused by or related to #3387.

@koeninger my experience from maintaining this crate for almost 5 years now tells me that making Uuid "just work" on decoding will lead to more confusion when users try to bind one for select/insert/update and get a type mismatch error at runtime (since MySQL doesn't return enough information for query!() to properly typecheck bind parameters).

It's much easier to teach, and for the user to remember, that Uuid == binary encoding and fmt::Hyphenated == text encoding. There's going to be a speedbump regardless. At least in this situation, we have control over the error message that's returned. I doubt the one that MySQL would spit out is nearly as helpful.

If I could go back in time, maybe we'd support only the text format, or use an adapter type for binary format.

Changing Uuid to just exclusively use text format is a major breaking change, and one that would be incredibly hard to warn the user about ahead of time. Just changing it in-place would likely cause a lot of headache and some potentially really problematic bugs, because I've learned the hard way that not everyone reads release notes as thoroughly as they should. And #[deprecated] doesn't work on trait impls.

To be safe, I'd want to drop the trait impls entirely for a full release cycle and force the user to use an adapter type, then maybe reintroduce the impls in a future release using the text format.

But frankly, only supporting text-encoded UUIDs is an incredibly shortsighted decision on the part of the MySQL developers. Taking 36 bytes to store or transmit one ID is just insane. Handling UUIDs in binary is the better choice from a purely technical perspective.

MariaDB's UUID type is a nice band-aid for the storage issue, but it doesn't save us here since it still uses the text encoding for interchange, and thus it still wastes 20 bytes on the wire.

It's for reasons like this why I will never use MySQL over Postgres. It's full of tons of really esoteric and shortsighted design decisions. By comparison, Uuid works pretty much flawlessly in Postgres because it's a natively supported data type. If we only supported Postgres and Tokio, it'd make my life a whole lot easier.

[koeninger]

I don't really understand your point about more confusion on binding; I'd probably need to see an example. I made that patch after a runtime error on query!, so the current main doesn't seem any better in that regard from a user perspective. Everything I've tried to do so far on the patched version works.

Making people put my_column as my_column: uuid::fmt::Hyphenated everywhere and then manually map over results to call into_uuid() on them, which I think is the alternative you're suggesting, doesn't seem particularly discoverable even after the error message change.

It doesn't ultimately matter to me, since we'll keep using a patched version of sqlx to paper over other issues we've run into anyway. Consider it as feedback - there are plenty of other people who will just run into this kind of thing and silently stop using sqlx.

I understand the mysql hate, but Postgres has its share of dumb stuff as well. For instance, lack of unsigned integer types, which is the reason we backed into using MariaDB.