From 139725ee00ffd51ce0d93dc855789ce666f468a4 Mon Sep 17 00:00:00 2001 From: Jannik Clausen <12862103+masseelch@users.noreply.github.com> Date: Fri, 23 Sep 2022 12:40:36 +0200 Subject: [PATCH] dialect/sql/schema: no longer allocate a PK range for a join table (#2950) * dialect/sql/schema: no longer allocate a PK range for a join table * test that no breaking change is introduced --- dialect/sql/schema/atlas.go | 2 +- dialect/sql/schema/migrate_test.go | 157 +++++++++++++++++++++++++---- 2 files changed, 141 insertions(+), 18 deletions(-) diff --git a/dialect/sql/schema/atlas.go b/dialect/sql/schema/atlas.go index 5535e1c80..254cfb78c 100644 --- a/dialect/sql/schema/atlas.go +++ b/dialect/sql/schema/atlas.go @@ -803,7 +803,7 @@ func (a *Atlas) tables(tables []*Table) ([]*schema.Table, error) { for i, et := range tables { at := schema.NewTable(et.Name) a.sqlDialect.atTable(et, at) - if a.universalID && et.Name != TypeTable { + if a.universalID && et.Name != TypeTable && len(et.PrimaryKey) == 1 { r, err := a.pkRange(et) if err != nil { return nil, err diff --git a/dialect/sql/schema/migrate_test.go b/dialect/sql/schema/migrate_test.go index 1ee2d96e0..650ddef2c 100644 --- a/dialect/sql/schema/migrate_test.go +++ b/dialect/sql/schema/migrate_test.go @@ -101,6 +101,137 @@ func TestMigrate_Formatter(t *testing.T) { require.Equal(t, migrate.DefaultFormatter, m.fmt) } +func TestMigrate_DiffJoinTableAllocationBC(t *testing.T) { + // Due to a bug in previous versions, if the universal ID option was enabled and the schema did contain an M2M + // relation, the join table would have had an entry for the join table in the types table. This test ensures, + // that the PK range allocated for the join table stays in place, since it's removal would break existing projects + // due to shifted ranges. + + db, err := sql.Open(dialect.SQLite, "file:test?mode=memory&_fk=1") + require.NoError(t, err) + + // Mock an existing database with an allocation for a join table. + for _, stmt := range []string{ + "CREATE TABLE `groups` (`id` integer NOT NULL PRIMARY KEY AUTOINCREMENT, `name` text NOT NULL);", + "CREATE INDEX `short` ON `groups` (`id`);", + "CREATE INDEX `long____________________________1cb2e7e47a309191385af4ad320875b1` ON `groups` (`id`);", + "CREATE TABLE `users` (`id` integer NOT NULL PRIMARY KEY AUTOINCREMENT, `name` text NOT NULL);", + "INSERT INTO sqlite_sequence (name, seq) VALUES (\"users\", 4294967296);", + "CREATE TABLE `user_groups` (`user_id` integer NOT NULL, `group_id` integer NOT NULL, PRIMARY KEY (`user_id`, `group_id`), CONSTRAINT `user_groups_user_id` FOREIGN KEY (`user_id`) REFERENCES `users` (`id`) ON DELETE CASCADE, CONSTRAINT `user_groups_group_id` FOREIGN KEY (`group_id`) REFERENCES `groups` (`id`) ON DELETE CASCADE);", + "INSERT INTO sqlite_sequence (name, seq) VALUES (\"user_groups\", 8589934592);", + "CREATE TABLE `ent_types` (`id` integer NOT NULL PRIMARY KEY AUTOINCREMENT, `type` text NOT NULL);", + "CREATE UNIQUE INDEX `ent_types_type_key` ON `ent_types` (`type`);", + "INSERT INTO `ent_types` (`type`) VALUES ('groups'), ('users'), ('user_groups');", + "INSERT INTO `groups` (`name`) VALUES ('seniors'), ('juniors')", + "INSERT INTO `users` (`name`) VALUES ('masseelch'), ('a8m'), ('rotemtam')", + "INSERT INTO `user_groups` (`user_id`, `group_id`) VALUES (4294967297, 1), (4294967298, 1), (4294967299, 2)", + } { + _, err := db.ExecContext(context.Background(), stmt) + require.NoError(t, err) + } + + // Expect to have no changes when migration runs with fix. + m, err := NewMigrate(db, WithGlobalUniqueID(true), WithDiffHook(func(next Differ) Differ { + return DiffFunc(func(current, desired *schema.Schema) ([]schema.Change, error) { + changes, err := next.Diff(current, desired) + if err != nil { + return nil, err + } + require.Len(t, changes, 0) + return changes, nil + }) + })) + require.NoError(t, err) + require.NoError(t, m.Create(context.Background(), tables...)) + + // Expect to have no changes to the allocation when the join table is dropped. + m, err = NewMigrate(db, WithGlobalUniqueID(true)) + require.NoError(t, err) + require.NoError(t, m.Create(context.Background(), groupsTable, usersTable)) + + rows, err := db.QueryContext(context.Background(), "SELECT `type` from `ent_types` ORDER BY `id` ASC") + require.NoError(t, err) + var types []string + for rows.Next() { + var typ string + require.NoError(t, rows.Scan(&typ)) + types = append(types, typ) + } + require.NoError(t, rows.Err()) + require.Equal(t, []string{"groups", "users", "user_groups"}, types) +} + +var ( + groupsColumns = []*Column{ + {Name: "id", Type: field.TypeInt, Increment: true}, + {Name: "name", Type: field.TypeString}, + } + groupsTable = &Table{ + Name: "groups", + Columns: groupsColumns, + PrimaryKey: []*Column{groupsColumns[0]}, + Indexes: []*Index{ + { + Name: "short", + Columns: []*Column{groupsColumns[0]}}, + { + Name: "long_" + strings.Repeat("_", 60), + Columns: []*Column{groupsColumns[0]}, + }, + }, + } + usersColumns = []*Column{ + {Name: "id", Type: field.TypeInt, Increment: true}, + {Name: "name", Type: field.TypeString}, + } + usersTable = &Table{ + Name: "users", + Columns: usersColumns, + PrimaryKey: []*Column{usersColumns[0]}, + } + userGroupsColumns = []*Column{ + {Name: "user_id", Type: field.TypeInt}, + {Name: "group_id", Type: field.TypeInt}, + } + userGroupsTable = &Table{ + Name: "user_groups", + Columns: userGroupsColumns, + PrimaryKey: []*Column{userGroupsColumns[0], userGroupsColumns[1]}, + ForeignKeys: []*ForeignKey{ + { + Symbol: "user_groups_user_id", + Columns: []*Column{userGroupsColumns[0]}, + RefColumns: []*Column{usersColumns[0]}, + OnDelete: Cascade, + }, + { + Symbol: "user_groups_group_id", + Columns: []*Column{userGroupsColumns[1]}, + RefColumns: []*Column{groupsColumns[0]}, + OnDelete: Cascade, + }, + }, + } + tables = []*Table{ + groupsTable, + usersTable, + userGroupsTable, + } + petColumns = []*Column{ + {Name: "id", Type: field.TypeInt, Increment: true}, + } + petsTable = &Table{ + Name: "pets", + Columns: petColumns, + PrimaryKey: petColumns, + } +) + +func init() { + userGroupsTable.ForeignKeys[0].RefTable = usersTable + userGroupsTable.ForeignKeys[1].RefTable = groupsTable +} + func TestMigrate_Diff(t *testing.T) { ctx := context.Background() @@ -132,7 +263,6 @@ func TestMigrate_Diff(t *testing.T) { require.NoError(t, d.WriteFile("tmp.sql", nil)) require.ErrorIs(t, m.Diff(ctx, &Table{Name: "users"}), migrate.ErrChecksumMismatch) - idCol := []*Column{{Name: "id", Type: field.TypeInt, Increment: true}} p = t.TempDir() d, err = migrate.NewLocalDir(p) require.NoError(t, err) @@ -144,35 +274,28 @@ func TestMigrate_Diff(t *testing.T) { ) require.NoError(t, err) + // Join tables (mapping between user and group) will not result in an entry to the types table. m, err = NewMigrate(db, WithFormatter(f), WithDir(d), WithGlobalUniqueID(true)) require.NoError(t, err) - require.NoError(t, m.Diff(ctx, - &Table{Name: "users", Columns: idCol, PrimaryKey: idCol}, - &Table{ - Name: "groups", - Columns: idCol, - PrimaryKey: idCol, - Indexes: []*Index{ - {Name: "short", Columns: idCol}, - {Name: "long_" + strings.Repeat("_", 60), Columns: idCol}, - }}, - )) + require.NoError(t, m.Diff(ctx, tables...)) changesSQL := strings.Join([]string{ - "CREATE TABLE `users` (`id` integer NOT NULL PRIMARY KEY AUTOINCREMENT);", - "CREATE TABLE `groups` (`id` integer NOT NULL PRIMARY KEY AUTOINCREMENT);", - fmt.Sprintf("INSERT INTO sqlite_sequence (name, seq) VALUES (\"groups\", %d);", 1<<32), + "CREATE TABLE `groups` (`id` integer NOT NULL PRIMARY KEY AUTOINCREMENT, `name` text NOT NULL);", "CREATE INDEX `short` ON `groups` (`id`);", "CREATE INDEX `long____________________________1cb2e7e47a309191385af4ad320875b1` ON `groups` (`id`);", + "CREATE TABLE `users` (`id` integer NOT NULL PRIMARY KEY AUTOINCREMENT, `name` text NOT NULL);", + fmt.Sprintf("INSERT INTO sqlite_sequence (name, seq) VALUES (\"users\", %d);", 1<<32), + "CREATE TABLE `user_groups` (`user_id` integer NOT NULL, `group_id` integer NOT NULL, PRIMARY KEY (`user_id`, `group_id`), CONSTRAINT `user_groups_user_id` FOREIGN KEY (`user_id`) REFERENCES `users` (`id`) ON DELETE CASCADE, CONSTRAINT `user_groups_group_id` FOREIGN KEY (`group_id`) REFERENCES `groups` (`id`) ON DELETE CASCADE);", "CREATE TABLE `ent_types` (`id` integer NOT NULL PRIMARY KEY AUTOINCREMENT, `type` text NOT NULL);", "CREATE UNIQUE INDEX `ent_types_type_key` ON `ent_types` (`type`);", - "INSERT INTO `ent_types` (`type`) VALUES ('users'), ('groups');", "", + "INSERT INTO `ent_types` (`type`) VALUES ('groups'), ('users');", + "", }, "\n") requireFileEqual(t, filepath.Join(p, "changes.sql"), changesSQL) // Adding another node will result in a new entry to the TypeTable (without actually creating it). _, err = db.ExecContext(ctx, changesSQL, nil, nil) require.NoError(t, err) - require.NoError(t, m.NamedDiff(ctx, "changes_2", &Table{Name: "pets", Columns: idCol, PrimaryKey: idCol})) + require.NoError(t, m.NamedDiff(ctx, "changes_2", petsTable)) requireFileEqual(t, filepath.Join(p, "changes_2.sql"), strings.Join([]string{ "CREATE TABLE `pets` (`id` integer NOT NULL PRIMARY KEY AUTOINCREMENT);",