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

FYI collisions when object_ids are case sensitive #896

Open
borwickatuw opened this issue Oct 20, 2021 · 3 comments
Open

FYI collisions when object_ids are case sensitive #896

borwickatuw opened this issue Oct 20, 2021 · 3 comments

Comments

@borwickatuw
Copy link

FYI when using django-reversion along with HashidAutoField from django-hashid-field, I observed that the Hashid values are case-sensitive but the default MySQL column for reversion_version is case-insensitive.

This can show up as receiving version history for more than one object when running

Version.objects.get_for_model(ModelUsingHashidAutoField).filter(object_id=HashedID)

For example, one object might have object_id of aBcDeF and another object has object_id of AbCdEf.

This is a database-level issue because by default MySQL does case-insensitive collation on varchar fields.

To address this issue, I ran:

ALTER TABLE reversion_version MODIFY object_id VARCHAR(191) BINARY NOT NULL;

(HashidAutoField itself doesn't run into this problem because those ids aren't actually stored in the database. They're calculated on the fly.)

I hope this helps others!

@etianen
Copy link
Owner

etianen commented Oct 20, 2021 via email

@borwickatuw
Copy link
Author

I'm not sure how many database backends have this issue, or how common case-sensitive object_ids are, which is why I just shared the manual way to do it. WIthout looking into it too closely I see a StackOverflow recommendation to create a custom Django model field https://stackoverflow.com/a/39221772 , but I don't know whether that's a good idea.

Maybe a workaround could be a custom database migration for MySQL to set the field to be case-insensitive? https://stackoverflow.com/a/38304105 That's probably not the best idea either :/

@etianen
Copy link
Owner

etianen commented Oct 20, 2021

Fair enough. Thanks for reporting this issue. It'll be a good record for anyone else with the same issue.

I'm not in a good position to dev against MySQL currently. If someone who regularly uses MySQL wants to contribute an MR to fix this at source, I'm very happy to accept! 🥺

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

No branches or pull requests

2 participants