Deprecate mismatched collation comparison for uniquness validator#35350
Merged
kamipo merged 1 commit intorails:masterfrom Mar 4, 2019
Merged
Conversation
In MySQL, the default collation is case insensitive. Since the uniqueness validator enforces case sensitive comparison by default, it frequently causes mismatched collation issues (performance, weird behavior, etc) to MySQL users. https://grosser.it/2009/12/11/validates_uniqness_of-mysql-slow/ rails#1399 rails#13465 gitlabhq/gitlabhq@c1dddf8 huginn/huginn#1330 (comment) I'd like to deprecate the implicit default enforcing since I frequently experienced the problems in code reviews. Note that this change has no effect to sqlite3, postgresql, and oracle-enhanced adapters which are implemented as case sensitive by default, only affect to mysql2 adapter (I can take a work if sqlserver adapter will support Rails 6.0).
4c5a18d to
9def053
Compare
kamipo
commented
Mar 4, 2019
|
|
||
| if current_adapter?(:Mysql2Adapter) | ||
| assert_equal 2, Topic.where(author_email_address: "david@loudthinking.com").count | ||
| assert_equal 2, Topic.where(author_email_address: "David@loudthinking.com").count |
Member
Author
There was a problem hiding this comment.
That happening this (break the uniqueness contract) easily without race condition is what I want to prevent, which frequently experienced to me as MySQL DBA.
Member
Author
|
I'm going to merge this, since the case sensitive comparison on the case insensitive column is potentially problematic, the enforcing implicitly would causes any problems silently without being noticed by MySQL users. If the deprecation message would be seen on the apps using MySQL, I strongly recommend to confirm the query execution plan and data integrity on the database, and to reconsider that the case sensitive comparison on the case insensitive column is whether or not what you want. |
suketa
added a commit
to suketa/rails_sandbox
that referenced
this pull request
Sep 1, 2019
Deprecate mismatched collation comparison for uniquness validator rails/rails#35350
suketa
added a commit
to suketa/rails_sandbox
that referenced
this pull request
Sep 1, 2019
Deprecate mismatched collation comparison for uniquness validator rails/rails#35350
Merged
8 tasks
Merged
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In MySQL, the default collation is case insensitive. Since the
uniqueness validator enforces case sensitive comparison by default, it
frequently causes mismatched collation issues (performance, weird
behavior, etc) to MySQL users.
https://grosser.it/2009/12/11/validates_uniqness_of-mysql-slow/
#1399
#13465
gitlabhq/gitlabhq@c1dddf8
huginn/huginn#1330 (comment)
I'd like to deprecate the implicit default enforcing since I frequently
experienced the problems in code reviews.
e.g.
Note that this change has no effect to sqlite3, postgresql, and
oracle-enhanced adapters which are implemented as case sensitive by
default, only affect to mysql2 adapter (I can take a work if sqlserver
adapter will support Rails 6.0).
As far as I can see, Basecamp, Shopify, and GitHub are using MySQL (or MariaDB).
Can we make this deprecated in Rails 6.0?
cc @jeremy @rafaelfranca @tenderlove