Skip to content

Commit

Permalink
Use comparators from tuple descriptor for merges, not clear if these …
Browse files Browse the repository at this point in the history
…are all correct
  • Loading branch information
zachmu committed Sep 26, 2024
1 parent bb6cf2a commit 934c0d0
Showing 1 changed file with 12 additions and 6 deletions.
18 changes: 12 additions & 6 deletions go/libraries/doltcore/merge/merge_prolly_rows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 isEqualComparator(m.baseVD.Comparator(), i, baseCol, rightCol, m.rightVD.Types[rightColIdx]) {
// right column did not change, so there is no conflict.
return false, nil
}
Expand All @@ -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 isEqualComparator(m.leftVD.Comparator(), i, baseCol, leftCol, m.leftVD.Types[leftColIdx]) {
// left column did not change, so there is no conflict.
return false, nil
}
Expand Down Expand Up @@ -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 isEqualComparator(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.
Expand Down Expand Up @@ -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 = !isEqualComparator(m.rightVD.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 isEqualComparator(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.
Expand All @@ -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 = !isEqualComparator(m.baseVD.Comparator(), i, leftCol, baseCol, resultType)

switch {
case leftModified && rightModified:
Expand Down Expand Up @@ -2141,6 +2141,12 @@ func isEqual(i int, left []byte, right []byte, resultType val.Type) bool {
return val.DefaultTupleComparator{}.CompareValues(i, left, right, resultType) == 0
}

func isEqualComparator(cmp val.TupleComparator, 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 cmp.CompareValues(i, left, right, resultType) == 0
}

func getColumn(tuple *val.Tuple, mapping *val.OrdinalMapping, idx int) (col []byte, colIndex int, exists bool) {
colIdx := (*mapping)[idx]
if colIdx == -1 {
Expand Down

0 comments on commit 934c0d0

Please sign in to comment.