Merge pull request #7179 from dolthub/aaron/sqle-databaseprovider-no-straggling-uninited-databases

go: sqle: DoltDatabaseProvider: If we encounter an error while creating a database, try to clean up after ourselves so we do not leave partially initialized database state around.
This commit is contained in:
Aaron Son
2023-12-15 17:32:29 -08:00
committed by GitHub
2 changed files with 39 additions and 2 deletions

View File

@@ -362,7 +362,7 @@ func (p *DoltDatabaseProvider) CreateDatabase(ctx *sql.Context, name string) err
return p.CreateCollatedDatabase(ctx, name, sql.Collation_Default)
}
func (p *DoltDatabaseProvider) CreateCollatedDatabase(ctx *sql.Context, name string, collation sql.CollationID) error {
func (p *DoltDatabaseProvider) CreateCollatedDatabase(ctx *sql.Context, name string, collation sql.CollationID) (err error) {
p.mu.Lock()
defer p.mu.Unlock()
@@ -373,10 +373,17 @@ func (p *DoltDatabaseProvider) CreateCollatedDatabase(ctx *sql.Context, name str
return fmt.Errorf("Cannot create DB, file exists at %s", name)
}
err := p.fs.MkDirs(name)
err = p.fs.MkDirs(name)
if err != nil {
return err
}
defer func() {
// We do not want to leave this directory behind if we do not
// successfully create the database.
if err != nil {
p.fs.Delete(name, true /* force / recursive */)
}
}()
newFs, err := p.fs.WithWorkingDir(name)
if err != nil {

View File

@@ -2970,6 +2970,36 @@ func TestThreeWayMergeWithSchemaChangeScriptsPrepared(t *testing.T) {
})
}
// If CREATE DATABASE has an error within the DatabaseProvider, it should not
// leave behind intermediate filesystem state.
func TestCreateDatabaseErrorCleansUp(t *testing.T) {
dh := newDoltHarness(t)
require.NotNil(t, dh)
e, err := dh.NewEngine(t)
require.NoError(t, err)
require.NotNil(t, e)
dh.provider.(*sqle.DoltDatabaseProvider).InitDatabaseHook = func(_ *sql.Context, _ *sqle.DoltDatabaseProvider, name string, _ *env.DoltEnv) error {
if name == "cannot_create" {
return fmt.Errorf("there was an error initializing this database. abort!")
}
return nil
}
err = dh.provider.CreateDatabase(enginetest.NewContext(dh), "can_create")
require.NoError(t, err)
err = dh.provider.CreateDatabase(enginetest.NewContext(dh), "cannot_create")
require.Error(t, err)
fs := dh.multiRepoEnv.FileSystem()
exists, _ := fs.Exists("cannot_create")
require.False(t, exists)
exists, isDir := fs.Exists("can_create")
require.True(t, exists)
require.True(t, isDir)
}
// runMergeScriptTestsInBothDirections creates a new test run, named |name|, and runs the specified merge |tests|
// in both directions (right to left merge, and left to right merge). If
// |runAsPrepared| is true then the test scripts will be run using the prepared