Skip to content
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

Max/drop pk #83

Merged
merged 5 commits into from
Aug 2, 2021
Merged

Max/drop pk #83

merged 5 commits into from
Aug 2, 2021

Conversation

max-hoffman
Copy link

No description provided.

@max-hoffman max-hoffman requested a review from zachmu as a code owner July 22, 2021 00:43
@@ -2288,18 +2288,24 @@ type IndexSpec struct {
Columns []*IndexColumn
// Options contains the index options when creating an index
Options []*IndexOption
// Primary indicates a PRIMARY KEY create or delete operation
Primary bool

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a part of the type parameter not a new field on a struct

@@ -2288,18 +2288,24 @@ type IndexSpec struct {
Columns []*IndexColumn
// Options contains the index options when creating an index
Options []*IndexOption
// Primary indicates a PRIMARY KEY create or delete operation
Primary bool
}

func (idx *IndexSpec) Format(buf *TrackedBuffer) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we using this as an index and not a constraint?

input: "alter table a add primary key",
output: "alter table a",
input: "alter table a add primary key (a, b)",
output: "alter table a add constraint PRIMARY primary key (a, b)",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like incorrect formatting. The output wouldn't run on mysql

Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -2174,6 +2174,15 @@ constraint_symbol_opt:
$$ = string($2)
}

pk_symbol_opt:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pk_name_opt

Signed-off-by: Vinai Rachakonda <rachakondavinai@gmail.com>
@VinaiRachakonda VinaiRachakonda merged commit 01de666 into master Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants