-
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
VReplication: Qualify and SQL escape tables in created AutoIncrement VSchema definitions #17174
VReplication: Qualify and SQL escape tables in created AutoIncrement VSchema definitions #17174
Conversation
…keyspace Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17174 +/- ##
==========================================
- Coverage 67.45% 67.44% -0.02%
==========================================
Files 1569 1569
Lines 252229 252244 +15
==========================================
- Hits 170150 170124 -26
- Misses 82079 82120 +41 ☔ View full report in Codecov by Sentry. |
a39994d
to
b286043
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
b286043
to
4ceed1c
Compare
if err != nil { | ||
return err | ||
} | ||
seqTableName = fmt.Sprintf("%s.%s", seqKeyspace, seqTableName) |
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.
🥳
Signed-off-by: Matt Lord <mattalord@gmail.com>
…VSchema definitions (vitessio#17174) Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Renan Rangel <rrangel@slack-corp.com>
Description
This is a follow-up to #16860 that addresses a couple of potential edge cases when we automatically replace the auto increment handling:
This PR addresses those respectively by 1) ensuring that the sequence table name is always properly SQL escaped 2) qualifying the sequence table with the keyspace and ensuring that the keyspace name is also properly SQL escaped.
I think that we should backport this to v21 as it heads off potential bugs related to keyspaces and table names that must be SQL escaped along with potential bugs related to the non-qualified generated sequence names NOT being unique across all keyspaces in the Vitess cluster.
Related Issue(s)
Checklist