Skip to content

Commit

Permalink
sql: catch unnamed/implicit indexes in diffing
Browse files Browse the repository at this point in the history
  • Loading branch information
a8m committed Jan 16, 2022
1 parent b760593 commit 82e5c60
Show file tree
Hide file tree
Showing 8 changed files with 196 additions and 42 deletions.
31 changes: 31 additions & 0 deletions internal/integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type T interface {
loadUsers() *schema.Table
posts() *schema.Table
loadPosts() *schema.Table
loadTable(string) *schema.Table
dropTables(...string)
migrate(...schema.Change)
diff(*schema.Table, *schema.Table) []schema.Change
Expand Down Expand Up @@ -97,6 +98,36 @@ func testEntIntegration(t T, dialect string, db *sql.DB) {
t.migrate(changes...)
}

func testImplicitIndexes(t T, db *sql.DB) {
const (
name = "implicit_indexes"
ddl = "create table implicit_indexes(c1 int unique, c2 int unique, unique(c1,c2), unique(c2,c1))"
)
t.dropTables(name)
_, err := db.Exec(ddl)
require.NoError(t, err)
current := t.loadTable(name)
c1, c2 := schema.NewNullIntColumn("c1", "int"), schema.NewNullIntColumn("c2", "int")
desired := schema.NewTable(name).
AddColumns(c1, c2).
AddIndexes(
schema.NewUniqueIndex("").AddColumns(c1),
schema.NewUniqueIndex("").AddColumns(c2),
schema.NewUniqueIndex("").AddColumns(c1, c2),
schema.NewUniqueIndex("").AddColumns(c2, c1),
)
changes := t.diff(current, desired)
require.Empty(t, changes)
desired.AddIndexes(
schema.NewIndex("c1_key").AddColumns(c1),
schema.NewIndex("c2_key").AddColumns(c2),
)
changes = t.diff(current, desired)
require.NotEmpty(t, changes)
t.migrate(&schema.ModifyTable{T: desired, Changes: changes})
ensureNoChange(t, desired)
}

func testHCLIntegration(t T, full string, empty string) {
t.applyHcl(full)
users := t.loadUsers()
Expand Down
20 changes: 13 additions & 7 deletions internal/integration/mysql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,12 @@ create table atlas_types_sanity
require.EqualValues(t, &expected, ts)
})
})

t.Run("ImplicitIndexes", func(t *testing.T) {
myRun(t, func(t *myTest) {
testImplicitIndexes(t, t.db)
})
})
}

func (t *myTest) dsn() string {
Expand Down Expand Up @@ -1109,19 +1115,19 @@ func (t *myTest) loadRealm() *schema.Realm {
}

func (t *myTest) loadUsers() *schema.Table {
realm := t.loadRealm()
require.Len(t, realm.Schemas, 1)
users, ok := realm.Schemas[0].Table("users")
require.True(t, ok)
return users
return t.loadTable("users")
}

func (t *myTest) loadPosts() *schema.Table {
return t.loadTable("posts")
}

func (t *myTest) loadTable(name string) *schema.Table {
realm := t.loadRealm()
require.Len(t, realm.Schemas, 1)
posts, ok := realm.Schemas[0].Table("posts")
table, ok := realm.Schemas[0].Table(name)
require.True(t, ok)
return posts
return table
}

func (t *myTest) mariadb() bool { return strings.HasPrefix(t.version, "Maria") }
Expand Down
20 changes: 13 additions & 7 deletions internal/integration/postgres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,12 @@ create table atlas_types_sanity
}
require.EqualValues(t, &expected, ts)
})

t.Run("ImplicitIndexes", func(t *testing.T) {
pgRun(t, func(t *pgTest) {
testImplicitIndexes(t, t.db)
})
})
}

func (t *pgTest) dsn() string {
Expand Down Expand Up @@ -910,19 +916,19 @@ func (t *pgTest) loadRealm() *schema.Realm {
}

func (t *pgTest) loadUsers() *schema.Table {
realm := t.loadRealm()
require.Len(t, realm.Schemas, 1)
users, ok := realm.Schemas[0].Table("users")
require.True(t, ok)
return users
return t.loadTable("users")
}

func (t *pgTest) loadPosts() *schema.Table {
return t.loadTable("posts")
}

func (t *pgTest) loadTable(name string) *schema.Table {
realm := t.loadRealm()
require.Len(t, realm.Schemas, 1)
posts, ok := realm.Schemas[0].Table("posts")
table, ok := realm.Schemas[0].Table(name)
require.True(t, ok)
return posts
return table
}

func (t *pgTest) users() *schema.Table {
Expand Down
20 changes: 13 additions & 7 deletions internal/integration/sqlite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,12 @@ create table atlas_types_sanity
}
require.EqualValues(t, &expected, ts)
})

t.Run("ImplicitIndexes", func(t *testing.T) {
liteRun(t, func(t *liteTest) {
testImplicitIndexes(t, t.db)
})
})
}

func (t *liteTest) applyHcl(spec string) {
Expand All @@ -712,19 +718,19 @@ func (t *liteTest) loadRealm() *schema.Realm {
}

func (t *liteTest) loadUsers() *schema.Table {
realm := t.loadRealm()
require.Len(t, realm.Schemas, 1)
users, ok := realm.Schemas[0].Table("users")
require.True(t, ok)
return users
return t.loadTable("users")
}

func (t *liteTest) loadPosts() *schema.Table {
return t.loadTable("posts")
}

func (t *liteTest) loadTable(name string) *schema.Table {
realm := t.loadRealm()
require.Len(t, realm.Schemas, 1)
posts, ok := realm.Schemas[0].Table("posts")
table, ok := realm.Schemas[0].Table(name)
require.True(t, ok)
return posts
return table
}

func (t *liteTest) users() *schema.Table {
Expand Down
86 changes: 65 additions & 21 deletions sql/internal/sqlx/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ type (
// changed. For example, an index-part collation.
IndexPartAttrChanged(from, to []schema.Attr) bool

// IsGeneratedIndexName reports if the index name was generated by the database
// for unnamed INDEX or UNIQUE constraints. In such cases, the differ will look
// for unnamed schema.Indexes on the desired state, before tagging the index as
// a candidate for deletion.
IsGeneratedIndexName(*schema.Table, *schema.Index) bool

// ReferenceChanged reports if the foreign key referential action was
// changed. For example, action was changed from RESTRICT to CASCADE.
ReferenceChanged(from, to schema.ReferenceOption) bool
Expand Down Expand Up @@ -186,27 +192,8 @@ func (d *Diff) TableDiff(from, to *schema.Table) ([]schema.Change, error) {
}
}

// Drop or modify indexes.
for _, idx1 := range from.Indexes {
idx2, ok := to.Index(idx1.Name)
if !ok {
changes = append(changes, &schema.DropIndex{I: idx1})
continue
}
if change := d.indexChange(idx1, idx2); change != schema.NoChange {
changes = append(changes, &schema.ModifyIndex{
From: idx1,
To: idx2,
Change: change,
})
}
}
// Add indexes.
for _, idx1 := range to.Indexes {
if _, ok := from.Index(idx1.Name); !ok {
changes = append(changes, &schema.AddIndex{I: idx1})
}
}
// Index changes.
changes = append(changes, d.indexDiff(from, to)...)

// Drop or modify foreign-keys.
for _, fk1 := range from.ForeignKeys {
Expand All @@ -232,6 +219,50 @@ func (d *Diff) TableDiff(from, to *schema.Table) ([]schema.Change, error) {
return changes, nil
}

// indexDiff returns the schema changes (if any) for migrating table
// indexes from current state to the desired state.
func (d *Diff) indexDiff(from, to *schema.Table) []schema.Change {
var (
changes []schema.Change
exists = make(map[*schema.Index]bool)
)
// Drop or modify indexes.
for _, idx1 := range from.Indexes {
idx2, ok := to.Index(idx1.Name)
// Found directly.
if ok {
if change := d.indexChange(idx1, idx2); change != schema.NoChange {
changes = append(changes, &schema.ModifyIndex{
From: idx1,
To: idx2,
Change: change,
})
}
exists[idx2] = true
continue
}
// Found indirectly.
if d.IsGeneratedIndexName(from, idx1) {
if idx2, ok := d.similarUnnamedIndex(to, idx1); ok {
exists[idx2] = true
continue
}
}
// Not found.
changes = append(changes, &schema.DropIndex{I: idx1})
}
// Add indexes.
for _, idx := range to.Indexes {
if exists[idx] {
continue
}
if _, ok := from.Index(idx.Name); !ok {
changes = append(changes, &schema.AddIndex{I: idx})
}
}
return changes
}

// pkChange returns the schema changes (if any) for migrating one primary key to the other.
func (d *Diff) pkChange(from, to *schema.Index) schema.ChangeKind {
change := d.indexChange(from, to)
Expand Down Expand Up @@ -311,6 +342,19 @@ func (d *Diff) fkChange(from, to *schema.ForeignKey) schema.ChangeKind {
return change
}

// similarUnnamedIndex searches for an unnamed index with the same index-parts in the table.
func (d *Diff) similarUnnamedIndex(t *schema.Table, idx1 *schema.Index) (*schema.Index, bool) {
for _, idx2 := range t.Indexes {
if idx2.Name != "" || len(idx2.Parts) != len(idx1.Parts) || idx2.Unique != idx1.Unique {
continue
}
if d.partsChange(idx1.Parts, idx2.Parts) == schema.NoChange {
return idx2, true
}
}
return nil, false
}

// CommentChange reports if the element comment was changed.
func CommentChange(from, to []schema.Attr) schema.ChangeKind {
var c1, c2 schema.Comment
Expand Down
28 changes: 28 additions & 0 deletions sql/mysql/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,34 @@ func (d *diff) ColumnChange(from, to *schema.Column) (schema.ChangeKind, error)
return change, nil
}

// IsGeneratedIndexName reports if the index name was generated by the database.
func (d *diff) IsGeneratedIndexName(_ *schema.Table, idx *schema.Index) bool {
// Auto-generated index names for functional/expression indexes. See.
// mysql-server/sql/sql_table.cc#add_functional_index_to_create_list
const f = "functional_index"
switch {
case d.supportsIndexExpr() && idx.Name == f:
return true
case d.supportsIndexExpr() && strings.HasPrefix(idx.Name+"_", f):
i, err := strconv.ParseInt(strings.TrimLeft(idx.Name, idx.Name+"_"), 10, 64)
return err == nil && i > 1
case len(idx.Parts) == 0 || idx.Parts[0].C == nil:
return false
}
// Unnamed INDEX or UNIQUE constraints are named by
// the first index-part (as column or part of it).
// For example, "c", "c_2", "c_3", etc.
switch name := idx.Parts[0].C.Name; {
case idx.Name == name:
return true
case strings.HasPrefix(idx.Name, name+"_"):
i, err := strconv.ParseInt(strings.TrimPrefix(idx.Name, name+"_"), 10, 64)
return err == nil && i > 1
default:
return false
}
}

// IndexAttrChanged reports if the index attributes were changed.
func (*diff) IndexAttrChanged(from, to []schema.Attr) bool {
return indexType(from).T != indexType(to).T
Expand Down
20 changes: 20 additions & 0 deletions sql/postgres/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package postgres
import (
"fmt"
"reflect"
"strconv"
"strings"
"unicode"

Expand Down Expand Up @@ -79,6 +80,25 @@ func (d *diff) defaultChanged(from, to *schema.Column) (bool, error) {
return !equals, nil
}

// IsGeneratedIndexName reports if the index name was generated by the database.
func (d *diff) IsGeneratedIndexName(t *schema.Table, idx *schema.Index) bool {
names := make([]string, len(idx.Parts))
for i, p := range idx.Parts {
if p.C == nil {
return false
}
names[i] = p.C.Name
}
// Auto-generate index names will have the following format: <table>_<c1>_..._key.
// In case of conflict, PostgreSQL adds additional index at the end (e.g. "key1").
p := fmt.Sprintf("%s_%s_key", t.Name, strings.Join(names, "_"))
if idx.Name == p {
return true
}
i, err := strconv.ParseInt(strings.TrimPrefix(idx.Name, p), 10, 64)
return err == nil && i > 0
}

// IndexAttrChanged reports if the index attributes were changed.
// The default type is BTREE if no type was specified.
func (*diff) IndexAttrChanged(from, to []schema.Attr) bool {
Expand Down
13 changes: 13 additions & 0 deletions sql/sqlite/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ package sqlite
import (
"fmt"
"reflect"
"strconv"
"strings"

"ariga.io/atlas/sql/internal/sqlx"
"ariga.io/atlas/sql/schema"
Expand Down Expand Up @@ -84,6 +86,17 @@ func (d *diff) defaultChanged(from, to *schema.Column) bool {
return err1 != nil || err2 != nil || x1 != x2
}

// IsGeneratedIndexName reports if the index name was generated by the database.
// See: https://github.com/sqlite/sqlite/blob/e937df8/src/build.c#L3583.
func (d *diff) IsGeneratedIndexName(t *schema.Table, idx *schema.Index) bool {
p := fmt.Sprintf("sqlite_autoindex_%s_", t.Name)
if !strings.HasPrefix(idx.Name, p) {
return false
}
i, err := strconv.ParseInt(strings.TrimPrefix(idx.Name, p), 10, 64)
return err == nil && i > 0
}

// IndexAttrChanged reports if the index attributes were changed.
func (*diff) IndexAttrChanged(from, to []schema.Attr) bool {
var p1, p2 IndexPredicate
Expand Down

0 comments on commit 82e5c60

Please sign in to comment.