-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Make Schema Tracking case-sensitive #14904
Make Schema Tracking case-sensitive #14904
Conversation
…itive Signed-off-by: Manan Gupta <manan@planetscale.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Manan Gupta <manan@planetscale.com>
…se sensitivity Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
…sensitivity Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
…required Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
I think we should port it to v18 as well. |
Signed-off-by: Manan Gupta <manan@planetscale.com>
Does this mean we need to separate out the cleanup from the fix here? |
No, I'll just resolve the conflicts in the backports. I don't want to create a separate PR unnecessarily. |
Nice catch and nice fix @GuptaManan100! |
…b3 is deprecated Signed-off-by: Manan Gupta <manan@planetscale.com>
9ef9fe9
to
cbe4b17
Compare
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
…ma-tracking Signed-off-by: Manan Gupta <manan@planetscale.com>
e07e92a
to
9ca2956
Compare
This fix only works with #14930, so we can't backport this without backporting that PR as well. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14904 +/- ##
==========================================
- Coverage 47.29% 47.29% -0.01%
==========================================
Files 1137 1137
Lines 238684 238647 -37
==========================================
- Hits 112895 112864 -31
- Misses 117168 117169 +1
+ Partials 8621 8614 -7 ☔ View full report in Codecov by Sentry. |
Why did we choose
https://dev.mysql.com/doc/refman/8.0/en/charset-unicode-utf8mb3.html |
See my comment in #14904 (comment)
|
Description
This PR fixes the issue #14671. This fix only works with #14930, so we can't backport this without backporting that PR as well.
Since the planner and all the other components of Vitess treat table names as case-sensitive, so should schema-tracking. This PR makes this change. Surprisingly the only change that was required was in the schema of the internal tables that we create. The
tables
andviews
tables for the schema engine where using the varchar columns (with the default collation) to store the table names, causing MySQL to error out with a duplicate key error, when trying to store the information of 2 tables such that their names only differ in casing.The columns in question have had their collations changed to
utfmb3_bin
, which is a case-sensitive collation resolving the issue.Moreover, while I was working on the schema-tracker, I have added more tests, and cleaned up the artificats left from the old way of schema-tracking which can now be removed.
Related Issue(s)
Checklist
Deployment Notes