Merge pull request #8399 from dolthub/zachmu/merge-2

Use the a type comparator for merge, not the generic comparator

go/libraries/doltcore/merge/merge_prolly_rows.go

@@ -1766,7 +1766,7 @@ func (m *valueMerger) processBaseColumn(ctx context.Context, i int, left, right,
 		if err != nil {
 			return false, err
 		}
-		if isEqual(i, baseCol, rightCol, m.rightVD.Types[rightColIdx]) {
+		if isEqual(m.baseVD.Comparator(), i, baseCol, rightCol, m.rightVD.Types[rightColIdx]) {
 			// right column did not change, so there is no conflict.
 			return false, nil
 		}
@@ -1789,7 +1789,7 @@ func (m *valueMerger) processBaseColumn(ctx context.Context, i int, left, right,
 		if err != nil {
 			return false, err
 		}
-		if isEqual(i, baseCol, leftCol, m.leftVD.Types[leftColIdx]) {
+		if isEqual(m.baseVD.Comparator(), i, baseCol, leftCol, m.leftVD.Types[leftColIdx]) {
 			// left column did not change, so there is no conflict.
 			return false, nil
 		}
@@ -1880,7 +1880,7 @@ func (m *valueMerger) processColumn(ctx *sql.Context, i int, left, right, base v
 			return nil, false, err
 		}
 
-		if isEqual(i, leftCol, rightCol, resultType) {
+		if isEqual(m.leftVD.Comparator(), i, leftCol, rightCol, resultType) {
 			// Columns are equal, returning either would be correct.
 			// However, for certain types the two columns may have different bytes.
 			// We need to ensure that merges are deterministic regardless of the merge direction.
@@ -1900,7 +1900,7 @@ func (m *valueMerger) processColumn(ctx *sql.Context, i int, left, right, base v
 		return nil, true, nil
 	}
 
-	// We can now assume that both left are right contain byte-level changes to an existing column.
+	// We can now assume that both left and right contain byte-level changes to an existing column.
 	// But we need to know if those byte-level changes represent a modification to the underlying value,
 	// and whether those changes represent the *same* modification, otherwise there's a conflict.
 
@@ -1934,14 +1934,14 @@ func (m *valueMerger) processColumn(ctx *sql.Context, i int, left, right, base v
 		if err != nil {
 			return nil, true, nil
 		}
-		rightModified = !isEqual(i, rightCol, baseCol, resultType)
+		rightModified = !isEqual(m.resultVD.Comparator(), i, rightCol, baseCol, resultType)
 	}
 
 	leftCol, err = convert(ctx, m.leftVD, m.resultVD, m.resultSchema, leftColIdx, i, left, leftCol, m.ns)
 	if err != nil {
 		return nil, true, nil
 	}
-	if isEqual(i, leftCol, rightCol, resultType) {
+	if isEqual(m.resultVD.Comparator(), i, leftCol, rightCol, resultType) {
 		// Columns are equal, returning either would be correct.
 		// However, for certain types the two columns may have different bytes.
 		// We need to ensure that merges are deterministic regardless of the merge direction.
@@ -1952,7 +1952,7 @@ func (m *valueMerger) processColumn(ctx *sql.Context, i int, left, right, base v
 		return rightCol, false, nil
 	}
 
-	leftModified = !isEqual(i, leftCol, baseCol, resultType)
+	leftModified = !isEqual(m.resultVD.Comparator(), i, leftCol, baseCol, resultType)
 
 	switch {
 	case leftModified && rightModified:
@@ -2135,10 +2135,8 @@ func mergeJSON(ctx context.Context, ns tree.NodeStore, base, left, right sql.JSO
 	}
 }
 
-func isEqual(i int, left []byte, right []byte, resultType val.Type) bool {
-	// We use a default comparator instead of the comparator in the schema.
-	// This is necessary to force a binary collation for string comparisons.
-	return val.DefaultTupleComparator{}.CompareValues(i, left, right, resultType) == 0
+func isEqual(cmp val.TupleComparator, i int, left []byte, right []byte, resultType val.Type) bool {
+	return cmp.CompareValues(i, left, right, resultType) == 0
 }
 
 func getColumn(tuple *val.Tuple, mapping *val.OrdinalMapping, idx int) (col []byte, colIndex int, exists bool) {

go/libraries/doltcore/merge/schema_merge_test.go

@@ -656,7 +656,7 @@ var collationTests = []schemaMergeTest{
 				ancestor: singleRow(1, 1, 1, "foo", decimal.New(100, 0)),
 				left:     singleRow(1, 1, 2, "FOO", decimal.New(100, 0)),
 				right:    singleRow(1, 2, 1, "foo", decimal.New(100, 0)),
-				merged:   singleRow(1, 2, 2, "FOO", decimal.New(100, 0)),
+				merged:   singleRow(1, 2, 2, "foo", decimal.New(100, 0)),
 			},
 			{
 				name:         "conflict removal and replace varchar with equal replacement",

go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go

@@ -15,7 +15,6 @@
 package dtables
 
 import (
-	"context"
 	"encoding/base64"
 	"fmt"
 
@@ -64,7 +63,7 @@ func newProllyConflictsTable(
 	}
 
 	return ProllyConflictsTable{
-		tblName:         tblName.Name,
+		tblName:         tblName,
 		sqlSch:          sqlSch,
 		baseSch:         baseSch,
 		ourSch:          ourSch,
@@ -81,7 +80,7 @@ func newProllyConflictsTable(
 // ProllyConflictsTable is a sql.Table implementation that uses the merge
 // artifacts table to persist and read conflicts.
 type ProllyConflictsTable struct {
-	tblName                   string
+	tblName                   doltdb.TableName
 	sqlSch                    sql.PrimaryKeySchema
 	baseSch, ourSch, theirSch schema.Schema
 	root                      doltdb.RootValue
@@ -96,11 +95,11 @@ var _ sql.UpdatableTable = ProllyConflictsTable{}
 var _ sql.DeletableTable = ProllyConflictsTable{}
 
 func (ct ProllyConflictsTable) Name() string {
-	return doltdb.DoltConfTablePrefix + ct.tblName
+	return doltdb.DoltConfTablePrefix + ct.tblName.Name
 }
 
 func (ct ProllyConflictsTable) String() string {
-	return doltdb.DoltConfTablePrefix + ct.tblName
+	return doltdb.DoltConfTablePrefix + ct.tblName.Name
 }
 
 func (ct ProllyConflictsTable) Schema() sql.Schema {
@@ -130,7 +129,7 @@ func (ct ProllyConflictsTable) Deleter(ctx *sql.Context) sql.RowDeleter {
 
 type prollyConflictRowIter struct {
 	itr     prolly.ConflictArtifactIter
-	tblName string
+	tblName doltdb.TableName
 	vrw     types.ValueReadWriter
 	ns      tree.NodeStore
 	ourRows prolly.Map
@@ -404,13 +403,13 @@ func (itr *prollyConflictRowIter) nextConflictVals(ctx *sql.Context) (c conf, er
 
 // loadTableMaps loads the maps specified in the metadata if they are different from
 // the currently loaded maps. |baseHash| and |theirHash| are table hashes.
-func (itr *prollyConflictRowIter) loadTableMaps(ctx context.Context, baseHash, theirHash hash.Hash) error {
+func (itr *prollyConflictRowIter) loadTableMaps(ctx *sql.Context, baseHash, theirHash hash.Hash) error {
 	if itr.baseHash.Compare(baseHash) != 0 {
 		rv, err := doltdb.LoadRootValueFromRootIshAddr(ctx, itr.vrw, itr.ns, baseHash)
 		if err != nil {
 			return err
 		}
-		baseTbl, ok, err := rv.GetTable(ctx, doltdb.TableName{Name: itr.tblName})
+		baseTbl, ok, err := rv.GetTable(ctx, itr.tblName)
 		if err != nil {
 			return err
 		}
@@ -435,7 +434,8 @@ func (itr *prollyConflictRowIter) loadTableMaps(ctx context.Context, baseHash, t
 		if err != nil {
 			return err
 		}
-		theirTbl, ok, err := rv.GetTable(ctx, doltdb.TableName{Name: itr.tblName})
+
+		theirTbl, ok, err := rv.GetTable(ctx, itr.tblName)
 		if err != nil {
 			return err
 		}
@@ -660,7 +660,7 @@ func (cd *prollyConflictDeleter) Close(ctx *sql.Context) error {
 		return err
 	}
 
-	updatedRoot, err := cd.ct.root.PutTable(ctx, doltdb.TableName{Name: cd.ct.tblName}, updatedTbl)
+	updatedRoot, err := cd.ct.root.PutTable(ctx, cd.ct.tblName, updatedTbl)
 	if err != nil {
 		return err
 	}

go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go

@@ -1292,7 +1292,7 @@ func TestDoltConflictsTableNameTable(t *testing.T) {
 // tests new format behavior for keyless merges that create CVs and conflicts
 func TestKeylessDoltMergeCVsAndConflicts(t *testing.T) {
 	h := newDoltEnginetestHarness(t)
-	RunKelyessDoltMergeCVsAndConflictsTests(t, h)
+	RunKeylessDoltMergeCVsAndConflictsTests(t, h)
 }
 
 // eventually this will be part of TestDoltMerge

go/libraries/doltcore/sqle/enginetest/dolt_engine_tests.go

@@ -958,7 +958,7 @@ func RunDoltConflictsTableNameTableTests(t *testing.T, h DoltEnginetestHarness)
 	}
 }
 
-func RunKelyessDoltMergeCVsAndConflictsTests(t *testing.T, h DoltEnginetestHarness) {
+func RunKeylessDoltMergeCVsAndConflictsTests(t *testing.T, h DoltEnginetestHarness) {
 	if !types.IsFormat_DOLT(types.Format_Default) {
 		t.Skip()
 	}

go/store/prolly/tree/node.go

@@ -223,7 +223,7 @@ func OutputProllyNode(ctx context.Context, w io.Writer, node Node, ns NodeStore,
 				w.Write([]byte(", "))
 			}
 
-			isAddr := val.IsAddrEncoding(kd.Types[j].Enc)
+			isAddr := j < len(kd.Types) && val.IsAddrEncoding(kd.Types[j].Enc)
 			if isAddr {
 				w.Write([]byte("#"))
 			}