diff --git a/go/libraries/doltcore/merge/merge_prolly_rows.go b/go/libraries/doltcore/merge/merge_prolly_rows.go index c1438d8bdb..2935e19a8e 100644 --- a/go/libraries/doltcore/merge/merge_prolly_rows.go +++ b/go/libraries/doltcore/merge/merge_prolly_rows.go @@ -340,6 +340,7 @@ func threeWayDiffer(ctx context.Context, tm *TableMerger, valueMerger *valueMerg ancRows.Tuples(), valueMerger.tryMerge, valueMerger.keyless, + mergeInfo.LeftAndRightSchemasDiffer, leftRows.Tuples().Order, ) } diff --git a/go/libraries/doltcore/merge/merge_schema.go b/go/libraries/doltcore/merge/merge_schema.go index dca434dc29..9793cddca3 100644 --- a/go/libraries/doltcore/merge/merge_schema.go +++ b/go/libraries/doltcore/merge/merge_schema.go @@ -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 } diff --git a/go/libraries/doltcore/merge/schema_merge_test.go b/go/libraries/doltcore/merge/schema_merge_test.go index 49ee530eac..2fddc191e4 100644 --- a/go/libraries/doltcore/merge/schema_merge_test.go +++ b/go/libraries/doltcore/merge/schema_merge_test.go @@ -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), diff --git a/go/store/prolly/tree/three_way_differ.go b/go/store/prolly/tree/three_way_differ.go index 3cf32a3403..1740255c48 100644 --- a/go/store/prolly/tree/three_way_differ.go +++ b/go/store/prolly/tree/three_way_differ.go @@ -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 }