diff --git a/go/libraries/doltcore/doltdb/root_val.go b/go/libraries/doltcore/doltdb/root_val.go index 2ec53938d94..5d07088766c 100644 --- a/go/libraries/doltcore/doltdb/root_val.go +++ b/go/libraries/doltcore/doltdb/root_val.go @@ -1369,7 +1369,7 @@ func ValidateTagUniqueness(ctx context.Context, root RootValue, tableName string err = sch.GetAllCols().Iter(func(tag uint64, col schema.Column) (stop bool, err error) { oldTableName, ok := existing.Get(tag) if ok && oldTableName != tableName { - return true, schema.ErrTagPrevUsed(tag, col.Name, tableName, oldTableName) + return true, schema.NewErrTagPrevUsed(tag, col.Name, tableName, oldTableName) } return false, nil }) diff --git a/go/libraries/doltcore/merge/merge.go b/go/libraries/doltcore/merge/merge.go index f81b2946971..5ae61e265ff 100644 --- a/go/libraries/doltcore/merge/merge.go +++ b/go/libraries/doltcore/merge/merge.go @@ -256,7 +256,7 @@ func MergeRoots( // If a Full-Text table was both modified and deleted, then we want to ignore the deletion. // If there's a true conflict, then the parent table will catch the conflict. stats = &MergeStats{Operation: TableModified} - } else if errors.Is(ErrTableDeletedAndSchemaModified, err) { + } else if errors.Is(ErrTableDeletedAndSchemaModified, err) || errors.Is(ErrTableDeletedAndModified, err) { tblToStats[tblName] = &MergeStats{ Operation: TableModified, SchemaConflicts: 1, @@ -307,6 +307,11 @@ func MergeRoots( } mergedRoot, err = mergedRoot.PutTable(ctx, tblName, mergedTable.table) + var errTagPreviouslyUsed schema.ErrTagPrevUsed + if errors.As(err, &errTagPreviouslyUsed) { + return nil, fmt.Errorf("cannot merge, column %s on table %s has duplicate tag as table %s. This was likely because one of the tables is a rename of the other", + errTagPreviouslyUsed.NewColName, errTagPreviouslyUsed.NewTableName, errTagPreviouslyUsed.OldTableName) + } if err != nil { return nil, err } diff --git a/go/libraries/doltcore/schema/tag.go b/go/libraries/doltcore/schema/tag.go index c4caff913c8..4f6d1d7e0a8 100644 --- a/go/libraries/doltcore/schema/tag.go +++ b/go/libraries/doltcore/schema/tag.go @@ -30,8 +30,26 @@ const ( ReservedTagMin uint64 = 1 << 50 ) -func ErrTagPrevUsed(tag uint64, newColName, newTableName, oldTableName string) error { - return fmt.Errorf("cannot create column %s on table %s, the tag %d was already used in table %s", newColName, newTableName, tag, oldTableName) +type ErrTagPrevUsed struct { + Tag uint64 + NewColName string + NewTableName string + OldTableName string +} + +var _ error = ErrTagPrevUsed{} + +func (e ErrTagPrevUsed) Error() string { + return fmt.Sprintf("cannot create column %s on table %s, the tag %d was already used in table %s", e.NewColName, e.NewTableName, e.Tag, e.OldTableName) +} + +func NewErrTagPrevUsed(tag uint64, newColName, newTableName, oldTableName string) ErrTagPrevUsed { + return ErrTagPrevUsed{ + Tag: tag, + NewColName: newColName, + NewTableName: newTableName, + OldTableName: oldTableName, + } } type TagMapping map[uint64]string diff --git a/go/libraries/doltcore/sqle/alterschema.go b/go/libraries/doltcore/sqle/alterschema.go index 27e241e953f..5176758b350 100755 --- a/go/libraries/doltcore/sqle/alterschema.go +++ b/go/libraries/doltcore/sqle/alterschema.go @@ -133,7 +133,7 @@ func validateNewColumn( cols := sch.GetAllCols() err = cols.Iter(func(currColTag uint64, currCol schema.Column) (stop bool, err error) { if currColTag == tag { - return false, schema.ErrTagPrevUsed(tag, newColName, tblName, tblName) + return false, schema.NewErrTagPrevUsed(tag, newColName, tblName, tblName) } else if strings.EqualFold(currCol.Name, newColName) { return true, fmt.Errorf("A column with the name %s already exists in table %s.", newColName, tblName) } @@ -150,7 +150,7 @@ func validateNewColumn( return err } if found { - return schema.ErrTagPrevUsed(tag, newColName, tblName, oldTblName.Name) + return schema.NewErrTagPrevUsed(tag, newColName, tblName, oldTblName.Name) } return nil diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go index e6bc6ed0d0a..8b39cce636a 100644 --- a/go/libraries/doltcore/sqle/database.go +++ b/go/libraries/doltcore/sqle/database.go @@ -1569,7 +1569,7 @@ func (db Database) createDoltTable(ctx *sql.Context, tableName string, schemaNam return true, err } if exists && oldTableName.Name != tableName { - errStr := schema.ErrTagPrevUsed(tag, col.Name, tableName, oldTableName.Name).Error() + errStr := schema.NewErrTagPrevUsed(tag, col.Name, tableName, oldTableName.Name).Error() conflictingTbls = append(conflictingTbls, errStr) } return false, nil diff --git a/go/libraries/doltcore/sqle/dtables/schema_conflicts_table.go b/go/libraries/doltcore/sqle/dtables/schema_conflicts_table.go index 050d38d3ea1..890c8903fbe 100644 --- a/go/libraries/doltcore/sqle/dtables/schema_conflicts_table.go +++ b/go/libraries/doltcore/sqle/dtables/schema_conflicts_table.go @@ -167,6 +167,7 @@ func newSchemaConflict(ctx *sql.Context, table doltdb.TableName, baseRoot doltdb baseFKs, _ := fkc.KeysForTable(table) + var description string var base string if baseSch != nil { var err error @@ -187,6 +188,12 @@ func newSchemaConflict(ctx *sql.Context, table doltdb.TableName, baseRoot doltdb } } else { ours = "" + if schema.SchemasAreEqual(baseSch, c.FromSch) { + // If the schemas are equal, then this conflict represents a dropped table conflicting with a data conflict. + description = "cannot merge a table deletion with data modification" + } else { + description = "cannot merge a table deletion with schema modification" + } } var theirs string @@ -198,6 +205,12 @@ func newSchemaConflict(ctx *sql.Context, table doltdb.TableName, baseRoot doltdb } } else { theirs = "" + if schema.SchemasAreEqual(baseSch, c.ToSch) { + // If the schemas are equal, then this conflict represents a dropped table conflicting with a data conflict. + description = "cannot merge a table deletion with data modification" + } else { + description = "cannot merge a table deletion with schema modification" + } } if c.ToSch == nil || c.FromSch == nil { @@ -206,7 +219,7 @@ func newSchemaConflict(ctx *sql.Context, table doltdb.TableName, baseRoot doltdb baseSch: base, ourSch: ours, theirSch: theirs, - description: "cannot merge a table deletion with schema modification", + description: description, }, nil } diff --git a/integration-tests/bats/cherry-pick.bats b/integration-tests/bats/cherry-pick.bats index 8421af0e4e2..88f826452ea 100644 --- a/integration-tests/bats/cherry-pick.bats +++ b/integration-tests/bats/cherry-pick.bats @@ -125,7 +125,7 @@ teardown() { dolt checkout main run dolt cherry-pick branch1 [ "$status" -eq "1" ] - [[ "$output" =~ "conflict: table with same name deleted and modified" ]] || false + [[ "$output" =~ "table was modified in one branch and deleted in the other" ]] || false run dolt sql -q "SHOW TABLES" -r csv [[ ! "$output" =~ "branch1table" ]] || false @@ -137,7 +137,7 @@ teardown() { dolt checkout main run dolt cherry-pick branch1 [ "$status" -eq "1" ] - [[ "$output" =~ "conflict: table with same name deleted and modified" ]] || false + [[ "$output" =~ "table was modified in one branch and deleted in the other" ]] || false run dolt sql -q "SHOW TABLES" -r csv [[ ! "$output" =~ "branch1table" ]] || false @@ -149,7 +149,7 @@ teardown() { dolt checkout main run dolt cherry-pick branch1 [ "$status" -eq "1" ] - [[ "$output" =~ "conflict: table with same name deleted and modified" ]] || false + [[ "$output" =~ "table was modified in one branch and deleted in the other" ]] || false run dolt sql -q "SHOW TABLES" -r csv [[ ! "$output" =~ "branch1table" ]] || false diff --git a/integration-tests/bats/merge.bats b/integration-tests/bats/merge.bats index 2c319cd7b60..c76a47f1c98 100644 --- a/integration-tests/bats/merge.bats +++ b/integration-tests/bats/merge.bats @@ -751,7 +751,19 @@ SQL run dolt merge feature-branch log_status_eq 1 - [[ "$output" =~ "conflict: table with same name deleted and modified" ]] || false + [[ "$output" =~ "CONFLICT (schema): Merge conflict in test" ]] || false + + run dolt conflicts cat . + [[ "$output" =~ "+------------+-------------------------------------------------------------------+-------------------------------------------------------------------+------------------------------------------------------+" ]] || false + [[ "$output" =~ "| our_schema | their_schema | base_schema | description |" ]] || false + [[ "$output" =~ "+------------+-------------------------------------------------------------------+-------------------------------------------------------------------+------------------------------------------------------+" ]] || false + [[ "$output" =~ '| | CREATE TABLE `test2` ( | CREATE TABLE `test2` ( | cannot merge a table deletion with data modification |' ]] || false + [[ "$output" =~ '| | `pk` int NOT NULL, | `pk` int NOT NULL, | |' ]] || false + [[ "$output" =~ '| | `c1` int, | `c1` int, | |' ]] || false + [[ "$output" =~ '| | PRIMARY KEY (`pk`) | PRIMARY KEY (`pk`) | |' ]] || false + [[ "$output" =~ '| | ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin; | ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin; | |' ]] || false + [[ "$output" =~ "+------------+-------------------------------------------------------------------+-------------------------------------------------------------------+------------------------------------------------------+" ]] || false + } @test "merge: merge a branch that edits the schema of a deleted table" { @@ -812,7 +824,20 @@ SQL run dolt merge feature-branch log_status_eq 1 - [[ "$output" =~ "conflict: table with same name deleted and modified" ]] || false + [[ "$output" =~ "CONFLICT (schema): Merge conflict in test" ]] || false + + run dolt conflicts cat . + echo "$output" + [[ "$output" =~ "+-------------------------------------------------------------------+--------------+-------------------------------------------------------------------+------------------------------------------------------+" ]] || false + [[ "$output" =~ "| our_schema | their_schema | base_schema | description |" ]] || false + [[ "$output" =~ "+-------------------------------------------------------------------+--------------+-------------------------------------------------------------------+------------------------------------------------------+" ]] || false + [[ "$output" =~ '| CREATE TABLE `test2` ( | | CREATE TABLE `test2` ( | cannot merge a table deletion with data modification |' ]] || false + [[ "$output" =~ '| `pk` int NOT NULL, | | `pk` int NOT NULL, | |' ]] || false + [[ "$output" =~ '| `c1` int, | | `c1` int, | |' ]] || false + [[ "$output" =~ '| PRIMARY KEY (`pk`) | | PRIMARY KEY (`pk`) | |' ]] || false + [[ "$output" =~ '| ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin; | | ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin; | |' ]] || false + [[ "$output" =~ "+-------------------------------------------------------------------+--------------+-------------------------------------------------------------------+------------------------------------------------------+" ]] || false + } @test "merge: merge a branch that deletes a schema-edited table" { @@ -1076,7 +1101,17 @@ SQL run dolt merge merge_branch log_status_eq 1 - [[ "$output" =~ "conflict: table with same name deleted and modified" ]] || false + run dolt conflicts cat . + [[ "$output" =~ "+------------+-------------------------------------------------------------------+-------------------------------------------------------------------+------------------------------------------------------+" ]] || false + [[ "$output" =~ "| our_schema | their_schema | base_schema | description |" ]] || false + [[ "$output" =~ "+------------+-------------------------------------------------------------------+-------------------------------------------------------------------+------------------------------------------------------+" ]] || false + [[ "$output" =~ '| | CREATE TABLE `test1` ( | CREATE TABLE `test1` ( | cannot merge a table deletion with data modification |' ]] || false + [[ "$output" =~ '| | `pk` int NOT NULL, | `pk` int NOT NULL, | |' ]] || false + [[ "$output" =~ '| | `c1` int, | `c1` int, | |' ]] || false + [[ "$output" =~ '| | PRIMARY KEY (`pk`) | PRIMARY KEY (`pk`) | |' ]] || false + [[ "$output" =~ '| | ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin; | ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin; | |' ]] || false + [[ "$output" =~ "+------------+-------------------------------------------------------------------+-------------------------------------------------------------------+------------------------------------------------------+" ]] || false + } @test "merge: ourRoot renames, theirRoot modifies the schema" { @@ -1119,7 +1154,7 @@ SQL run dolt merge merge_branch log_status_eq 1 - [[ "$output" =~ "conflict: table with same name deleted and modified" ]] || false + [[ "$output" =~ "cannot merge, column pk on table new_name has duplicate tag as table test1. This was likely because one of the tables is a rename of the other" ]] || false } @test "merge: ourRoot modifies the schema, theirRoot renames" { @@ -1134,8 +1169,7 @@ SQL run dolt merge merge_branch log_status_eq 1 - [[ "$output" =~ "cannot create column pk on table new_name" ]] || false - [[ "$output" =~ "already used in table test1" ]] || false + [[ "$output" =~ "cannot merge, column pk on table new_name has duplicate tag as table test1. This was likely because one of the tables is a rename of the other" ]] || false } @test "merge: dolt merge commits successful non-fast-forward merge" { diff --git a/integration-tests/bats/sql-cherry-pick.bats b/integration-tests/bats/sql-cherry-pick.bats index 5f279e7971c..09f3acc622e 100644 --- a/integration-tests/bats/sql-cherry-pick.bats +++ b/integration-tests/bats/sql-cherry-pick.bats @@ -139,7 +139,7 @@ SQL dolt checkout main run dolt sql -q "CALL DOLT_CHERRY_PICK('branch1')" [ "$status" -eq "1" ] - [[ "$output" =~ "conflict: table with same name deleted and modified" ]] || false + [[ "$output" =~ "table was modified in one branch and deleted in the other" ]] || false run dolt sql -q "SHOW TABLES" -r csv [[ ! "$output" =~ "branch1table" ]] || false @@ -151,7 +151,7 @@ SQL dolt checkout main run dolt sql -q "CALL DOLT_CHERRY_PICK('branch1')" [ "$status" -eq "1" ] - [[ "$output" =~ "conflict: table with same name deleted and modified" ]] || false + [[ "$output" =~ "table was modified in one branch and deleted in the other" ]] || false run dolt sql -q "SHOW TABLES" -r csv [[ ! "$output" =~ "branch1table" ]] || false @@ -163,7 +163,7 @@ SQL dolt checkout main run dolt sql -q "CALL DOLT_CHERRY_PICK('branch1')" [ "$status" -eq "1" ] - [[ "$output" =~ "conflict: table with same name deleted and modified" ]] || false + [[ "$output" =~ "table was modified in one branch and deleted in the other" ]] || false run dolt sql -q "SHOW TABLES" -r csv [[ ! "$output" =~ "branch1table" ]] || false