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

MySQL: Remove collation compatibility check for strings #2652

Merged
merged 2 commits into from
Jun 20, 2024

Conversation

alu
Copy link
Contributor

@alu alu commented Jul 25, 2023

Compatibility check in<str as sqlx::Type<sqlx::MySql>>::compatible(tv) for collation is not necessary because collation does not affect to output string.

RELATE: #1241

@abonander
Copy link
Collaborator

I'm trying to remember why we initially discounted BINARY_FLAG and BLOB_FLAG for telling the difference between a text string and a binary string, but nothing's coming to mind.

@alu
Copy link
Contributor Author

alu commented Aug 1, 2023

As the title suggests, I tried to remove the judgment by collation from the comatibility method, but since the type could not be judged correctly, I returned to correcting only the field name.
Specifically, this test fails because _1 is judged as a string instead of a byte string.

@alu
Copy link
Contributor Author

alu commented Aug 1, 2023

Since it became possible to judge with flags, a new commit was added.

@alu
Copy link
Contributor Author

alu commented Sep 5, 2023

@abonander
Hi!

Is it not possible to merge this change?

@abonander
Copy link
Collaborator

I merge PRs as I get to them. SQLx is only a small part of my full-time job currently.

@abonander
Copy link
Collaborator

My gut is telling me this should wait for a major release as it could change some type mappings in the macros, but I'm not sure on that.

@abonander abonander changed the title Remove collation compatibility check. MySQL: Remove collation compatibility check for strings Jun 20, 2024
@abonander abonander merged commit 33aee07 into launchbadge:main Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants