diff --git a/go/logic/applier.go b/go/logic/applier.go index c025000b5..9f05a28d6 100644 --- a/go/logic/applier.go +++ b/go/logic/applier.go @@ -596,7 +596,8 @@ func (apl *Applier) createTargetTable(targetTableName string) error { } // createTargetTableFromStatement creates the table on the applier host to which the applier will -// apply changes. +// apply changes. In move-tables mode this executes on moveTablesTargetDB (the target cluster); +// in standard mode it executes on apl.db (the source/applier host). func (apl *Applier) createTargetTableFromStatement(targetTableName, createStatement string) error { targetDatabase := apl.migrationContext.GetTargetDatabaseName() apl.migrationContext.Log.Infof("Creating target table %s.%s", @@ -604,8 +605,13 @@ func (apl *Applier) createTargetTableFromStatement(targetTableName, createStatem sql.EscapeName(targetTableName), ) + db := apl.db + if apl.migrationContext.IsMoveTablesMode() { + db = apl.moveTablesTargetDB + } + err := func() error { - tx, err := apl.db.Begin() + tx, err := db.Begin() if err != nil { return err } @@ -640,12 +646,35 @@ func (apl *Applier) CreateGhostTable() error { return apl.createTargetTable(apl.migrationContext.GetGhostTableName()) } -// CreateTargetTable creates the target table on the target host (for move-tables) +// CreateTargetTable creates the target table on the target host (for move-tables). +// It aborts with an error if the target table already exists on the target cluster, +// to prevent silently writing into a table that has unrelated data or a different +// schema (move_table_mode.md ยง1.3: "Don't use IF NOT EXISTS for the target table. +// An existing table is an error condition, not a no-op."). func (apl *Applier) CreateTargetTable(createStatement string) error { if !apl.migrationContext.IsMoveTablesMode() { return errors.New("CreateTargetTable is only available in MoveTables mode") } - return apl.createTargetTableFromStatement(apl.originalTableName(), createStatement) + targetTableName := apl.originalTableName() + targetDatabase := apl.migrationContext.GetTargetDatabaseName() + + // Explicit pre-check: abort before any data is copied if the target table + // already exists. The CREATE TABLE would also fail (MySQL ERROR 1050), but + // this gives operators a clear gh-ost error message explaining what to do. + var count int + err := apl.moveTablesTargetDB.QueryRow( + "SELECT COUNT(*) FROM information_schema.tables WHERE table_schema=? AND table_name=?", + targetDatabase, targetTableName, + ).Scan(&count) + if err != nil { + return fmt.Errorf("failed to check for existing target table: %w", err) + } + if count > 0 { + return fmt.Errorf("target table %s.%s already exists on the target cluster. Aborting to prevent writing into a table with unrelated data. Drop the table manually if this is intentional", + sql.EscapeName(targetDatabase), sql.EscapeName(targetTableName)) + } + + return apl.createTargetTableFromStatement(targetTableName, createStatement) } // AlterGhost applies `alter` statement on ghost table diff --git a/go/logic/applier_test.go b/go/logic/applier_test.go index e0fd034b9..fdebf1013 100644 --- a/go/logic/applier_test.go +++ b/go/logic/applier_test.go @@ -770,6 +770,114 @@ func (suite *ApplierTestSuite) TestCreateGhostTable() { suite.Require().Equal("CREATE TABLE `_testing_gho` (\n `id` int DEFAULT NULL,\n `item_id` int DEFAULT NULL\n) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci", createDDL) } +// TestCreateTargetTable_HappyPath exercises #8207 AC #1: +// "A move table run targeting a clean target schema creates the migrated table +// with a SHOW CREATE TABLE output equivalent to the source's." +// +// It calls CreateTargetTable (not just IsMoveTablesMode()), asserts the table +// exists on the target database, verifies schema equivalence via SHOW CREATE TABLE, +// and confirms no table was accidentally created on the source. +func (suite *ApplierTestSuite) TestCreateTargetTable_HappyPath() { + ctx := context.Background() + + _, err := suite.db.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (id INT PRIMARY KEY, name VARCHAR(64), updated_at DATETIME);", getTestTableName())) + suite.Require().NoError(err) + + connectionConfig, err := getTestConnectionConfig(ctx, suite.mysqlContainer) + suite.Require().NoError(err) + + migrationContext := newTestMigrationContext() + migrationContext.MoveTables.TableNames = []string{testMysqlTableName} + migrationContext.MoveTables.TargetDatabase = testMysqlDatabaseOther + migrationContext.ApplierConnectionConfig = connectionConfig + migrationContext.MoveTables.ConnectionConfig = connectionConfig + migrationContext.SetConnectionConfig("innodb") + migrationContext.OriginalTableColumns = sql.NewColumnList([]string{"id", "name", "updated_at"}) + + applier := NewApplier(migrationContext) + defer applier.Teardown() + + err = applier.InitDBConnections() + suite.Require().NoError(err) + + var dummy, sourceCreateDDL string + err = suite.db.QueryRow(fmt.Sprintf("SHOW CREATE TABLE %s", getTestTableName())).Scan(&dummy, &sourceCreateDDL) + suite.Require().NoError(err) + + var count int + err = suite.otherDB.QueryRow( + "SELECT COUNT(*) FROM information_schema.tables WHERE table_schema=? AND table_name=?", + testMysqlDatabaseOther, testMysqlTableName, + ).Scan(&count) + suite.Require().NoError(err) + suite.Require().Equal(0, count, "precondition: target table must not exist before CreateTargetTable") + + err = applier.CreateTargetTable(sourceCreateDDL) + suite.Require().NoError(err) + + var targetTableName, targetCreateDDL string + err = suite.otherDB.QueryRow(fmt.Sprintf("SHOW CREATE TABLE `%s`.`%s`", testMysqlDatabaseOther, testMysqlTableName)).Scan(&targetTableName, &targetCreateDDL) + suite.Require().NoError(err) + suite.Require().Equal(testMysqlTableName, targetTableName) + suite.Require().Equal(sourceCreateDDL, targetCreateDDL, "target table schema must be equivalent to source") + + err = suite.otherDB.QueryRow( + "SELECT COUNT(*) FROM information_schema.tables WHERE table_schema=? AND table_name=?", + testMysqlDatabaseOther, testMysqlTableName, + ).Scan(&count) + suite.Require().NoError(err) + suite.Require().Equal(1, count, "target table must exist exactly once on the target database") +} + +// TestCreateTargetTable_AbortsIfExists exercises #8207 AC #2: +// "A move table run aborts before any data is copied if the target table already exists." +// +// It pre-creates the target table, then calls CreateTargetTable and asserts it +// returns a descriptive error (not just MySQL's raw ERROR 1050). +func (suite *ApplierTestSuite) TestCreateTargetTable_AbortsIfExists() { + ctx := context.Background() + + _, err := suite.db.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (id INT PRIMARY KEY);", getTestTableName())) + suite.Require().NoError(err) + + _, err = suite.otherDB.ExecContext(ctx, fmt.Sprintf("CREATE TABLE `%s`.`%s` (id INT PRIMARY KEY);", testMysqlDatabaseOther, testMysqlTableName)) + suite.Require().NoError(err) + + var count int + err = suite.otherDB.QueryRow( + "SELECT COUNT(*) FROM information_schema.tables WHERE table_schema=? AND table_name=?", + testMysqlDatabaseOther, testMysqlTableName, + ).Scan(&count) + suite.Require().NoError(err) + suite.Require().Equal(1, count, "precondition: target table must exist before CreateTargetTable") + + connectionConfig, err := getTestConnectionConfig(ctx, suite.mysqlContainer) + suite.Require().NoError(err) + + migrationContext := newTestMigrationContext() + migrationContext.MoveTables.TableNames = []string{testMysqlTableName} + migrationContext.MoveTables.TargetDatabase = testMysqlDatabaseOther + migrationContext.ApplierConnectionConfig = connectionConfig + migrationContext.MoveTables.ConnectionConfig = connectionConfig + migrationContext.SetConnectionConfig("innodb") + migrationContext.OriginalTableColumns = sql.NewColumnList([]string{"id"}) + + applier := NewApplier(migrationContext) + defer applier.Teardown() + + err = applier.InitDBConnections() + suite.Require().NoError(err) + + var dummy, sourceCreateDDL string + err = suite.db.QueryRow(fmt.Sprintf("SHOW CREATE TABLE %s", getTestTableName())).Scan(&dummy, &sourceCreateDDL) + suite.Require().NoError(err) + + err = applier.CreateTargetTable(sourceCreateDDL) + suite.Require().Error(err, "CreateTargetTable must return an error when target table already exists") + suite.Require().Contains(err.Error(), "already exists", "error message must mention 'already exists'") + suite.Require().Contains(err.Error(), testMysqlTableName, "error message must name the table") +} + func (suite *ApplierTestSuite) TestPanicOnWarningsInApplyIterationInsertQuerySucceedsWithUniqueKeyWarningInsertedByDMLEvent() { ctx := context.Background()