Skip to content

Commit

Permalink
Merge pull request #7179 from dolthub/aaron/sqle-databaseprovider-no-…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
reltuk authored Dec 16, 2023
2 parents 91d31ad + 33927ec commit b16a3d3
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
11 changes: 9 additions & 2 deletions go/libraries/doltcore/sqle/database_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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 {
Expand Down
30 changes: 30 additions & 0 deletions go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b16a3d3

Please sign in to comment.