Sqlx treats mysql mediumint as u8

[norbertkeri]

I'm very new to both rust and sqlx, so I might be wrong here, but it seems like sqlx tries to convert mysql mediumint columns to rust u8s:

struct Forum {
    forum_posts: u8,
}

#[async_std::main]
async fn main() -> Result<(), sqlx::Error> {
    let mut conn = MySqlConnection::connect("mysql://root:root@127.0.0.1/xy").await?;

    let forums = sqlx::query_as!(Forum, "SELECT forum_posts FROM nuke_bbforums").fetch_all(&mut conn).await?;
}

Table:

  +--------------------+--------------------+------+-----+---------+----------------+
  | Field              | Type               | Null | Key | Default | Extra          |
  +--------------------+--------------------+------+-----+---------+----------------+
  | forum_posts        | mediumint unsigned | NO   |     | 0       |                |
  +--------------------+--------------------+------+-----+---------+----------------+

Based on this, the above should be a compile error, since the maximum for u8 is 255, but the maximum for unsigned mediumint is 16777215.

If I replace the the query!() with query_as!(), it panics:

#[async_std::main]
async fn main() -> Result<(), sqlx::Error> {
    let mut conn = MySqlConnection::connect("mysql://root:root@127.0.0.1/xy").await?;

    let forums = sqlx::query!("SELECT forum_posts FROM nuke_bbforums").fetch_all(&mut conn).await?;

    for f in forums {
    }
}

Error: ColumnDecode { index: "1", source: TryFromIntError(()) }

My guess is, it's because the numbers in that column are higher than 255, and it fails when trying to cast to u8.

[abonander]

The problem is that we treat all integer widths as compatible with each other which isn't strictly correct. Sure, we can do truncation and sign extension but TypeInfo::compatible() should only return true for integers of the same width or wider, not narrower.

[abonander]

If you're using 0.4.0-beta.1 you can override this behavior by doing select forum_posts as `forum_posts: u32` ... which will force the type to be u32.

[norbertkeri]

Thanks, this seems to work.

Is this the planned behavior, or is it just something that hasn't been addressed yet?

[LinusU]

Ran into this problem today as well, I have a table with a field id that is mediumint unsigned NOT NULL. Running the following query returns the error:

sqlx::query!(
        "SELECT id FROM mytable WHERE id = ?",
        33457
    )
    .fetch_one(&pool)
    .await?;

Error: error occurred while decoding column 0: out of range integral type conversion attempted

Caused by:
    out of range integral type conversion attempted

It works correctly for columns that are int unsigned NOT NULL, so it seems to be something specifically with medium int that maps to u8 instead of ~u16 🤔~ edit: just learned that mediumint in MySQL is 24 bit 😅 but would probably be best to map to u32 then?

more edit:

It seems like the code treats both int and mediumint as 4 bytes, so not sure where the problem originates 🤔

https://github.com/launchbadge/sqlx/blob/e10789d9d76cbff2755f977b7a126bd67a1ec5e5/sqlx-mysql/src/protocol/statement/row.rs#L54-L57

[CommanderStorm]

I think the issue might lie here instead (sorry if this is a red herring), in the Type::compatible() which is overrridden for for mysql (for example) here:

https://github.com/launchbadge/sqlx/blob/e10789d9d76cbff2755f977b7a126bd67a1ec5e5/sqlx-mysql/src/types/int.rs#L26-L28

https://github.com/launchbadge/sqlx/blob/e10789d9d76cbff2755f977b7a126bd67a1ec5e5/sqlx-mysql/src/types/int.rs#L10-L19

[abonander]

Yeah, so what's happening is this:

The query macros are made aware of supported types via impl_type_checking!{}, which for MySQL is done here: https://github.com/launchbadge/sqlx/blob/e10789d9d76cbff2755f977b7a126bd67a1ec5e5/sqlx-mysql/src/type_checking.rs

In the expansion of impl_type_checking!{}, it first checks if the type passed in is equal to one of the types listed, and if so, returns that: https://github.com/launchbadge/sqlx/blob/e10789d9d76cbff2755f977b7a126bd67a1ec5e5/sqlx-core/src/type_checking.rs#L157-L165

Only after those cases fall through does it check TypeInfo::compatible().

The implementation of compatible() is deliberately broad here, because people may have legitimate use-cases for decoding a smaller type than that suggested by the column definition. Although I can't think of any specific discussions to cite, I believe it was originally supposed to be more strict but we actually loosened it at someone's request.

The type checking works just fine for other integer types because they have direct mappings to Rust types, so one of the equality cases is always hit instead of any the compatible() cases, as intended.

mediumint is an edge case because it has no directly corresponding Rust type. i32 is closest, but i32 directly maps to int because it's a full 32-bit integer.

So the equality cases fall through, and it picks the first type that returns true for compatible(), which is u8 because it's first in the list. The checking of the UNSIGNED flag in int_compatible() is erroneous, but that's not really the problem here. That's just why it's u8 and not i8.

A simple fix would be to re-order the list to put either u32/i32 or u64/i64 first, so the fall-through picks a reasonable type. Also compatible() should be fixed to check the UNSIGNED flag properly; I believe there was originally supposed to be a uint_compatible() function to go along with int_compatible() but it's probably worth re-thinking that approach entirely.

Idiomatically, I might also recommend the creation of wrapper types for mediumint and mediumint unsigned that enforce the correct value range. It would be useful especially for encoding since we could provide a checked constructor that could provide feedback sooner than during execution of the query, e.g. in parsing/validation of a request.

Strictly speaking, any change here would be breaking because it'd affect the type inference of the macros. The code being emitted currently is not really correct, but we can't assume that no one must be depending on the current behavior just because it's wrong. Maybe they thought the u8 mapping was weird, but they never decode any value larger than 255 anyway, so they just rolled with it. It's impossible to know.

As a workaround, you can use the type override syntax: https://docs.rs/sqlx/latest/sqlx/macro.query.html#force-a-differentcustom-type