Merge pull request #8643 from dolthub/zachmu/enginetest10

Support for schema names in table renames
dolthub · Dec 7, 2024 · aba8d2a · aba8d2a
2 parents 161a217 + f783a21
commit aba8d2a
Show file tree

Hide file tree

Showing 8 changed files with 32 additions and 19 deletions.
diff --git a/go/go.mod b/go/go.mod
@@ -57,7 +57,7 @@ require (
 	github.com/cespare/xxhash/v2 v2.2.0
 	github.com/creasty/defaults v1.6.0
 	github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2
-	github.com/dolthub/go-mysql-server v0.18.2-0.20241206195637-151301aa7662
+	github.com/dolthub/go-mysql-server v0.18.2-0.20241206235354-d3cdbb6be4b4
 	github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63
 	github.com/dolthub/swiss v0.1.0
 	github.com/goccy/go-json v0.10.2

diff --git a/go/go.sum b/go/go.sum
@@ -183,8 +183,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U=
 github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0=
 github.com/dolthub/go-icu-regex v0.0.0-20240916130659-0118adc6b662 h1:aC17hZD6iwzBwwfO5M+3oBT5E5gGRiQPdn+vzpDXqIA=
 github.com/dolthub/go-icu-regex v0.0.0-20240916130659-0118adc6b662/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168=
-github.com/dolthub/go-mysql-server v0.18.2-0.20241206195637-151301aa7662 h1:tqpaQ8F4wamAf7cpGkRzxEIu6bzHAXKOFnctAYX3oyo=
-github.com/dolthub/go-mysql-server v0.18.2-0.20241206195637-151301aa7662/go.mod h1:5j8LgJ+mRFpm4TG9Dtp7XQRTq13GlwpkPGPicMx2tl8=
+github.com/dolthub/go-mysql-server v0.18.2-0.20241206235354-d3cdbb6be4b4 h1:2T/vff9AhzGoaqZAgl7bNYFrqeZ9ft38t7tKXTM9NY0=
+github.com/dolthub/go-mysql-server v0.18.2-0.20241206235354-d3cdbb6be4b4/go.mod h1:5j8LgJ+mRFpm4TG9Dtp7XQRTq13GlwpkPGPicMx2tl8=
 github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63 h1:OAsXLAPL4du6tfbBgK0xXHZkOlos63RdKYS3Sgw/dfI=
 github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63/go.mod h1:lV7lUeuDhH5thVGDCKXbatwKy2KW80L4rMT46n+Y2/Q=
 github.com/dolthub/ishell v0.0.0-20240701202509-2b217167d718 h1:lT7hE5k+0nkBdj/1UOSFwjWpNxf+LCApbRHgnCA17XE=

diff --git a/go/libraries/doltcore/sqle/alterschema.go b/go/libraries/doltcore/sqle/alterschema.go
@@ -28,15 +28,14 @@ import (
 )
 
 // renameTable renames a table with in a RootValue and returns the updated root.
-func renameTable(ctx context.Context, root doltdb.RootValue, oldName, newName string) (doltdb.RootValue, error) {
+func renameTable(ctx context.Context, root doltdb.RootValue, oldName, newName doltdb.TableName) (doltdb.RootValue, error) {
 	if newName == oldName {
 		return root, nil
 	} else if root == nil {
 		panic("invalid parameters")
 	}
 
-	// TODO: schema name
-	return root.RenameTable(ctx, doltdb.TableName{Name: oldName}, doltdb.TableName{Name: newName})
+	return root.RenameTable(ctx, oldName, newName)
 }
 
 // Nullable represents whether a column can have a null value.

diff --git a/go/libraries/doltcore/sqle/alterschema_test.go b/go/libraries/doltcore/sqle/alterschema_test.go
@@ -96,7 +96,7 @@ func TestRenameTable(t *testing.T) {
 			require.NoError(t, err)
 			beforeSch := schemas[doltdb.TableName{Name: tt.oldName}]
 
-			updatedRoot, err := renameTable(ctx, root, tt.oldName, tt.newName)
+			updatedRoot, err := renameTable(ctx, root, doltdb.TableName{Name: tt.oldName}, doltdb.TableName{Name: tt.newName})
 			if len(tt.expectedErr) > 0 {
 				assert.Error(t, err)
 				assert.Contains(t, err.Error(), tt.expectedErr)

diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go
@@ -1726,12 +1726,23 @@ func (db Database) RenameTable(ctx *sql.Context, oldName, newName string) error
 		return ErrInvalidTableName.New(newName)
 	}
 
-	if _, ok, _ := db.GetTableInsensitive(ctx, newName); ok {
-		return sql.ErrTableAlreadyExists.New(newName)
+	oldNameWithSchema, _, exists, err := resolve.Table(ctx, root, oldName)
+	if err != nil {
+		return err
 	}
 
-	newRoot, err := renameTable(ctx, root, oldName, newName)
+	if !exists {
+		return sql.ErrTableNotFound.New(oldName)
+	}
+
+	// TODO: we have no way to rename a table to a different schema, need to change the GMS interface for that
+	newNameWithSchema := doltdb.TableName{Schema: oldNameWithSchema.Schema, Name: newName}
+	_, exists, err = root.ResolveTableName(ctx, newNameWithSchema)
+	if exists {
+		return sql.ErrTableAlreadyExists.New(newName)
+	}
 
+	newRoot, err := renameTable(ctx, root, oldNameWithSchema, newNameWithSchema)
 	if err != nil {
 		return err
 	}

diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_tests.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_tests.go
@@ -683,6 +683,7 @@ func RunDoltRevisionDbScriptsTest(t *testing.T, h DoltEnginetestHarness) {
 	require.NoError(t, err)
 	defer e.Close()
 	ctx := h.NewContext()
+	ctx.SetCurrentDatabase("mydb")
 
 	setupScripts := []setup.SetupScript{
 		{"create table t01 (pk int primary key, c1 int)"},

diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_harness.go b/go/libraries/doltcore/sqle/enginetest/dolt_harness.go
@@ -270,6 +270,10 @@ func (d *DoltHarness) NewEngine(t *testing.T) (enginetest.QueryEngine, error) {
 			if err != nil {
 				return nil, err
 			}
+
+			// Get a fresh session after running setup scripts, since some setup scripts can change the session state
+			d.session, err = dsess.NewDoltSession(enginetest.NewBaseSession(), d.provider, d.multiRepoEnv.Config(), d.branchControl, d.statsPro, writer.NewWriteSession)
+			require.NoError(t, err)
 		}
 
 		if d.configureStats {
@@ -304,16 +308,14 @@ func (d *DoltHarness) NewEngine(t *testing.T) (enginetest.QueryEngine, error) {
 	d.engine.Analyzer.Catalog.MySQLDb.AddRootAccount()
 	d.engine.Analyzer.Catalog.StatsProvider = statspro.NewProvider(d.provider.(*sqle.DoltDatabaseProvider), statsnoms.NewNomsStatsFactory(d.multiRepoEnv.RemoteDialProvider()))
 
-	// Get a fresh session if we are reusing the engine
-	if !initializeEngine {
-		var err error
-		d.session, err = dsess.NewDoltSession(enginetest.NewBaseSession(), d.provider, d.multiRepoEnv.Config(), d.branchControl, d.statsPro, writer.NewWriteSession)
-		require.NoError(t, err)
-	}
-
+	var err error
 	ctx := enginetest.NewContext(d)
 	e, err := enginetest.RunSetupScripts(ctx, d.engine, d.resetScripts(), d.SupportsNativeIndexCreation())
 
+	// Get a fresh session after running setup scripts, since some setup scripts can change the session state
+	d.session, err = dsess.NewDoltSession(enginetest.NewBaseSession(), d.provider, d.multiRepoEnv.Config(), d.branchControl, d.statsPro, writer.NewWriteSession)
+	require.NoError(t, err)
+
 	return e, err
 }
 

diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go
@@ -1661,7 +1661,7 @@ var MergeScripts = []queries.ScriptTest{
 				Expected: []sql.Row{{doltCommit, 1, 0, "merge successful"}},
 			},
 			{
-				Query:    "INSERT INTO t VALUES (NULL,5),(6,6),(NULL,7);",
+				Query:    "INSERT INTO t(c0) VALUES (5),(6),(7);",
 				Expected: []sql.Row{{types.OkResult{RowsAffected: 3, InsertID: 5}}},
 			},
 			{
@@ -1698,7 +1698,7 @@ var MergeScripts = []queries.ScriptTest{
 				Expected: []sql.Row{{doltCommit, 0, 0, "merge successful"}},
 			},
 			{
-				Query:    "INSERT INTO t VALUES (NULL,6),(7,7),(NULL,8);",
+				Query:    "INSERT INTO t(c0) VALUES (6),(7),(8);",
 				Expected: []sql.Row{{types.OkResult{RowsAffected: 3, InsertID: 6}}},
 			},
 			{