-
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
schemadiff: normalize PRIMARY KEY
definition
#12016
schemadiff: normalize PRIMARY KEY
definition
#12016
Conversation
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
…ition splits into 'id int' and 'primary key (id)' parts. Primary key is always the first key in the list of keys. Handle queries such as 'modify id int primary key' (validate there isn't already a primary key, or that the existing primary key is over 'id'. add column with primary key definition... Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
If a new flag is being introduced:
If a workflow is added or modified:
Bug fixes
Non-trivial changes
New/Existing features
Backward compatibility
|
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
ColKeyNone ColumnKeyOption = iota | ||
ColKeyPrimary | ||
ColKeySpatialKey | ||
ColKeyFulltextKey | ||
ColKeyUnique | ||
ColKeyUniqueKey | ||
ColKey |
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.
The change in go/vt/sqlparser
is to make the above constants public, so that we can introspect a ColumnDefinition
and see whether it has a specific key option.
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
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
Description
In this PR we normalize and validate
PRIMARY KEY
definitions:1. Normalize and validate
CREATE TABLE
This statement:
is normalized into:
Table definition is validated to have no more than one
PRIMARY KEY
.2. Validate
ADD COLUMN
In this statement:
We validate that the table does not already have a
PRIMARY KEY
. Once applied, the table definition is normalized as per the above.3. Validate
MODIFY COLUMN
In this statement:
We validate that the table either:
PRIMARY KEY
, orPRIMARY KEY (i)
, ie no change to thePRIMARY KEY
itself.Once applied, the table definition is normalized as per the above.
4. Table diffs change
The diff of these two tables:
is:
Note that there is nothing to say that
id
is aPRIMARY KEY
even though the original definitions included that information as part of column definition options. The reason is that internally the tables are normalized as per (1) above.Several tests are added to validate all the above. Otherwise there's been a massive impact to unit tests because we used the
id int primary key
generously throughout the tests, and had to rewrite dozens of those.Related Issue(s)
tracking: #10203
This work is needed for better foreign key support in
schemadiff
: it needs to validate that a parent table referenced by a foreign key, has the appropriate indexes over the referenced columns. It's then important to have thePRIMARY KEY
normalized as this PR does. See #11975 and #11944Otherwise, this is just the right way of normalizing a table.
SHOW CREATE TABLE
in MySQL always extractsPRIMARY KEY
information outside the column definition.Checklist
Deployment Notes