Skip to content

Commit

Permalink
Merge pull request #7107 from dolthub/aaron/sqle-database-provider-cl…
Browse files Browse the repository at this point in the history
…one-dbLocations-fix

go/libraries/doltcore/sqle: DatabaseProvider: Fix a bug where a database created with call dolt_clone() would not be dropped until the server was restarted.
  • Loading branch information
reltuk authored Dec 6, 2023
2 parents 7c88abe + 0e1bbfc commit db3472a
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 34 deletions.
42 changes: 8 additions & 34 deletions go/libraries/doltcore/sqle/database_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ func (p *DoltDatabaseProvider) CloneDatabaseFromRemote(
return fmt.Errorf("cannot create DB, file exists at %s", dbName)
}

dEnv, err := p.cloneDatabaseFromRemote(ctx, dbName, remoteName, branch, remoteUrl, remoteParams)
err := p.cloneDatabaseFromRemote(ctx, dbName, remoteName, branch, remoteUrl, remoteParams)
if err != nil {
// Make a best effort to clean up any artifacts on disk from a failed clone
// before we return the error
Expand All @@ -499,7 +499,7 @@ func (p *DoltDatabaseProvider) CloneDatabaseFromRemote(
return err
}

return ConfigureReplicationDatabaseHook(ctx, p, dbName, dEnv)
return nil
}

// cloneDatabaseFromRemote encapsulates the inner logic for cloning a database so that if any error
Expand All @@ -510,60 +510,34 @@ func (p *DoltDatabaseProvider) cloneDatabaseFromRemote(
ctx *sql.Context,
dbName, remoteName, branch, remoteUrl string,
remoteParams map[string]string,
) (*env.DoltEnv, error) {
) error {
if p.remoteDialer == nil {
return nil, fmt.Errorf("unable to clone remote database; no remote dialer configured")
return fmt.Errorf("unable to clone remote database; no remote dialer configured")
}

// TODO: params for AWS, others that need them
r := env.NewRemote(remoteName, remoteUrl, nil)
srcDB, err := r.GetRemoteDB(ctx, types.Format_Default, p.remoteDialer)
if err != nil {
return nil, err
return err
}

dEnv, err := actions.EnvForClone(ctx, srcDB.ValueReadWriter().Format(), r, dbName, p.fs, "VERSION", env.GetCurrentUserHomeDir)
if err != nil {
return nil, err
return err
}

err = actions.CloneRemote(ctx, srcDB, remoteName, branch, false, dEnv)
if err != nil {
return nil, err
return err
}

err = dEnv.RepoStateWriter().UpdateBranch(dEnv.RepoState.CWBHeadRef().GetPath(), env.BranchConfig{
Merge: dEnv.RepoState.Head,
Remote: remoteName,
})

fkChecks, err := ctx.GetSessionVariable(ctx, "foreign_key_checks")
if err != nil {
return nil, err
}

opts := editor.Options{
Deaf: dEnv.DbEaFactory(),
// TODO: this doesn't seem right, why is this getting set in the constructor to the DB
ForeignKeyChecksDisabled: fkChecks.(int8) == 0,
}

db, err := NewDatabase(ctx, dbName, dEnv.DbData(), opts)
if err != nil {
return nil, err
}

// If we have an initialization hook, invoke it. By default, this will
// be ConfigureReplicationDatabaseHook, which will setup replication
// for the new database if a remote url template is set.
err = p.InitDatabaseHook(ctx, p, dbName, dEnv)
if err != nil {
return nil, err
}

p.databases[formatDbMapKeyName(db.Name())] = db

return dEnv, nil
return p.registerNewDatabase(ctx, dbName, dEnv)
}

// DropDatabase implements the sql.MutableDatabaseProvider interface
Expand Down
31 changes: 31 additions & 0 deletions integration-tests/bats/clone-drop.bats
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/usr/bin/env bats
load $BATS_TEST_DIRNAME/helper/common.bash

setup() {
setup_no_dolt_init
}

teardown() {
stop_sql_server
assert_feature_version
teardown_common
}

@test "clone-drop: clone a database and then drop it" {
mkdir repo
cd repo
dolt init
dolt remote add pushed 'file://../pushed'
dolt push pushed main:main
dolt sql -q 'call dolt_clone("file://../pushed", "cloned"); drop database cloned;'
}

@test "clone-drop: sql-server: clone a database and then drop it" {
mkdir repo
cd repo
dolt init
dolt remote add pushed 'file://../pushed'
dolt push pushed main:main
start_sql_server
dolt sql -q 'call dolt_clone("file://../pushed", "cloned"); drop database cloned;'
}
1 change: 1 addition & 0 deletions integration-tests/bats/helper/local-remote.bash
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ SKIP_SERVER_TESTS=$(cat <<-EOM
~1pk5col-strings.bats~
~sql-tags.bats~
~empty-repo.bats~
~clone-drops.bats~
~verify-constraints.bats~
~db-revision-specifiers.bats~
~ignore.bats~
Expand Down

0 comments on commit db3472a

Please sign in to comment.