support rename constraint syntax #330
Conversation
| case *ForeignKeyDefinition: | ||
| buf.Myprintf(" rename constraint %s to", node.TableSpec.Constraints[0].Name) | ||
| } | ||
| switch node.TableSpec.Constraints[1].Details.(type) { | ||
| case *ForeignKeyDefinition: | ||
| buf.Myprintf(" %s", node.TableSpec.Constraints[1].Name) | ||
| } |
There was a problem hiding this comment.
Do we need to switch on the type of Details? I see that this field will currently always be a ForeignKeyDefinition, but it seems like that would be likely to change in the future if we expanded this to support renaming other constraints, and then this code wouldn't print out the statement correctly anymore. Not a huge deal, since we should catch that in development, but seems like it could be simpler if we just printed out the constraint name without asserting the type.
go/vt/sqlparser/sql.y
Outdated
| ddl.TableSpec.AddConstraint(&ConstraintDefinition{Name: string($3), Details: &ForeignKeyDefinition{}}) | ||
| ddl.TableSpec.AddConstraint(&ConstraintDefinition{Name: string($5), Details: &ForeignKeyDefinition{}}) |
There was a problem hiding this comment.
Similar to the other comment... do we have to supply the Details: &ForeignKeyDefinition{}, or can we just supply the name and let GMS figure out what type it is? Seems like GMS is going to have to validate it anyway, and this syntax seems useful/intuitive to apply to other constraint types eventually, too.
There was a problem hiding this comment.
It's probably possible to have GMS determine what kind of constraint it is, but right now gms/sql/planbuilder/ddl.go:convertConstraintDefinition switches on Details to create the right constraint type.
2 participants
This PR adds syntax support for
ALTER TABLE ... RENAME CONSTRAINT [FOREIGN KEY / CHECK]...for foreign key constraints.