Skip to content

Commit

Permalink
sql/mysql: fix the way we ignore indexes that were created automatica…
Browse files Browse the repository at this point in the history
…lly by foreign-keys
  • Loading branch information
a8m committed Jan 6, 2022
1 parent 2f42956 commit c54055a
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 19 deletions.
11 changes: 5 additions & 6 deletions sql/mysql/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,20 +96,19 @@ func (*diff) ReferenceChanged(from, to schema.ReferenceOption) bool {

// Normalize implements the sqlx.Normalizer interface.
func (d *diff) Normalize(from, to *schema.Table) {
for i, idx := range from.Indexes {
if _, ok := to.Index(idx.Name); ok {
continue
}
indexes := make([]*schema.Index, 0, len(from.Indexes))
for _, idx := range from.Indexes {
// MySQL requires that foreign key columns be indexed; Therefore, if the child
// table is defined on non-indexed columns, an index is automatically created
// to satisfy the constraint.
// Therefore, if no such key was defined on the desired state, the diff will
// recommend to drop it on migration. Therefore, we fix it by dropping it from
// the current state manually.
if keySupportsFK(from, idx) {
from.Indexes = append(from.Indexes[:i], from.Indexes[i+1:]...)
if _, ok := to.Index(idx.Name); ok || !keySupportsFK(from, idx) {
indexes = append(indexes, idx)
}
}
from.Indexes = indexes
}

// collationChange returns the schema change for migrating the collation if
Expand Down
13 changes: 8 additions & 5 deletions sql/mysql/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -535,28 +535,31 @@ func Build(phrase string) *sqlx.Builder {
// skipAutoChanges filters unnecessary changes that are automatically
// happened by the database when ALTER TABLE is executed.
func skipAutoChanges(changes []schema.Change) []schema.Change {
dropC := make(map[string]bool)
var (
dropC = make(map[string]bool)
planned = make([]schema.Change, 0, len(changes))
)
for _, c := range changes {
if c, ok := c.(*schema.DropColumn); ok {
dropC[c.C.Name] = true
}
}
search:
for i, c := range changes {
// Simple case for skipping key dropping, if its columns are dropped.
// https://dev.mysql.com/doc/refman/8.0/en/alter-table.html#alter-table-add-drop-column
c, ok := c.(*schema.DropIndex)
if !ok {
planned = append(planned, changes[i])
continue
}
for _, p := range c.I.Parts {
if p.C == nil || !dropC[p.C.Name] {
continue search
planned = append(planned, c)
break
}
}
changes = append(changes[:i], changes[i+1:]...)
}
return changes
return planned
}

// checks writes the CHECK constraint to the builder.
Expand Down
19 changes: 11 additions & 8 deletions sql/postgres/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -622,36 +622,39 @@ func Build(phrase string) *sqlx.Builder {
// skipAutoChanges filters unnecessary changes that are automatically
// happened by the database when ALTER TABLE is executed.
func skipAutoChanges(changes []schema.Change) []schema.Change {
dropC := make(map[string]bool)
var (
dropC = make(map[string]bool)
planned = make([]schema.Change, 0, len(changes))
)
for _, c := range changes {
if c, ok := c.(*schema.DropColumn); ok {
dropC[c.C.Name] = true
}
}
for i, c := range changes {
search:
for _, c := range changes {
switch c := c.(type) {
// Indexes involving the column are automatically dropped
// with it. This true for multi-columns indexes as well.
// See https://www.postgresql.org/docs/current/sql-altertable.html
case *schema.DropIndex:
for _, p := range c.I.Parts {
if p.C == nil && dropC[p.C.Name] {
changes = append(changes[:i], changes[i+1:]...)
break
if p.C != nil && dropC[p.C.Name] {
continue search
}
}
// Simple case for skipping constraint dropping,
// if the child table columns were dropped.
case *schema.DropForeignKey:
for _, c := range c.F.Columns {
if dropC[c.Name] {
changes = append(changes[:i], changes[i+1:]...)
break
continue search
}
}
}
planned = append(planned, c)
}
return changes
return planned
}

// checks writes the CHECK constraint to the builder.
Expand Down

0 comments on commit c54055a

Please sign in to comment.