Proposed Change: Add unsigned types to MysqlType


#1

Description of the change

Unsigned variants of all numeric types have been added to MysqlType.

Why is this needed?

We released Diesel 1.0 with completely broken support for unsigned types in MySQL. There’s an unsigned flag which we need to set separately to indicate that data should be interpreted as unsigned. If we don’t set this flag, MySQL will ignore all type information it has, and assume we gave it a signed integer, erroring out if we try to use a negative number in an unsigned context.

We shipped a backwards compatible solution in https://github.com/diesel-rs/diesel/pull/1751, but this was a hack that involves a bunch of undocumented methods which anyone manually implementing HasSqlType might accidentally break. It’s also just ugly, and the incompatible solution proposed in https://github.com/diesel-rs/diesel/pull/1749 is much cleaner. To even get the core team to sign off on that change, I had to make it clear that #1751 was temporary and intended to be removed. So… Let’s do that. Because it was terrible.

Why can’t this have a deprecation cycle?

This involves adding variants to an enum which doesn’t have a non-exhaustive guard on it. Similar to most of these sort of changes, the only way we could deprecate this would be to deprecate the entire Mysql backend and introduce a Mysql2 backend with this change, which would change this from a change expected to impact exactly 0 users into one that impacts everyone using MySQL with Diesel.

Expected user impact

Nobody. The only way someone could be impacted by this is to be exhaustively matching on MysqlType. I can’t think of any reason to do that unless you’re implementing a custom MySQL implementation. I’m not aware of any.

Migration strategy

Pretty much the same as https://github.com/diesel-rs/diesel/pull/1749. Anyone matching on MysqlType will need to handle these new variants, or add a wildcard case. If you’re matching on this enum at all, it’s unlikely that a wildcard case will actually work (you can’t just treat an unsigned integer as text and expect that to be fine).

Unresolved questions

Should we make this enum non-exhaustive? Missing the interaction with the unsigned flag was a legit mistake that should have been caught prior to 1.0, but I’m confident that we now have all of them. The only reason we would add new variants to this enum is if MySQL added new types to it on their end. There isn’t any policy on this, but they’ve historically made all new types be transmitted as Text rather than adding a new representation on the client side.

I’m pretty liberal about making enums non-exhaustive generally speaking, but this is one that I think really benefits from allowing exhaustive matches. Literally the only reason I can come up with to match on this is if you’re writing a custom MySQL implementation (which is reasonable for a separate Percona/MariaDB impl that handles minor differences more than we do in a way that doesn’t justify a full new backend). Every case I can think of for someone to match on this enum at all can’t have a reasonable wildcard case, so I think there’s a lot of value in continuing to leave this enum exhaustive.