Track whether the left and right schemas are equal. If they're not, then the three way differ should never produce a convergent edit.

This commit is contained in:
Nick Tobey
2023-11-22 02:10:50 -08:00
parent 23c09343c6
commit 4c746771bb
4 changed files with 34 additions and 28 deletions

View File

@@ -340,6 +340,7 @@ func threeWayDiffer(ctx context.Context, tm *TableMerger, valueMerger *valueMerg
ancRows.Tuples(),
valueMerger.tryMerge,
valueMerger.keyless,
mergeInfo.LeftAndRightSchemasDiffer,
leftRows.Tuples().Order,
)
}

View File

@@ -153,9 +153,10 @@ func (c ChkConflict) String() string {
var ErrMergeWithDifferentPks = errors.New("error: cannot merge two tables with different primary keys")
type MergeInfo struct {
TableRewrite bool
LeftSchemaChange bool
RightSchemaChange bool
TableRewrite bool
LeftSchemaChange bool
RightSchemaChange bool
LeftAndRightSchemasDiffer bool
}
// SchemaMerge performs a three-way merge of |ourSch|, |theirSch|, and |ancSch|, and returns: the merged schema,
@@ -396,6 +397,7 @@ func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, their
tableRewrite := false
ourSchemaChanged := false
theirSchemaChanged := false
leftAndRightSchemasDiffer := false
// After we've checked for schema conflicts, merge the columns together
// TODO: We don't currently preserve all column position changes; the returned merged columns are always based on
@@ -411,18 +413,22 @@ func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, their
// if an ancestor does not exist, and the column exists only on one side, use that side
// (if an ancestor DOES exist, this means the column was deleted, so it's a no-op)
theirSchemaChanged = true
leftAndRightSchemasDiffer = true
mergedColumns = append(mergedColumns, *theirs)
case anc == nil && ours != nil && theirs == nil:
// if an ancestor does not exist, and the column exists only on one side, use that side
// (if an ancestor DOES exist, this means the column was deleted, so it's a no-op)
ourSchemaChanged = true
leftAndRightSchemasDiffer = true
mergedColumns = append(mergedColumns, *ours)
case anc != nil && ours == nil && theirs != nil:
// The column was dropped by ours
ourSchemaChanged = true
leftAndRightSchemasDiffer = true
case anc != nil && ours != nil && theirs == nil:
// The column was dropped by theirs
theirSchemaChanged = true
leftAndRightSchemasDiffer = true
case ours == nil && theirs == nil:
// if the column is deleted on both sides... just let it fall out
ourSchemaChanged = true
@@ -439,8 +445,11 @@ func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, their
// If not, don't report a conflict, since this case is already handled in checkForColumnConflicts.
if ours.Equals(*theirs) {
mergedColumns = append(mergedColumns, *theirs)
} else {
leftAndRightSchemasDiffer = true
}
} else if theirsChanged {
leftAndRightSchemasDiffer = true
// In this case, only theirsChanged, so we need to check if moving from ours->theirs
// is valid, otherwise it's a conflict
compatible, rewrite := compatChecker.IsTypeChangeCompatible(ours.TypeInfo, theirs.TypeInfo)
@@ -457,6 +466,7 @@ func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, their
})
}
} else if oursChanged {
leftAndRightSchemasDiffer = true
// In this case, only oursChanged, so we need to check if moving from theirs->ours
// is valid, otherwise it's a conflict
compatible, rewrite := compatChecker.IsTypeChangeCompatible(theirs.TypeInfo, ours.TypeInfo)
@@ -484,6 +494,8 @@ func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, their
// If not, don't report a conflict, since this case is already handled in checkForColumnConflicts.
if ours.Equals(*theirs) {
mergedColumns = append(mergedColumns, *ours)
} else {
leftAndRightSchemasDiffer = true
}
}
}
@@ -496,9 +508,10 @@ func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, their
}
mergeInfo := MergeInfo{
TableRewrite: tableRewrite,
LeftSchemaChange: ourSchemaChanged,
RightSchemaChange: theirSchemaChanged,
TableRewrite: tableRewrite,
LeftSchemaChange: ourSchemaChanged,
RightSchemaChange: theirSchemaChanged,
LeftAndRightSchemasDiffer: leftAndRightSchemasDiffer,
}
return schema.NewColCollection(mergedColumns...), nil, mergeInfo, nil
}

View File

@@ -236,9 +236,6 @@ var columnAddDropTests = []schemaMergeTest{
merged: singleRow(1, 2),
},
{
// Skipped because the differ currently doesn't see this as a data conflict because
// both left and right tuple representations are the same.
// (https://github.com/dolthub/dolt/issues/6748)
name: "one side sets to NULL, other drops non-NULL",
ancestor: singleRow(1, 2, 3),
left: singleRow(1, 2),
@@ -247,8 +244,6 @@ var columnAddDropTests = []schemaMergeTest{
skip: true,
},
{
// Skipped because the differ currently doesn't try to merge the dropped column.
// (https://github.com/dolthub/dolt/issues/6747)
name: "one side sets to NULL, other drops non-NULL, plus data change",
ancestor: singleRow(1, 2, 3),
left: singleRow(1, 2),
@@ -256,9 +251,6 @@ var columnAddDropTests = []schemaMergeTest{
dataConflict: true,
},
{
// Skipped because the differ doesn't see left as modified because
// it has the same tuple representation as ancestor.
// (https://github.com/dolthub/dolt/issues/6746)
name: "one side sets to non-NULL, other drops NULL",
ancestor: singleRow(1, 2, nil),
left: singleRow(1, 2),
@@ -373,9 +365,6 @@ var columnAddDropTests = []schemaMergeTest{
merged: singleRow(1, 2, nil),
},
{
// Skipped because the differ doesn't see left as modified because
// it has the same tuple representation as ancestor.
// (https://github.com/dolthub/dolt/issues/6746)
name: "convergent adds with differing nullness",
ancestor: singleRow(1, 2),
left: singleRow(1, 2, nil),

View File

@@ -27,13 +27,14 @@ import (
// ThreeWayDiffer is an iterator that gives an increased level of granularity
// of diffs between three root values. See diffOp for the classes of diffs.
type ThreeWayDiffer[K ~[]byte, O Ordering[K]] struct {
lIter, rIter Differ[K, O]
resolveCb resolveCb
lDiff Diff
rDiff Diff
lDone bool
rDone bool
keyless bool
lIter, rIter Differ[K, O]
resolveCb resolveCb
lDiff Diff
rDiff Diff
lDone bool
rDone bool
keyless bool
leftAndRightSchemasDiffer bool
}
//var _ DiffIter = (*threeWayDiffer[Item, val.TupleDesc])(nil)
@@ -50,6 +51,7 @@ func NewThreeWayDiffer[K, V ~[]byte, O Ordering[K]](
base StaticMap[K, V, O],
resolveCb resolveCb,
keyless bool,
leftAndRightSchemasDiffer bool,
order O,
) (*ThreeWayDiffer[K, O], error) {
// probably compute each of these separately
@@ -64,10 +66,11 @@ func NewThreeWayDiffer[K, V ~[]byte, O Ordering[K]](
}
return &ThreeWayDiffer[K, O]{
lIter: ld,
rIter: rd,
resolveCb: resolveCb,
keyless: keyless,
lIter: ld,
rIter: rd,
resolveCb: resolveCb,
keyless: keyless,
leftAndRightSchemasDiffer: leftAndRightSchemasDiffer,
}, nil
}