dialect/sql/schema: inspect outside transaction in auto migrate (#4290)

Since SQLite does not allow enabling/disabling foreign key checks within a transaction, Atlas disabled foreign key checks before opening a transaction and re-enables them after commit/rollback. This involves checking for violations every time the auto migrate tool checks for changing. By opening a transaction only in case there are changes, we can avoid this when not needed.

Closes https://github.com/ariga/atlas/issues/3297
This commit is contained in:
Jannik Clausen
2025-01-09 08:32:47 +01:00
committed by GitHub
parent fc57ede2fe
commit d5c8b282de
2 changed files with 43 additions and 25 deletions

View File

@@ -664,6 +664,18 @@ func (a *Atlas) create(ctx context.Context, tables ...*Table) (err error) {
if err := a.sqlDialect.init(ctx); err != nil {
return err
}
a.atDriver, err = a.sqlDialect.atOpen(a.sqlDialect)
if err != nil {
return err
}
defer func() { a.atDriver = nil }()
plan, err := a.planInspect(ctx, a.sqlDialect, "changes", tables)
if err != nil {
return fmt.Errorf("sql/schema: %w", err)
}
if len(plan.Changes) == 0 {
return nil
}
// Open a transaction for backwards compatibility,
// even if the migration is not transactional.
tx, err := a.sqlDialect.Tx(ctx)
@@ -674,34 +686,23 @@ func (a *Atlas) create(ctx context.Context, tables ...*Table) (err error) {
if err != nil {
return err
}
defer func() { a.atDriver = nil }()
if err := func() error {
plan, err := a.planInspect(ctx, tx, "changes", tables)
if err != nil {
return err
}
// Apply plan (changes).
var applier Applier = ApplyFunc(func(ctx context.Context, tx dialect.ExecQuerier, plan *migrate.Plan) error {
for _, c := range plan.Changes {
if err := tx.Exec(ctx, c.Cmd, c.Args, nil); err != nil {
if c.Comment != "" {
err = fmt.Errorf("%s: %w", c.Comment, err)
}
return err
// Apply plan (changes).
var applier Applier = ApplyFunc(func(ctx context.Context, tx dialect.ExecQuerier, plan *migrate.Plan) error {
for _, c := range plan.Changes {
if err := tx.Exec(ctx, c.Cmd, c.Args, nil); err != nil {
if c.Comment != "" {
err = fmt.Errorf("%s: %w", c.Comment, err)
}
return err
}
return nil
})
for i := len(a.applyHook) - 1; i >= 0; i-- {
applier = a.applyHook[i](applier)
}
return applier.Apply(ctx, tx, plan)
}(); err != nil {
err = fmt.Errorf("sql/schema: %w", err)
if rerr := tx.Rollback(); rerr != nil {
err = fmt.Errorf("%w: %v", err, rerr)
}
return err
return nil
})
for i := len(a.applyHook) - 1; i >= 0; i-- {
applier = a.applyHook[i](applier)
}
if err = applier.Apply(ctx, tx, plan); err != nil {
return errors.Join(fmt.Errorf("sql/schema: %w", err), tx.Rollback())
}
return tx.Commit()
}

View File

@@ -241,6 +241,23 @@ func TestSQLite(t *testing.T) {
require.NoError(t, err)
defer vdrv.Close()
Versioned(t, vdrv, "sqlite3://file?mode=memory&cache=shared&_fk=1", versioned.NewClient(versioned.Driver(vdrv)))
// In case there are no changes, no transaction should be opened and foreign keys should not be touched.
fkdrv, err := sql.Open("sqlite3", "file:fk_ent?mode=memory&cache=shared&_fk=1")
require.NoError(t, err)
defer fkdrv.Close()
fkclient := entv1.NewClient(entv1.Driver(fkdrv), entv1.Log(func(a ...any) {
s := a[0].(string)
switch {
case strings.HasPrefix(s, "driver.Tx"):
t.Errorf("unexpected transaction, %q", s)
case strings.HasPrefix(s, "driver.Exec") && strings.Contains(s, "PRAGMA foreign_keys"):
t.Errorf("unexpected foreign keys pragma: %q", s)
}
}))
require.NoError(t, fkclient.Schema.Create(ctx))
// Apply again has no changes -> there should not be any pragma changes and no transaction to be opened.
require.NoError(t, fkclient.Debug().Schema.Create(ctx))
}
// https://github.com/ent/ent/issues/2954