Skip to content

Conversation

vsalvino
Copy link
Contributor

@vsalvino vsalvino commented Nov 3, 2022

Fixes #563

This change does not affect existing behavior if the collate option is not specified.

To test this out locally, e.g. in a Django project:

  • Make sure you have MySQL or MariaDB C connector, and a C compiler installed.
  • pip install git+https://github.com/vsalvino/mysqlclient.git
  • Set your database connection as so:
DATABASES = {
    "default": {
        "ENGINE": "django.db.backends.mysql",
        "HOST": "",
        "NAME": "",
        "USER": "",
        "PASSWORD": "",
        "OPTIONS": {
            "charset": "utf8mb4",
            "collation": "utf8mb4_unicode_ci",
        },
    }
}

@codecov
Copy link

codecov bot commented Nov 3, 2022

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.10 🎉

Comparison is base (58465cf) 86.00% compared to head (fe0bb09) 86.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #564      +/-   ##
==========================================
+ Coverage   86.00%   86.10%   +0.10%     
==========================================
  Files           6        6              
  Lines         550      554       +4     
==========================================
+ Hits          473      477       +4     
  Misses         77       77              
Impacted Files Coverage Δ
MySQLdb/connections.py 87.12% <100.00%> (+0.40%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@vsalvino
Copy link
Contributor Author

@methane do you have any opinions on this change? I do not want to publish a fork to PyPI, but we are in an impossible situation due to this behavior.

@vanschelven
Copy link

@methane any chance you could look at this?

@ViggieM
Copy link

ViggieM commented Jan 12, 2023

@methane please take a look, as it is a pressing issue also for me

@vsalvino
Copy link
Contributor Author

@methane checking on this again. It is causing major breakage and customer support issues for us. I have no choice but to publish a fork to PyPI and then change the Django and Wagtail documentation to direct users to the fork instead of the official package. I really DO NOT want to do this - relying on a fork is worse for everybody!

If you need help maintaining the package, by all means get in touch and I would be happy to volunteer. My email is [email protected].

@methane
Copy link
Member

methane commented Feb 28, 2023

I'm very busy for now. I will take it in a month.

@methane
Copy link
Member

methane commented Feb 28, 2023

@methane methane changed the title Add collate option Add collation option Mar 23, 2023
@methane
Copy link
Member

methane commented Mar 23, 2023

@vsalvino
Copy link
Contributor Author

In the future I would like to try changing the MariaDB and MySQL C libraries to accept collation when setting the character set, to reduce this to a single query, as I think that would be the most "proper" fix. However that would be a breaking API change or require adding a new API; so I imagine it would be unlikely to happen, or require a long time and much debate.

Thanks for your help.

@raisons
Copy link

raisons commented Apr 24, 2023

When will the new version be released?

@vsalvino
Copy link
Contributor Author

vsalvino commented May 3, 2023

Update for anyone following, I published this change as mysqlclient-collate https://pypi.org/project/mysqlclient-collate/

@methane methane merged commit df52e23 into PyMySQL:main May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Impossible to set correct collation
5 participants