Repository: ariga/atlas
Pull request #243 (2021-11-23 20:46:54)

sql/sqlite: ent integration tests and migration bug fixes


Patch:
diff --git a/sql/sqlite/diff.go b/sql/sqlite/diff.go
index a214cd0c..bf817fca 100644
--- a/sql/sqlite/diff.go
+++ b/sql/sqlite/diff.go
@@ -72,44 +72,8 @@ func (d *diff) typeChanged(from, to *schema.Column) (bool, error) {
 	if fromT == nil || toT == nil {
 		return false, fmt.Errorf("sqlite: missing type infromation for column %q", from.Name)
 	}
-	if reflect.TypeOf(fromT) != reflect.TypeOf(toT) {
-		return true, nil
-	}
-	var changed bool
-	switch fromT := fromT.(type) {
-	case *schema.BoolType:
-		toT := toT.(*schema.BoolType)
-		changed = fromT.T != toT.T
-	case *schema.BinaryType:
-		toT := toT.(*schema.BinaryType)
-		changed = fromT.T != toT.T
-	case *schema.DecimalType:
-		toT := toT.(*schema.DecimalType)
-		changed = fromT.T != toT.T
-	case *schema.FloatType:
-		toT := toT.(*schema.FloatType)
-		changed = fromT.T != toT.T
-	case *schema.EnumType:
-		toT := toT.(*schema.EnumType)
-		changed = !sqlx.ValuesEqual(fromT.Values, toT.Values)
-	case *schema.IntegerType:
-		// All integer types have the same "type affinity".
-	case *schema.JSONType:
-		toT := toT.(*schema.JSONType)
-		changed = fromT.T != toT.T
-	case *schema.StringType:
-		toT := toT.(*schema.StringType)
-		changed = fromT.T != toT.T
-	case *schema.SpatialType:
-		toT := toT.(*schema.SpatialType)
-		changed = fromT.T != toT.T
-	case *schema.TimeType:
-		toT := toT.(*schema.TimeType)
-		changed = fromT.T != toT.T
-	default:
-		return false, &sqlx.UnsupportedTypeError{Type: fromT}
-	}
-	return changed, nil
+	// Types are mismatched if they do not have the same "type affinity".
+	return reflect.TypeOf(fromT) != reflect.TypeOf(toT), nil
 }
 
 // defaultChanged reports if the a default value of a column
@@ -149,17 +113,18 @@ func (*diff) ReferenceChanged(from, to schema.ReferenceOption) bool {
 
 // Normalize implements the sqlx.Normalizer interface.
 func (d *diff) Normalize(from, to *schema.Table) {
+	used := make([]bool, len(to.ForeignKeys))
+	// In SQLite, there is no easy way to get the foreign-key constraint
+	// name, except for parsing the CREATE statement). Therefore, we check
+	// if there is a foreign-key with identical properties.
 	for _, fk1 := range from.ForeignKeys {
-		if _, ok := to.ForeignKey(fk1.Symbol); ok {
-			continue
-		}
-		// In SQLite, there is no easy way to get the foreign-key constraint
-		// name, except for parsing the CREATE statement). Therefore, we check
-		// if there is a foreign-key with identical properties.
-		for _, fk2 := range to.ForeignKeys {
-			if sameFK(fk1, fk2) {
+		for i, fk2 := range to.ForeignKeys {
+			if used[i] {
+				continue
+			}
+			if fk2.Symbol == fk1.Symbol && !isNumber(fk1.Symbol) || sameFK(fk1, fk2) {
 				fk1.Symbol = fk2.Symbol
-				break
+				used[i] = true
 			}
 		}
 	}
diff --git a/sql/sqlite/driver.go b/sql/sqlite/driver.go
index e9f21d96..3f17ed02 100644
--- a/sql/sqlite/driver.go
+++ b/sql/sqlite/driver.go
@@ -233,7 +233,7 @@ func parseRawType(c string) (schema.Type, error) {
 			ct.Scale = int(s)
 		}
 		return ct, nil
-	case "character", "varchar", "varying character", "nchar", "native character", "nvarchar", "text", "clob":
+	case "char", "character", "varchar", "varying character", "nchar", "native character", "nvarchar", "text", "clob":
 		ct := &schema.StringType{T: t}
 		if len(parts) > 1 {
 			p, err := strconv.ParseInt(parts[1], 10, 64)
@@ -289,7 +289,10 @@ func (d *Driver) addIndexes(t *schema.Table, rows *sql.Rows) error {
 			Name:   name.String,
 			Unique: uniq,
 			Table:  t,
-			Attrs:  []schema.Attr{&CreateStmt{S: stmt.String}},
+			Attrs: []schema.Attr{
+				&CreateStmt{S: stmt.String},
+				&IndexOrigin{O: origin.String},
+			},
 		}
 		if partial {
 			i := strings.Index(stmt.String, "WHERE")
@@ -501,6 +504,13 @@ type (
 		P string
 	}
 
+	// IndexOrigin describes how the index was created.
+	// See: https://www.sqlite.org/pragma.html#pragma_index_list
+	IndexOrigin struct {
+		schema.Attr
+		O string
+	}
+
 	// A UUIDType defines a UUID type.
 	UUIDType struct {
 		schema.Type
@@ -535,7 +545,7 @@ const (
 	// Query to list attached database files.
 	databasesQuery = "SELECT `name`, `file` FROM pragma_database_list()"
 	// Query to list database tables.
-	tablesQuery = "SELECT `name`, `sql` FROM sqlite_master WHERE type='table'"
+	tablesQuery = "SELECT `name`, `sql` FROM sqlite_master WHERE `type`='table' AND `name` NOT LIKE 'sqlite_%'"
 	// Query to list table information.
 	columnsQuery = "SELECT `name`, `type`, (not `notnull`) AS `nullable`, `dflt_value`, (`pk` <> 0) AS `pk`  FROM pragma_table_info('%s') ORDER BY `pk`, `cid`"
 	// Query to list table indexes.
diff --git a/sql/sqlite/migrate.go b/sql/sqlite/migrate.go
index 87cd52ab..550cb4f9 100644
--- a/sql/sqlite/migrate.go
+++ b/sql/sqlite/migrate.go
@@ -7,6 +7,7 @@ package sqlite
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"ariga.io/atlas/sql/internal/sqlx"
 	"ariga.io/atlas/sql/schema"
@@ -126,6 +127,26 @@ func (m *migrate) column(b *sqlx.Builder, c *schema.Column) {
 
 func (m *migrate) addIndexes(ctx context.Context, t *schema.Table, indexes ...*schema.Index) error {
 	for _, idx := range indexes {
+		// PRIMARY KEY or UNIQUE columns automatically create indexes with the generated name.
+		// See: sqlite/build.c#sqlite3CreateIndex. Therefore, we ignore such PKs, but create
+		// the inlined UNIQUE constraints manually with custom name, because SQLite does not
+		// allow creating indexes with such names manually. Note, this case is possible if
+		// "apply" schema that was inspected from the database as-is.
+		if strings.HasPrefix(idx.Name, "sqlite_autoindex") {
+			if i := (IndexOrigin{}); sqlx.Has(idx.Attrs, &i) && i.O == "p" {
+				continue
+			}
+			// Use the following format: <Table>_<Columns>.
+			names := make([]string, len(idx.Parts)+1)
+			names[0] = t.Name
+			for i, p := range idx.Parts {
+				if p.C == nil {
+					return fmt.Errorf("unexpected index part %s (%d)", idx.Name, i)
+				}
+				names[i+1] = p.C.Name
+			}
+			idx.Name = strings.Join(names, "_")
+		}
 		b := Build("CREATE")
 		if idx.Unique {
 			b.P("UNIQUE")
@@ -216,12 +237,27 @@ func (m *migrate) copyRows(ctx context.Context, from *schema.Table, to *schema.T
 		// Find a change that associated with this column, if exists.
 		var change schema.Change
 		for i := range changes {
-			switch changes[i].(type) {
-			case *schema.AddColumn, *schema.DropColumn, *schema.ModifyColumn:
+			switch c := changes[i].(type) {
+			case *schema.AddColumn:
+				if c.C.Name != column.Name {
+					break
+				}
 				if change != nil {
-					return fmt.Errorf("duplicate changes for column: %q: %T, %T", column.Name, change, changes[i])
+					return fmt.Errorf("duplicate changes for column: %q: %T, %T", column.Name, change, c)
 				}
 				change = changes[i]
+			case *schema.ModifyColumn:
+				if c.To.Name != column.Name {
+					break
+				}
+				if change != nil {
+					return fmt.Errorf("duplicate changes for column: %q: %T, %T", column.Name, change, c)
+				}
+				change = changes[i]
+			case *schema.DropColumn:
+				if c.C.Name == column.Name {
+					return fmt.Errorf("unexpected drop column: %q", column.Name)
+				}
 			}
 		}
 		switch change := change.(type) {
@@ -244,7 +280,7 @@ func (m *migrate) copyRows(ctx context.Context, from *schema.Table, to *schema.T
 			fromC = append(fromC, column.Name)
 		}
 	}
-	stmt := fmt.Sprintf("INSERT INTO %s (%s) SELECT (%s) FROM %s", to.Name, toC, fromC, from.Name)
+	stmt := fmt.Sprintf("INSERT INTO %s (%s) SELECT %s FROM %s", to.Name, strings.Join(toC, ", "), strings.Join(fromC, ", "), from.Name)
 	if _, err := m.ExecContext(ctx, stmt, args...); err != nil {
 		return fmt.Errorf("copy rows from %q to %q: %w", from.Name, to.Name, err)
 	}


Test patch:
diff --git a/internal/integration/sqlite_test.go b/internal/integration/sqlite_test.go
index 609a4e19..0b1f43c5 100644
--- a/internal/integration/sqlite_test.go
+++ b/internal/integration/sqlite_test.go
@@ -13,6 +13,7 @@ import (
 	"ariga.io/atlas/sql/schema"
 	"ariga.io/atlas/sql/sqlite"
 
+	"entgo.io/ent/dialect"
 	_ "github.com/mattn/go-sqlite3"
 	"github.com/stretchr/testify/require"
 )
@@ -47,8 +48,96 @@ func TestSQLite_Relation(t *testing.T) {
 	})
 }
 
+func TestSQLite_Ent(t *testing.T) {
+	liteRun(t, func(t *liteTest) {
+		testEntIntegration(t, dialect.SQLite, t.db)
+	})
+}
+
+func TestSQLite_AddIndexedColumns(t *testing.T) {
+	liteRun(t, func(t *liteTest) {
+		usersT := &schema.Table{
+			Name:    "users",
+			Columns: []*schema.Column{{Name: "id", Type: &schema.ColumnType{Type: &schema.IntegerType{T: "int"}}}},
+		}
+		t.migrate(&schema.AddTable{T: usersT})
+		t.dropTables(usersT.Name)
+
+		// Insert 2 records to the users table, and make sure they are there
+		// after executing migration.
+		_, err := t.db.Exec("INSERT INTO users (id) VALUES (1), (2)")
+		require.NoError(t, err)
+
+		usersT.Columns = append(usersT.Columns, &schema.Column{
+			Name:    "a",
+			Type:    &schema.ColumnType{Type: &schema.IntegerType{T: "integer"}, Null: true},
+			Default: &schema.RawExpr{X: "10"},
+		}, &schema.Column{
+			Name:    "b",
+			Type:    &schema.ColumnType{Type: &schema.IntegerType{T: "integer"}, Null: true},
+			Default: &schema.RawExpr{X: "20"},
+		}, &schema.Column{
+			Name:    "c",
+			Type:    &schema.ColumnType{Type: &schema.IntegerType{T: "integer"}, Null: true},
+			Default: &schema.RawExpr{X: "30"},
+		})
+		usersT.Indexes = append(usersT.Indexes, &schema.Index{
+			Unique: true,
+			Name:   "id_a_b_c_unique",
+			Parts:  []*schema.IndexPart{{C: usersT.Columns[0]}, {C: usersT.Columns[1]}, {C: usersT.Columns[2]}, {C: usersT.Columns[3]}},
+		})
+		changes := t.diff(t.loadUsers(), usersT)
+		require.Len(t, changes, 4, "usersT contains 3 new columns and 1 new index")
+		t.migrate(&schema.ModifyTable{T: usersT, Changes: changes})
+		ensureNoChange(t, usersT)
+
+		// Scan records from the table to ensure correctness of
+		// the rows transferring.
+		rows, err := t.db.Query("SELECT * FROM users")
+		require.NoError(t, err)
+		require.True(t, rows.Next())
+		var v [4]int
+		require.NoError(t, rows.Scan(&v[0], &v[1], &v[2], &v[3]))
+		require.Equal(t, [4]int{1, 10, 20, 30}, v)
+		require.True(t, rows.Next())
+		require.NoError(t, rows.Scan(&v[0], &v[1], &v[2], &v[3]))
+		require.Equal(t, [4]int{2, 10, 20, 30}, v)
+		require.False(t, rows.Next())
+		require.NoError(t, rows.Close())
+
+		// Dropping a column from both table and index.
+		usersT = t.loadUsers()
+		idx, ok := usersT.Index("id_a_b_c_unique")
+		require.True(t, ok)
+		require.Len(t, idx.Parts, 4)
+		usersT.Columns = usersT.Columns[:len(usersT.Columns)-1]
+		idx.Parts = idx.Parts[:len(idx.Parts)-1]
+		changes = t.diff(t.loadUsers(), usersT)
+		require.Len(t, changes, 2)
+		t.migrate(&schema.ModifyTable{T: usersT, Changes: changes})
+		ensureNoChange(t, t.loadUsers())
+
+		// Scan records from the table to ensure correctness of
+		// the rows transferring.
+		rows, err = t.db.Query("SELECT * FROM users")
+		require.NoError(t, err)
+		require.True(t, rows.Next())
+		var u [3]int
+		require.NoError(t, rows.Scan(&u[0], &u[1], &u[2]))
+		require.Equal(t, [3]int{1, 10, 20}, u)
+		require.True(t, rows.Next())
+		require.NoError(t, rows.Scan(&u[0], &u[1], &u[2]))
+		require.Equal(t, [3]int{2, 10, 20}, u)
+		require.False(t, rows.Next())
+		require.NoError(t, rows.Close())
+
+	})
+}
+
 func (t *liteTest) loadRealm() *schema.Realm {
-	r, err := t.drv.InspectRealm(context.Background(), &schema.InspectRealmOption{})
+	r, err := t.drv.InspectRealm(context.Background(), &schema.InspectRealmOption{
+		Schemas: []string{"main"},
+	})
 	require.NoError(t, err)
 	return r
 }
diff --git a/sql/sqlite/diff_test.go b/sql/sqlite/diff_test.go
index 190e44e0..fcc53be1 100644
--- a/sql/sqlite/diff_test.go
+++ b/sql/sqlite/diff_test.go
@@ -176,11 +176,11 @@ func TestDiff_TableDiff(t *testing.T) {
 				}
 			)
 			from.ForeignKeys = []*schema.ForeignKey{
-				{Table: from, Columns: from.Columns, RefTable: ref, RefColumns: ref.Columns[:1]},
-				{Symbol: "0", Table: from, Columns: from.Columns, RefTable: ref, RefColumns: ref.Columns[:1]},
+				{Symbol: "fk1", Table: to, Columns: to.Columns, RefTable: ref, RefColumns: ref.Columns[1:]},
+				{Symbol: "1", Table: from, Columns: from.Columns, RefTable: ref, RefColumns: ref.Columns[:1]},
 			}
 			to.ForeignKeys = []*schema.ForeignKey{
-				{Table: to, Columns: to.Columns, RefTable: ref, RefColumns: ref.Columns[1:]},
+				{Symbol: "fk1", Table: to, Columns: to.Columns, RefTable: ref, RefColumns: ref.Columns[:1]},
 				// The below "constraint" is identical to "0" above, therefore, the differ does not report a change.
 				{Symbol: "constraint", Table: from, Columns: from.Columns, RefTable: ref, RefColumns: ref.Columns[:1]},
 			}
diff --git a/sql/sqlite/driver_test.go b/sql/sqlite/driver_test.go
index 42a74bdf..0a2add05 100644
--- a/sql/sqlite/driver_test.go
+++ b/sql/sqlite/driver_test.go
@@ -135,6 +135,7 @@ func TestDriver_InspectTable(t *testing.T) {
 						},
 						Attrs: []schema.Attr{
 							&CreateStmt{S: "CREATE UNIQUE INDEX c1u on users(c1, c2)"},
+							&IndexOrigin{O: "c"},
 						},
 					},
 					{
@@ -146,6 +147,7 @@ func TestDriver_InspectTable(t *testing.T) {
 						},
 						Attrs: []schema.Attr{
 							&CreateStmt{S: "CREATE INDEX c1_c2 on users(c1, c2*2) WHERE c1 <> NULL"},
+							&IndexOrigin{O: "c"},
 							&IndexPredicate{P: "c1 <> NULL"},
 						},
 					},