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

Fix encoding and decoding of MySQL enums in sqlx::Type #3371

Merged
merged 5 commits into from
Jul 25, 2024

Conversation

alu
Copy link
Contributor

@alu alu commented Jul 24, 2024

fixes #1241

@abonander
Copy link
Collaborator

abonander commented Jul 25, 2024

As it turns out, we didn't actually have a comprehensive test covering enum encoding and decoding for MySQL, so it's not clear if it ever actually worked correctly. Because this PR was opened without context and the MySQL internal documentation is garbage, it took writing and running the test to actually understand the situation. (The C API documentation mentions this briefly but also lists MYSQL_TYPE_ENUM as corresponding to ENUM fields just above it, so that's not much help.)

The problem is that how MySQL interprets MYSQL_TYPE_ENUM appears to have changed over time. In 5.7, it accepts string inputs with the type MYSQL_TYPE_ENUM and binds them correctly. MySQL 8.4, on the other hand, doesn't like it and throws an error:

1210 (HY000): Incorrect arguments to mysqld_stmt_execute

I suspect it's expecting the integer encoding that's mainly used internally. Or it just doesn't like it altogether; I'm not really sure and I don't want to get into it.

Both 5.7 and 8 return columns with MYSQL_TYPE_STRING and the ENUM flag set, so I'm not really sure what we were thinking when the code was originally written. This is why it's important to have real tests for things.

Your change did result in the correct behavior, with one exception: MySQL doesn't appear to care about the BINARY flag with enums and doesn't set it on returned columns, so MySqlTypeInfo::__enum() wasn't actually being considered compatible with what MySQL was returning. Removing the BINARY flag fixed it.

I've taken the liberty of pushing my changes. Since it's still your PR, you'll get the credit in the CHANGELOG. I don't think this is a breaking change because it fixes something that never really worked in the first place.

@abonander abonander changed the title Support for MySQL enums in sqlx::Type Fix encoding and decoding of MySQL enums in sqlx::Type Jul 25, 2024
@abonander abonander merged commit f2fea27 into launchbadge:main Jul 25, 2024
65 checks passed
@alu
Copy link
Contributor Author

alu commented Jul 25, 2024

@abonander

Thanks for the merge and your dedication and commitment.
I also did a local test and should have shared it with you. I am sorry.

I love this library with all my heart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MySQL: rust enum not compatible with SQL ENUM
2 participants