-
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
SidecarDB: Don't ignore table charset and collation #16670
Conversation
Signed-off-by: Matt Lord <[email protected]>
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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16670 +/- ##
==========================================
- Coverage 68.93% 68.90% -0.03%
==========================================
Files 1562 1562
Lines 200767 200941 +174
==========================================
+ Hits 138407 138468 +61
- Misses 62360 62473 +113 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
And then ignore charset/collation when either is empty Signed-off-by: Matt Lord <[email protected]>
8fdbd39
to
ebad904
Compare
Signed-off-by: Matt Lord <[email protected]>
@@ -396,7 +396,7 @@ func (si *schemaInit) getCurrentSchema(tableName string) (string, error) { | |||
// or an ALTER if the table exists but has a different schema. | |||
func (si *schemaInit) findTableSchemaDiff(tableName, current, desired string) (string, error) { | |||
hints := &schemadiff.DiffHints{ | |||
TableCharsetCollateStrategy: schemadiff.TableCharsetCollateIgnoreAlways, | |||
TableCharsetCollateStrategy: schemadiff.TableCharsetCollateIgnoreEmpty, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and good catch. We might also just as well "upgrade" to TableCharsetCollateStrict
- but as this works then that's fine.
Description
Any table level character set and collation changes that we wanted to make were previously ignored during tablet init. That caused issues, in particular, with Vitess clusters that have been around for some time or that had modified defaults and e.g. may then be using a mix of utf8mb3 and utf8mb4. This can then lead to a host of unexpected failures when Vitess queries these internal tables, often times joining them with MySQL system tables e.g. in
information_schema
. In order to ensure consistent expected behavior we should be prescriptive and explicit about the character set and collations we want/expect on the sidecar (internal vitess) tables.This PR explicitly adds a character set for each sidecar table and modifies the settings used with
schemadiff
during the sidecar DB init process so that we apply any future changes to these within the code.Related Issue(s)
Checklist