diff --git a/go/libraries/doltcore/merge/merge_prolly_rows.go b/go/libraries/doltcore/merge/merge_prolly_rows.go index dc2427b2504..3b44d98f74c 100644 --- a/go/libraries/doltcore/merge/merge_prolly_rows.go +++ b/go/libraries/doltcore/merge/merge_prolly_rows.go @@ -51,7 +51,7 @@ var ErrUnableToMergeColumnDefaultValue = errorkinds.NewKind("unable to automatic // table's primary index will also be rewritten. This function merges the table's artifacts (e.g. recorded // conflicts), migrates any existing table data to the specified |mergedSch|, and merges table data from both // sides of the merge together. -func mergeProllyTable(ctx context.Context, tm *TableMerger, mergedSch schema.Schema, diffInfo tree.ThreeWayDiffInfo) (*doltdb.Table, *MergeStats, error) { +func mergeProllyTable(ctx context.Context, tm *TableMerger, mergedSch schema.Schema, mergeInfo MergeInfo, diffInfo tree.ThreeWayDiffInfo) (*doltdb.Table, *MergeStats, error) { mergeTbl, err := mergeTableArtifacts(ctx, tm, tm.leftTbl) if err != nil { return nil, nil, err @@ -85,7 +85,7 @@ func mergeProllyTable(ctx context.Context, tm *TableMerger, mergedSch schema.Sch } var stats *MergeStats - mergeTbl, stats, err = mergeProllyTableData(sqlCtx, tm, mergedSch, mergeTbl, valueMerger, mergeInfo) + mergeTbl, stats, err = mergeProllyTableData(sqlCtx, tm, mergedSch, mergeTbl, valueMerger, mergeInfo, diffInfo) if err != nil { return nil, nil, err } @@ -111,8 +111,8 @@ func mergeProllyTable(ctx context.Context, tm *TableMerger, mergedSch schema.Sch // as well as any secondary indexes, and also checking for unique constraints incrementally. When // conflicts are detected, this function attempts to resolve them automatically if possible, and // if not, they are recorded as conflicts in the table's artifacts. -func mergeProllyTableData(ctx *sql.Context, tm *TableMerger, finalSch schema.Schema, mergeTbl *doltdb.Table, valueMerger *valueMerger, mergeInfo MergeInfo) (*doltdb.Table, *MergeStats, error) { - iter, err := threeWayDiffer(ctx, tm, valueMerger) +func mergeProllyTableData(ctx *sql.Context, tm *TableMerger, finalSch schema.Schema, mergeTbl *doltdb.Table, valueMerger *valueMerger, mergeInfo MergeInfo, diffInfo tree.ThreeWayDiffInfo) (*doltdb.Table, *MergeStats, error) { + iter, err := threeWayDiffer(ctx, tm, valueMerger, diffInfo) if err != nil { return nil, nil, err } diff --git a/go/libraries/doltcore/merge/merge_rows.go b/go/libraries/doltcore/merge/merge_rows.go index 084417d17e7..f4e874c4572 100644 --- a/go/libraries/doltcore/merge/merge_rows.go +++ b/go/libraries/doltcore/merge/merge_rows.go @@ -126,7 +126,7 @@ func (rm *RootMerger) MergeTable(ctx *sql.Context, tblName string, opts editor.O } // Calculate a merge of the schemas, but don't apply it yet - mergeSch, schConflicts, mergeInfo, err := SchemaMerge(ctx, tm.vrw.Format(), tm.leftSch, tm.rightSch, tm.ancSch, tblName) + mergeSch, schConflicts, mergeInfo, diffInfo, err := SchemaMerge(ctx, tm.vrw.Format(), tm.leftSch, tm.rightSch, tm.ancSch, tblName) if err != nil { return nil, nil, err } @@ -148,7 +148,7 @@ func (rm *RootMerger) MergeTable(ctx *sql.Context, tblName string, opts editor.O var tbl *doltdb.Table if types.IsFormat_DOLT(tm.vrw.Format()) { - tbl, stats, err = mergeProllyTable(ctx, tm, mergeSch, mergeInfo) + tbl, stats, err = mergeProllyTable(ctx, tm, mergeSch, mergeInfo, diffInfo) } else { tbl, stats, err = mergeNomsTable(ctx, tm, mergeSch, rm.vrw, opts) } diff --git a/go/libraries/doltcore/merge/merge_schema.go b/go/libraries/doltcore/merge/merge_schema.go index a4c1c9ffa0c..b1e81c8261c 100644 --- a/go/libraries/doltcore/merge/merge_schema.go +++ b/go/libraries/doltcore/merge/merge_schema.go @@ -156,7 +156,7 @@ var ErrMergeWithDifferentPks = errors.New("error: cannot merge two tables with d // 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 // unexpected error encountered while merging the schemas. -func SchemaMerge(ctx context.Context, format *storetypes.NomsBinFormat, ourSch, theirSch, ancSch schema.Schema, tblName string) (sch schema.Schema, sc SchemaConflict, mergeInfo MergeInfo, err error) { +func SchemaMerge(ctx context.Context, format *storetypes.NomsBinFormat, ourSch, theirSch, ancSch schema.Schema, tblName string) (sch schema.Schema, sc SchemaConflict, mergeInfo MergeInfo, diffInfo tree.ThreeWayDiffInfo, err error) { // (sch - ancSch) ∪ (mergeSch - ancSch) ∪ (sch ∩ mergeSch) sc = SchemaConflict{ TableName: tblName, @@ -165,38 +165,38 @@ 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, ErrMergeWithDifferentPks + return nil, SchemaConflict{}, mergeInfo, diffInfo, ErrMergeWithDifferentPks } var mergedCC *schema.ColCollection - mergedCC, sc.ColConflicts, mergeInfo, err = mergeColumns(tblName, format, ourSch.GetAllCols(), theirSch.GetAllCols(), ancSch.GetAllCols()) + mergedCC, sc.ColConflicts, mergeInfo, diffInfo, err = mergeColumns(tblName, format, ourSch.GetAllCols(), theirSch.GetAllCols(), ancSch.GetAllCols()) if err != nil { - return nil, SchemaConflict{}, mergeInfo, err + return nil, SchemaConflict{}, mergeInfo, diffInfo, err } if len(sc.ColConflicts) > 0 { - return nil, sc, mergeInfo, nil + return nil, sc, mergeInfo, diffInfo, nil } var mergedIdxs schema.IndexCollection mergedIdxs, sc.IdxConflicts = mergeIndexes(mergedCC, ourSch, theirSch, ancSch) if len(sc.IdxConflicts) > 0 { - return nil, sc, mergeInfo, nil + return nil, sc, mergeInfo, diffInfo, nil } sch, err = schema.SchemaFromCols(mergedCC) if err != nil { - return nil, sc, mergeInfo, err + return nil, sc, mergeInfo, diffInfo, err } sch, err = mergeTableCollation(ctx, tblName, ancSch, ourSch, theirSch, sch) if err != nil { - return nil, sc, mergeInfo, err + return nil, sc, mergeInfo, diffInfo, err } // TODO: Merge conflict should have blocked any primary key ordinal changes err = sch.SetPkOrdinals(ourSch.GetPkOrdinals()) if err != nil { - return nil, sc, mergeInfo, err + return nil, sc, mergeInfo, diffInfo, err } _ = mergedIdxs.Iter(func(index schema.Index) (stop bool, err error) { @@ -208,17 +208,17 @@ func SchemaMerge(ctx context.Context, format *storetypes.NomsBinFormat, ourSch, var mergedChks []schema.Check mergedChks, sc.ChkConflicts, err = mergeChecks(ctx, ourSch.Checks(), theirSch.Checks(), ancSch.Checks()) if err != nil { - return nil, SchemaConflict{}, mergeInfo, err + return nil, SchemaConflict{}, mergeInfo, diffInfo, err } if len(sc.ChkConflicts) > 0 { - return nil, sc, mergeInfo, nil + return nil, sc, mergeInfo, diffInfo, nil } // Look for invalid CHECKs for _, chk := range mergedChks { // CONFLICT: a CHECK now references a column that no longer exists in schema if ok, err := isCheckReferenced(sch, chk); err != nil { - return nil, sc, mergeInfo, err + return nil, sc, mergeInfo, diffInfo, err } else if !ok { // Append to conflicts sc.ChkConflicts = append(sc.ChkConflicts, ChkConflict{ @@ -233,7 +233,7 @@ func SchemaMerge(ctx context.Context, format *storetypes.NomsBinFormat, ourSch, sch.Checks().AddCheck(chk.Name(), chk.Expression(), chk.Enforced()) } - return sch, sc, mergeInfo, nil + return sch, sc, mergeInfo, diffInfo, nil } // ForeignKeysMerge performs a three-way merge of (ourRoot, theirRoot, ancRoot) and using mergeRoot to validate FKs. @@ -376,21 +376,22 @@ type MergeInfo struct { // compatible with the current stored format. The merged columns, any column conflicts, and a boolean value stating if // a full table rewrite is needed to align the existing table rows with the new, merged schema. If any unexpected error // occurs, then that error is returned and the other response fields should be ignored. -func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, theirCC, ancCC *schema.ColCollection) (*schema.ColCollection, []ColConflict, MergeInfo, error) { +func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, theirCC, ancCC *schema.ColCollection) (*schema.ColCollection, []ColConflict, MergeInfo, tree.ThreeWayDiffInfo, error) { mergeInfo := MergeInfo{} + diffInfo := tree.ThreeWayDiffInfo{} columnMappings, err := mapColumns(ourCC, theirCC, ancCC) if err != nil { - return nil, nil, mergeInfo, err + return nil, nil, mergeInfo, diffInfo, err } conflicts, err := checkSchemaConflicts(columnMappings) if err != nil { - return nil, nil, mergeInfo, err + return nil, nil, mergeInfo, diffInfo, err } err = checkUnmergeableNewColumns(tblName, columnMappings) if err != nil { - return nil, nil, mergeInfo, err + return nil, nil, mergeInfo, diffInfo, err } compatChecker := newTypeCompatabilityCheckerForStorageFormat(format) @@ -409,47 +410,47 @@ 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) mergeInfo.LeftNeedsRewrite = true - theirSchemaChanged = true - leftAndRightSchemasDiffer = true + diffInfo.RightSchemaChange = true + diffInfo.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) mergeInfo.RightNeedsRewrite = true - ourSchemaChanged = true - leftAndRightSchemasDiffer = true + diffInfo.LeftSchemaChange = true + diffInfo.LeftAndRightSchemasDiffer = true mergedColumns = append(mergedColumns, *ours) case anc != nil && ours == nil && theirs != nil: // column was deleted on our side mergeInfo.RightNeedsRewrite = true - ourSchemaChanged = true - leftAndRightSchemasDiffer = true + diffInfo.LeftSchemaChange = true + diffInfo.LeftAndRightSchemasDiffer = true case anc != nil && ours != nil && theirs == nil: // column was deleted on their side mergeInfo.LeftNeedsRewrite = true - theirSchemaChanged = true - leftAndRightSchemasDiffer = true + diffInfo.RightSchemaChange = true + diffInfo.LeftAndRightSchemasDiffer = true case ours == nil && theirs == nil: // if the column is deleted on both sides... just let it fall out - ourSchemaChanged = true - theirSchemaChanged = true + diffInfo.LeftSchemaChange = true + diffInfo.RightSchemaChange = true case ours != nil && theirs != nil: // otherwise, we have two valid columns and we need to figure out which one to use if anc != nil { oursChanged := !anc.Equals(*ours) - ourSchemaChanged = ourSchemaChanged || oursChanged + diffInfo.LeftSchemaChange = diffInfo.LeftSchemaChange || oursChanged theirsChanged := !anc.Equals(*theirs) - theirSchemaChanged = theirSchemaChanged || theirsChanged + diffInfo.RightSchemaChange = diffInfo.RightSchemaChange || theirsChanged if oursChanged && theirsChanged { // If both columns changed in the same way, the modifications converge, so accept the column. // 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 + diffInfo.LeftAndRightSchemasDiffer = true } } else if theirsChanged { - leftAndRightSchemasDiffer = true + diffInfo.LeftAndRightSchemasDiffer = true // In this case, only theirsChanged, so we need to check if moving from ours->theirs // is valid, otherwise it's a conflict mergeInfo.LeftNeedsRewrite = true @@ -467,7 +468,7 @@ func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, their }) } } else if oursChanged { - leftAndRightSchemasDiffer = true + diffInfo.LeftAndRightSchemasDiffer = true // In this case, only oursChanged, so we need to check if moving from theirs->ours // is valid, otherwise it's a conflict mergeInfo.RightNeedsRewrite = true @@ -490,14 +491,14 @@ func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, their } } else { // The column was added on both branches. - ourSchemaChanged = true - theirSchemaChanged = true + diffInfo.LeftSchemaChange = true + diffInfo.RightSchemaChange = true // If both columns changed in the same way, the modifications converge, so accept the column. // 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 + diffInfo.LeftAndRightSchemasDiffer = true } } } @@ -506,10 +507,10 @@ func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, their // Check that there are no duplicate column names or tags in the merged column set conflicts = append(conflicts, checkForColumnConflicts(mergedColumns)...) if conflicts != nil { - return nil, conflicts, mergeInfo, nil + return nil, conflicts, mergeInfo, diffInfo, nil } - return schema.NewColCollection(mergedColumns...), nil, mergeInfo, nil + return schema.NewColCollection(mergedColumns...), nil, mergeInfo, diffInfo, nil } // checkForColumnConflicts iterates over |mergedColumns|, checks for duplicate column names or column tags, and returns diff --git a/go/libraries/doltcore/merge/schema_integration_test.go b/go/libraries/doltcore/merge/schema_integration_test.go index d4af9de1548..af734f34a79 100644 --- a/go/libraries/doltcore/merge/schema_integration_test.go +++ b/go/libraries/doltcore/merge/schema_integration_test.go @@ -635,7 +635,7 @@ func testMergeSchemasWithConflicts(t *testing.T, test mergeSchemaConflictTest) { otherSch := getSchema(t, dEnv) - _, actConflicts, mergeInfo, err := merge.SchemaMerge(context.Background(), types.Format_Default, mainSch, otherSch, ancSch, "test") + _, 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))