Merge pull request #7721 from dolthub/nicktobey/pk

Improve error messaging when encountering a primary key schema change during a merge.

go/libraries/doltcore/merge/merge_schema.go

@@ -16,7 +16,6 @@ package merge
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"sort"
 	"strings"
@@ -159,7 +158,8 @@ func (c ChkConflict) String() string {
 	return ""
 }
 
-var ErrMergeWithDifferentPks = errors.New("error: cannot merge two tables with different primary keys")
+var ErrMergeWithDifferentPks = errorkinds.NewKind("error: cannot merge because table %s has different primary keys")
+var ErrMergeWithDifferentPksFromAncestor = errorkinds.NewKind("error: cannot merge because table %s has different primary keys in its common ancestor")
 
 // SchemaMerge performs a three-way merge of |ourSch|, |theirSch|, and |ancSch|, and returns: the merged schema,
 // any schema conflicts identified, whether moving to the new schema requires a full table rewrite, and any
@@ -172,8 +172,11 @@ func SchemaMerge(ctx context.Context, format *storetypes.NomsBinFormat, ourSch,
 
 	// TODO: We'll remove this once it's possible to get diff and merge on different primary key sets
 	// TODO: decide how to merge different orders of PKS
-	if !schema.ArePrimaryKeySetsDiffable(format, ourSch, theirSch) || !schema.ArePrimaryKeySetsDiffable(format, ourSch, ancSch) {
-		return nil, SchemaConflict{}, mergeInfo, diffInfo, ErrMergeWithDifferentPks
+	if !schema.ArePrimaryKeySetsDiffable(format, ourSch, theirSch) {
+		return nil, SchemaConflict{}, mergeInfo, diffInfo, ErrMergeWithDifferentPks.New(tblName)
+	}
+	if !schema.ArePrimaryKeySetsDiffable(format, ourSch, ancSch) {
+		return nil, SchemaConflict{}, mergeInfo, diffInfo, ErrMergeWithDifferentPksFromAncestor.New(tblName)
 	}
 
 	var mergedCC *schema.ColCollection

go/libraries/doltcore/merge/schema_integration_test.go

@@ -16,7 +16,6 @@ package merge_test
 
 import (
 	"context"
-	"errors"
 	"testing"
 
 	"github.com/dolthub/go-mysql-server/sql"
@@ -460,7 +459,7 @@ var mergeSchemaConflictTests = []mergeSchemaConflictTest{
 			{commands.CommitCmd{}, []string{"-m", "modified branch other"}},
 			{commands.CheckoutCmd{}, []string{env.DefaultInitBranch}},
 		},
-		expectedErr: merge.ErrMergeWithDifferentPks,
+		expectedErr: merge.ErrMergeWithDifferentPks.New("test"),
 	},
 }
 
@@ -638,7 +637,8 @@ func testMergeSchemasWithConflicts(t *testing.T, test mergeSchemaConflictTest) {
 	_, actConflicts, mergeInfo, _, err := merge.SchemaMerge(context.Background(), types.Format_Default, mainSch, otherSch, ancSch, "test")
 	assert.False(t, mergeInfo.InvalidateSecondaryIndexes)
 	if test.expectedErr != nil {
-		assert.True(t, errors.Is(err, test.expectedErr))
+		// We don't use errors.Is here because errors generated by `Kind.New` compare stack traces in their `Is` implementation.
+		assert.Equal(t, err.Error(), test.expectedErr.Error(), "Expected error '%s', instead got '%s'", test.expectedErr.Error(), err.Error())
 		return
 	}
 

go/libraries/doltcore/sqle/enginetest/dolt_queries.go

@@ -5447,7 +5447,7 @@ var DoltCherryPickTests = []queries.ScriptTest{
 		Assertions: []queries.ScriptTestAssertion{
 			{
 				Query:          "CALL Dolt_Cherry_Pick(@commit1);",
-				ExpectedErrStr: "error: cannot merge two tables with different primary keys",
+				ExpectedErrStr: "error: cannot merge because table t has different primary keys",
 			},
 		},
 	},

go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go

@@ -2105,7 +2105,7 @@ var MergeScripts = []queries.ScriptTest{
 		Assertions: []queries.ScriptTestAssertion{
 			{
 				Query:          "CALL DOLT_MERGE('right');",
-				ExpectedErrStr: "error: cannot merge two tables with different primary keys",
+				ExpectedErrStr: "error: cannot merge because table t has different primary keys",
 			},
 		},
 	},

integration-tests/bats/cherry-pick.bats

@@ -609,5 +609,5 @@ teardown() {
     dolt checkout main
     run dolt cherry-pick branch1
     [ $status -eq 1 ]
-    [[ $output =~ "error: cannot merge two tables with different primary keys" ]] || false
+    [[ $output =~ "error: cannot merge because table test has different primary keys" ]] || false
 }

integration-tests/bats/primary-key-changes.bats

@@ -261,7 +261,7 @@ teardown() {
 
     run dolt merge test -m "merge other"
     [ "$status" -eq 1 ]
-    [[ "$output" =~ 'error: cannot merge two tables with different primary keys' ]] || false
+    [[ "$output" =~ 'error: cannot merge because table t has different primary keys' ]] || false
 }
 
 @test "primary-key-changes: merge on branch with primary key added throws an error" {
@@ -288,7 +288,36 @@ teardown() {
 
     run dolt merge test -m "merge other"
     [ "$status" -eq 1 ]
-    [[ "$output" =~ 'error: cannot merge two tables with different primary keys' ]] || false
+    [[ "$output" =~ 'error: cannot merge because table t has different primary keys' ]] || false
+}
+
+@test "primary-key-changes: merge on branches with different primary key from ancestor throws an error" {
+    dolt sql -q "create table t(pk int, val1 int, val2 int)"
+    dolt add .
+    dolt sql -q "INSERT INTO t values (1,1,1)"
+    dolt commit -am "cm1"
+    dolt checkout -b test
+
+    dolt sql -q "ALTER TABLE t add PRIMARY key (pk)"
+    dolt add .
+
+    run dolt status
+    [[ "$output" =~ 'Changes to be committed' ]] || false
+    [[ "$output" =~ ([[:space:]]*modified:[[:space:]]*t) ]] || false
+    ! [[ "$output" =~ 'deleted' ]] || false
+    ! [[ "$output" =~ 'new table' ]] || false
+
+    dolt commit -m "cm2"
+    dolt checkout main
+
+    dolt sql -q "ALTER TABLE t add PRIMARY key (pk)"
+    dolt sql -q "INSERT INTO t values (2,2,2)"
+    dolt commit -am "cm3"
+
+    run dolt merge test -m "merge other"
+    [ "$status" -eq 1 ]
+
+    [[ "$output" =~ 'error: cannot merge because table t has different primary keys' ]] || false
 }
 
 @test "primary-key-changes: diff on primary key schema change shows schema level diff but does not show row level diff" {
@@ -527,11 +556,11 @@ SQL
 
     run dolt merge test -m "merge other"
     [ "$status" -eq 1 ]
-    [[ "$output" =~ 'error: cannot merge two tables with different primary keys' ]] || false
+    [[ "$output" =~ 'error: cannot merge because table t has different primary keys' ]] || false
 
     run dolt sql -q "call dolt_merge('test')"
     [ "$status" -eq 1 ]
-    [[ "$output" =~ 'error: cannot merge two tables with different primary keys' ]] || false
+    [[ "$output" =~ 'error: cannot merge because table t has different primary keys' ]] || false
 
     skip "Dolt doesn't correctly store primary key order if it doesn't match the column order"
 }

integration-tests/bats/sql-cherry-pick.bats

@@ -506,5 +506,5 @@ SQL
     dolt checkout main
     run dolt sql -q "CALL DOLT_CHERRY_PICK('branch1')"
     [ $status -eq 1 ]
-    [[ $output =~ "error: cannot merge two tables with different primary keys" ]] || false
+    [[ $output =~ "error: cannot merge because table test has different primary keys" ]] || false
 }