diff --git a/go/cmd/dolt/dolt.go b/go/cmd/dolt/dolt.go index 5c09e820eb..444a07ad50 100644 --- a/go/cmd/dolt/dolt.go +++ b/go/cmd/dolt/dolt.go @@ -64,7 +64,7 @@ import ( ) const ( - Version = "1.27.0" + Version = "1.28.1" ) var dumpDocsCommand = &commands.DumpDocsCmd{} diff --git a/go/go.mod b/go/go.mod index d9eec2e0a4..d120bc69ad 100644 --- a/go/go.mod +++ b/go/go.mod @@ -59,7 +59,7 @@ require ( github.com/cespare/xxhash v1.1.0 github.com/creasty/defaults v1.6.0 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 - github.com/dolthub/go-mysql-server v0.17.1-0.20231117210301-8d70c233f221 + github.com/dolthub/go-mysql-server v0.17.1-0.20231121231446-0a2b90169f58 github.com/dolthub/swiss v0.1.0 github.com/goccy/go-json v0.10.2 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 diff --git a/go/go.sum b/go/go.sum index 4c2624b4f8..7b204f6b6d 100644 --- a/go/go.sum +++ b/go/go.sum @@ -181,8 +181,8 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U= github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw1+y/N5SSCkma7FhAPw7KeGmD6c9PBZW9Y= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168= -github.com/dolthub/go-mysql-server v0.17.1-0.20231117210301-8d70c233f221 h1:4tXnkotftNS4nxPTCfwy5WFPPq+YFG3xBp5nfDz0Zto= -github.com/dolthub/go-mysql-server v0.17.1-0.20231117210301-8d70c233f221/go.mod h1:Z3EbOzC1yoK9MoYBxl6LDksV8GRRyjjHDZTu2lWpT/E= +github.com/dolthub/go-mysql-server v0.17.1-0.20231121231446-0a2b90169f58 h1:zw4LBAwaz0cEPdphNrsQ+m0JmFX7dOZu1iA56EEpQQs= +github.com/dolthub/go-mysql-server v0.17.1-0.20231121231446-0a2b90169f58/go.mod h1:Z3EbOzC1yoK9MoYBxl6LDksV8GRRyjjHDZTu2lWpT/E= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/dolthub/jsonpath v0.0.2-0.20230525180605-8dc13778fd72 h1:NfWmngMi1CYUWU4Ix8wM+USEhjc+mhPlT9JUR/anvbQ= diff --git a/go/libraries/doltcore/merge/merge_prolly_rows.go b/go/libraries/doltcore/merge/merge_prolly_rows.go index a128a294ad..dc2427b250 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, rebuildIndexes bool) (*doltdb.Table, *MergeStats, error) { +func mergeProllyTable(ctx context.Context, tm *TableMerger, mergedSch schema.Schema, diffInfo tree.ThreeWayDiffInfo) (*doltdb.Table, *MergeStats, error) { mergeTbl, err := mergeTableArtifacts(ctx, tm, tm.leftTbl) if err != nil { return nil, nil, err @@ -69,6 +69,14 @@ func mergeProllyTable(ctx context.Context, tm *TableMerger, mergedSch schema.Sch leftRows := durable.ProllyMapFromIndex(lr) valueMerger := newValueMerger(mergedSch, tm.leftSch, tm.rightSch, tm.ancSch, leftRows.Pool(), tm.ns) + if !valueMerger.leftMapping.IsIdentityMapping() { + mergeInfo.LeftNeedsRewrite = true + } + + if !valueMerger.rightMapping.IsIdentityMapping() { + mergeInfo.RightNeedsRewrite = true + } + // We need a sql.Context to apply column default values in merges; if we don't have one already, // create one, since this code also gets called from the CLI merge code path. sqlCtx, ok := ctx.(*sql.Context) @@ -77,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, diffInfo, rebuildIndexes) + mergeTbl, stats, err = mergeProllyTableData(sqlCtx, tm, mergedSch, mergeTbl, valueMerger, mergeInfo) if err != nil { return nil, nil, err } @@ -102,19 +110,9 @@ func mergeProllyTable(ctx context.Context, tm *TableMerger, mergedSch schema.Sch // to the right-side, we apply it to the left-side by merging it into the left-side's primary index // 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. If |rebuildIndexes| is set to -// true, then the table indexes will be rebuilt instead of being incrementally merged together. This -// is less efficient, but safer, especially when type changes have been applied to a table's schema. -func mergeProllyTableData(ctx *sql.Context, tm *TableMerger, finalSch schema.Schema, mergeTbl *doltdb.Table, valueMerger *valueMerger, diffInfo tree.ThreeWayDiffInfo, rebuildIndexes bool) (*doltdb.Table, *MergeStats, error) { - - schemasDifferentSize := len(tm.leftSch.GetAllCols().GetColumns()) != len(finalSch.GetAllCols().GetColumns()) - - // If columns were dropped, added, or reordered by the merge, we must rebuild the primary index, - // but the secondary indexes are unaffected. - rebuildPrimaryIndex := rebuildIndexes || schemasDifferentSize || !valueMerger.leftMapping.IsIdentityMapping() - rebuildSecondaryIndexes := rebuildIndexes - - iter, err := threeWayDiffer(ctx, tm, valueMerger, diffInfo) +// 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) if err != nil { return nil, nil, err } @@ -133,11 +131,11 @@ func mergeProllyTableData(ctx *sql.Context, tm *TableMerger, finalSch schema.Sch keyless := schema.IsKeyless(tm.leftSch) - pri, err := newPrimaryMerger(leftEditor, tm, valueMerger, finalSch) + pri, err := newPrimaryMerger(leftEditor, tm, valueMerger, finalSch, mergeInfo) if err != nil { return nil, nil, err } - sec, err := newSecondaryMerger(ctx, tm, valueMerger, finalSch) + sec, err := newSecondaryMerger(ctx, tm, valueMerger, finalSch, mergeInfo) if err != nil { return nil, nil, err } @@ -198,11 +196,9 @@ func mergeProllyTableData(ctx *sql.Context, tm *TableMerger, finalSch schema.Sch // In the event that the right side introduced a schema change, account for it here. // We still have to migrate when the diff is `tree.DiffOpLeftModify` because of the corner case where // the right side contains a schema change but the changed column is null, so row bytes don't change. - if rebuildPrimaryIndex { - err = pri.merge(ctx, diff, tm.leftSch) - if err != nil { - return nil, nil, err - } + err = pri.merge(ctx, diff, tm.leftSch) + if err != nil { + return nil, nil, err } case tree.DiffOpDivergentModifyConflict, tree.DiffOpDivergentDeleteConflict: @@ -290,7 +286,7 @@ func mergeProllyTableData(ctx *sql.Context, tm *TableMerger, finalSch schema.Sch return nil, nil, err } - finalIdxs, err := mergeProllySecondaryIndexes(ctx, tm, leftIdxs, rightIdxs, finalSch, finalRows, conflicts.ae, rebuildSecondaryIndexes) + finalIdxs, err := mergeProllySecondaryIndexes(ctx, tm, leftIdxs, rightIdxs, finalSch, finalRows, conflicts.ae, mergeInfo.InvalidateSecondaryIndexes) if err != nil { return nil, nil, err } @@ -1017,14 +1013,16 @@ type primaryMerger struct { valueMerger *valueMerger tableMerger *TableMerger finalSch schema.Schema + mergeInfo MergeInfo } -func newPrimaryMerger(leftEditor *prolly.MutableMap, tableMerger *TableMerger, valueMerger *valueMerger, finalSch schema.Schema) (*primaryMerger, error) { +func newPrimaryMerger(leftEditor *prolly.MutableMap, tableMerger *TableMerger, valueMerger *valueMerger, finalSch schema.Schema, mergeInfo MergeInfo) (*primaryMerger, error) { return &primaryMerger{ mut: leftEditor, valueMerger: valueMerger, tableMerger: tableMerger, finalSch: finalSch, + mergeInfo: mergeInfo, }, nil } @@ -1045,28 +1043,31 @@ func (m *primaryMerger) merge(ctx *sql.Context, diff tree.ThreeWayDiff, sourceSc return fmt.Errorf("cannot merge keyless tables with reordered columns") } } else { - defaults, err := resolveDefaults(ctx, m.tableMerger.name, m.finalSch, m.tableMerger.rightSch) - if err != nil { - return err - } + // Remapping when there's no schema change is harmless, but slow. + if m.mergeInfo.RightNeedsRewrite { + defaults, err := resolveDefaults(ctx, m.tableMerger.name, m.finalSch, m.tableMerger.rightSch) + if err != nil { + return err + } - tempTupleValue, err := remapTupleWithColumnDefaults( - ctx, - diff.Key, - diff.Right, - sourceSch.GetValueDescriptor(), - m.valueMerger.rightMapping, - m.tableMerger, - m.tableMerger.rightSch, - m.finalSch, - defaults, - m.valueMerger.syncPool, - true, - ) - if err != nil { - return err + tempTupleValue, err := remapTupleWithColumnDefaults( + ctx, + diff.Key, + diff.Right, + sourceSch.GetValueDescriptor(), + m.valueMerger.rightMapping, + m.tableMerger, + m.tableMerger.rightSch, + m.finalSch, + defaults, + m.valueMerger.syncPool, + true, + ) + if err != nil { + return err + } + newTupleValue = tempTupleValue } - newTupleValue = tempTupleValue } return m.mut.Put(ctx, diff.Key, newTupleValue) case tree.DiffOpRightDelete: @@ -1103,6 +1104,10 @@ func (m *primaryMerger) merge(ctx *sql.Context, diff tree.ThreeWayDiff, sourceSc return m.mut.Put(ctx, diff.Key, merged) case tree.DiffOpLeftAdd, tree.DiffOpLeftModify, tree.DiffOpDivergentModifyConflict, tree.DiffOpDivergentDeleteConflict: + // Remapping when there's no schema change is harmless, but slow. + if !m.mergeInfo.LeftNeedsRewrite { + return nil + } // If the right side has a schema change, then newly added rows from the left must be migrated to the new schema. // Rows with unresolvable conflicts must also be migrated to the new schema so that they can resolved manually. if diff.Left == nil { @@ -1221,11 +1226,12 @@ type secondaryMerger struct { valueMerger *valueMerger mergedSchema schema.Schema tableMerger *TableMerger + mergeInfo MergeInfo } const secondaryMergerPendingSize = 650_000 -func newSecondaryMerger(ctx *sql.Context, tm *TableMerger, valueMerger *valueMerger, mergedSchema schema.Schema) (*secondaryMerger, error) { +func newSecondaryMerger(ctx *sql.Context, tm *TableMerger, valueMerger *valueMerger, mergedSchema schema.Schema, mergeInfo MergeInfo) (*secondaryMerger, error) { ls, err := tm.leftTbl.GetIndexSet(ctx) if err != nil { return nil, err @@ -1249,11 +1255,15 @@ func newSecondaryMerger(ctx *sql.Context, tm *TableMerger, valueMerger *valueMer valueMerger: valueMerger, mergedSchema: mergedSchema, tableMerger: tm, + mergeInfo: mergeInfo, }, nil } func (m *secondaryMerger) merge(ctx *sql.Context, diff tree.ThreeWayDiff, leftSchema, rightSchema schema.Schema, tm *TableMerger, finalSchema schema.Schema) error { var err error + if m.mergeInfo.InvalidateSecondaryIndexes { + return nil + } for _, idx := range m.leftIdxes { switch diff.Op { case tree.DiffOpDivergentModifyResolved: @@ -1267,57 +1277,59 @@ func (m *secondaryMerger) merge(ctx *sql.Context, diff tree.ThreeWayDiff, leftSc newTupleValue := diff.Right baseTupleValue := diff.Base - if schema.IsKeyless(rightSchema) { - if m.valueMerger.rightMapping.IsIdentityMapping() == false { - return fmt.Errorf("cannot merge keyless tables with reordered columns") - } - } else { - defaults, err := resolveDefaults(ctx, m.tableMerger.name, m.mergedSchema, m.tableMerger.rightSch) - if err != nil { - return err - } - - // Convert right value to result schema - tempTupleValue, err := remapTupleWithColumnDefaults( - ctx, - diff.Key, - diff.Right, - m.valueMerger.rightSchema.GetValueDescriptor(), - m.valueMerger.rightMapping, - m.tableMerger, - m.tableMerger.rightSch, - m.mergedSchema, - defaults, - m.valueMerger.syncPool, - true, - ) - if err != nil { - return err - } - newTupleValue = tempTupleValue - if diff.Base != nil { - defaults, err := resolveDefaults(ctx, m.tableMerger.name, m.mergedSchema, m.tableMerger.ancSch) + if m.mergeInfo.RightNeedsRewrite { + if schema.IsKeyless(rightSchema) { + if m.valueMerger.rightMapping.IsIdentityMapping() == false { + return fmt.Errorf("cannot merge keyless tables with reordered columns") + } + } else { + defaults, err := resolveDefaults(ctx, m.tableMerger.name, m.mergedSchema, m.tableMerger.rightSch) if err != nil { return err } - // Convert base value to result schema - baseTupleValue, err = remapTupleWithColumnDefaults( + // Convert right value to result schema + tempTupleValue, err := remapTupleWithColumnDefaults( ctx, diff.Key, - diff.Base, - // Only the right side was modified, so the base schema must be the same as the left schema - leftSchema.GetValueDescriptor(), - m.valueMerger.baseMapping, - tm, - m.tableMerger.ancSch, - finalSchema, + diff.Right, + m.valueMerger.rightSchema.GetValueDescriptor(), + m.valueMerger.rightMapping, + m.tableMerger, + m.tableMerger.rightSch, + m.mergedSchema, defaults, m.valueMerger.syncPool, - false) + true, + ) if err != nil { return err } + newTupleValue = tempTupleValue + if diff.Base != nil { + defaults, err := resolveDefaults(ctx, m.tableMerger.name, m.mergedSchema, m.tableMerger.ancSch) + if err != nil { + return err + } + + // Convert base value to result schema + baseTupleValue, err = remapTupleWithColumnDefaults( + ctx, + diff.Key, + diff.Base, + // Only the right side was modified, so the base schema must be the same as the left schema + leftSchema.GetValueDescriptor(), + m.valueMerger.baseMapping, + tm, + m.tableMerger.ancSch, + finalSchema, + defaults, + m.valueMerger.syncPool, + false) + if err != nil { + return err + } + } } } diff --git a/go/libraries/doltcore/merge/merge_rows.go b/go/libraries/doltcore/merge/merge_rows.go index 0525619757..084417d17e 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, diffInfo, rebuildIndexes, err := SchemaMerge(ctx, tm.vrw.Format(), tm.leftSch, tm.rightSch, tm.ancSch, tblName) + mergeSch, schConflicts, mergeInfo, 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, diffInfo, rebuildIndexes) + tbl, stats, err = mergeProllyTable(ctx, tm, mergeSch, mergeInfo) } 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 8366d402a7..a4c1c9ffa0 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, diffInfo tree.ThreeWayDiffInfo, rebuildIndexes bool, err error) { +func SchemaMerge(ctx context.Context, format *storetypes.NomsBinFormat, ourSch, theirSch, ancSch schema.Schema, tblName string) (sch schema.Schema, sc SchemaConflict, mergeInfo MergeInfo, 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{}, diffInfo, rebuildIndexes, ErrMergeWithDifferentPks + return nil, SchemaConflict{}, mergeInfo, ErrMergeWithDifferentPks } var mergedCC *schema.ColCollection - mergedCC, sc.ColConflicts, diffInfo, rebuildIndexes, err = mergeColumns(tblName, format, ourSch.GetAllCols(), theirSch.GetAllCols(), ancSch.GetAllCols()) + mergedCC, sc.ColConflicts, mergeInfo, err = mergeColumns(tblName, format, ourSch.GetAllCols(), theirSch.GetAllCols(), ancSch.GetAllCols()) if err != nil { - return nil, SchemaConflict{}, diffInfo, rebuildIndexes, err + return nil, SchemaConflict{}, mergeInfo, err } if len(sc.ColConflicts) > 0 { - return nil, sc, diffInfo, rebuildIndexes, nil + return nil, sc, mergeInfo, nil } var mergedIdxs schema.IndexCollection mergedIdxs, sc.IdxConflicts = mergeIndexes(mergedCC, ourSch, theirSch, ancSch) if len(sc.IdxConflicts) > 0 { - return nil, sc, diffInfo, rebuildIndexes, nil + return nil, sc, mergeInfo, nil } sch, err = schema.SchemaFromCols(mergedCC) if err != nil { - return nil, sc, diffInfo, rebuildIndexes, err + return nil, sc, mergeInfo, err } sch, err = mergeTableCollation(ctx, tblName, ancSch, ourSch, theirSch, sch) if err != nil { - return nil, sc, diffInfo, rebuildIndexes, err + return nil, sc, mergeInfo, err } // TODO: Merge conflict should have blocked any primary key ordinal changes err = sch.SetPkOrdinals(ourSch.GetPkOrdinals()) if err != nil { - return nil, sc, diffInfo, rebuildIndexes, err + return nil, sc, mergeInfo, 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{}, diffInfo, rebuildIndexes, err + return nil, SchemaConflict{}, mergeInfo, err } if len(sc.ChkConflicts) > 0 { - return nil, sc, diffInfo, rebuildIndexes, nil + return nil, sc, mergeInfo, 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, diffInfo, rebuildIndexes, err + return nil, sc, mergeInfo, 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, diffInfo, rebuildIndexes, nil + return sch, sc, mergeInfo, nil } // ForeignKeysMerge performs a three-way merge of (ourRoot, theirRoot, ancRoot) and using mergeRoot to validate FKs. @@ -362,6 +362,12 @@ func checkUnmergeableNewColumns(tblName string, columnMappings columnMappings) e return nil } +type MergeInfo struct { + LeftNeedsRewrite bool + RightNeedsRewrite bool + InvalidateSecondaryIndexes bool +} + // mergeColumns merges the columns from |ourCC|, |theirCC| into a single column collection, using the ancestor column // definitions in |ancCC| to determine on which side a column has changed. If merging is not possible because of // conflicting changes to the columns in |ourCC| and |theirCC|, then a set of ColConflict instances are returned @@ -370,29 +376,25 @@ func checkUnmergeableNewColumns(tblName string, columnMappings columnMappings) e // 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, tree.ThreeWayDiffInfo, bool, error) { +func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, theirCC, ancCC *schema.ColCollection) (*schema.ColCollection, []ColConflict, MergeInfo, error) { + mergeInfo := MergeInfo{} columnMappings, err := mapColumns(ourCC, theirCC, ancCC) if err != nil { - return nil, nil, tree.ThreeWayDiffInfo{}, false, err + return nil, nil, mergeInfo, err } conflicts, err := checkSchemaConflicts(columnMappings) if err != nil { - return nil, nil, tree.ThreeWayDiffInfo{}, false, err + return nil, nil, mergeInfo, err } err = checkUnmergeableNewColumns(tblName, columnMappings) if err != nil { - return nil, nil, tree.ThreeWayDiffInfo{}, false, err + return nil, nil, mergeInfo, err } compatChecker := newTypeCompatabilityCheckerForStorageFormat(format) - 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 // their position in |ourCC|, with any new columns from |theirCC| added at the end of the column collection. @@ -406,21 +408,25 @@ func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, their 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.LeftNeedsRewrite = true 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) + mergeInfo.RightNeedsRewrite = true ourSchemaChanged = true leftAndRightSchemasDiffer = true mergedColumns = append(mergedColumns, *ours) case anc != nil && ours == nil && theirs != nil: - // The column was dropped by ours + // column was deleted on our side + mergeInfo.RightNeedsRewrite = true ourSchemaChanged = true leftAndRightSchemasDiffer = true case anc != nil && ours != nil && theirs == nil: - // The column was dropped by theirs + // column was deleted on their side + mergeInfo.LeftNeedsRewrite = true theirSchemaChanged = true leftAndRightSchemasDiffer = true case ours == nil && theirs == nil: @@ -446,9 +452,10 @@ func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, their 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 compatible, rewrite := compatChecker.IsTypeChangeCompatible(ours.TypeInfo, theirs.TypeInfo) if rewrite { - tableRewrite = true + mergeInfo.InvalidateSecondaryIndexes = true } if compatible { mergedColumns = append(mergedColumns, *theirs) @@ -463,9 +470,10 @@ func mergeColumns(tblName string, format *storetypes.NomsBinFormat, ourCC, their 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 compatible, rewrite := compatChecker.IsTypeChangeCompatible(theirs.TypeInfo, ours.TypeInfo) if rewrite { - tableRewrite = true + mergeInfo.InvalidateSecondaryIndexes = true } if compatible { mergedColumns = append(mergedColumns, *ours) @@ -498,15 +506,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, tree.ThreeWayDiffInfo{}, tableRewrite, nil + return nil, conflicts, mergeInfo, nil } - diffInfo := tree.ThreeWayDiffInfo{ - LeftSchemaChange: ourSchemaChanged, - RightSchemaChange: theirSchemaChanged, - LeftAndRightSchemasDiffer: leftAndRightSchemasDiffer, - } - return schema.NewColCollection(mergedColumns...), nil, diffInfo, tableRewrite, nil + return schema.NewColCollection(mergedColumns...), nil, mergeInfo, 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 a2847461b9..d4af9de154 100644 --- a/go/libraries/doltcore/merge/schema_integration_test.go +++ b/go/libraries/doltcore/merge/schema_integration_test.go @@ -635,8 +635,8 @@ func testMergeSchemasWithConflicts(t *testing.T, test mergeSchemaConflictTest) { otherSch := getSchema(t, dEnv) - _, actConflicts, _, rebuildIndexes, err := merge.SchemaMerge(context.Background(), types.Format_Default, mainSch, otherSch, ancSch, "test") - assert.False(t, rebuildIndexes) + _, 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)) return diff --git a/integration-tests/bats/default-values.bats b/integration-tests/bats/default-values.bats index 93169c172a..777fab1336 100644 --- a/integration-tests/bats/default-values.bats +++ b/integration-tests/bats/default-values.bats @@ -1,571 +1,571 @@ -#!/usr/bin/env bats -load $BATS_TEST_DIRNAME/helper/common.bash - -setup() { - setup_common - - dolt sql < bad-update.csv -pk,v1,v2 -5,,5 -6,,6 -DELIM - - run dolt table import -u test bad-update.csv - [ "$status" -eq "1" ] - [[ "$output" =~ "bad row" ]] || false - [[ "$output" =~ "pk: 5" ]] || false - [[ "$output" =~ "v1: " ]] || false - [[ "$output" =~ "v2: 5" ]] || false - [[ "$output" =~ "column name 'v1' is non-nullable but attempted to set a value of null" ]] || false -} - -@test "default-values: Defining literal default value to NULL or EMPTY value in ALTER TABLE" { - dolt sql -q "CREATE TABLE test(pk BIGINT PRIMARY KEY, c1 BIGINT, c2 BIGINT, c3 INT)" - run dolt sql -q "SELECT column_name, is_nullable, column_default FROM information_schema.columns WHERE table_name = 'test'" -r=csv - [ "$status" -eq "0" ] - [[ "$output" =~ "COLUMN_NAME,IS_NULLABLE,COLUMN_DEFAULT" ]] || false - [[ "$output" =~ "pk,NO," ]] || false - [[ "$output" =~ "c1,YES," ]] || false - [[ "$output" =~ "c2,YES," ]] || false - [[ "$output" =~ "c3,YES," ]] || false - - dolt sql -q "ALTER TABLE test CHANGE c1 c1 INT NULL DEFAULT NULL" - dolt sql -q "ALTER TABLE test CHANGE c2 c2 varchar(4) NOT NULL DEFAULT ''" - dolt sql -q "ALTER TABLE test CHANGE c3 c3 varchar(4) NOT NULL DEFAULT 'ln'" - run dolt sql -q "SELECT column_name, is_nullable, column_default FROM information_schema.columns WHERE table_name = 'test'" -r=csv - [ "$status" -eq "0" ] - [[ "$output" =~ "COLUMN_NAME,IS_NULLABLE,COLUMN_DEFAULT" ]] || false - [[ "$output" =~ "pk,NO," ]] || false - [[ "$output" =~ "c1,YES," ]] || false - [[ "$output" =~ "c2,NO," ]] || false - [[ "$output" =~ "c3,NO,ln" ]] || false - - dolt sql -q "ALTER TABLE test CHANGE c1 c1 FLOAT NOT NULL DEFAULT '4.44'" - dolt sql -q "ALTER TABLE test CHANGE c2 c2 DOUBLE NOT NULL DEFAULT '3.333'" - dolt sql -q "ALTER TABLE test CHANGE c3 c3 BOOLEAN NULL DEFAULT FALSE" - run dolt sql -q "SELECT column_name, is_nullable, column_default FROM information_schema.columns WHERE table_name = 'test'" -r=csv - [ "$status" -eq "0" ] - [[ "$output" =~ "COLUMN_NAME,IS_NULLABLE,COLUMN_DEFAULT" ]] || false - [[ "$output" =~ "pk,NO," ]] || false - [[ "$output" =~ "c1,NO,4.44" ]] || false - [[ "$output" =~ "c2,NO,3.333" ]] || false - [[ "$output" =~ "c3,YES,0" ]] || false - - dolt sql -q "ALTER TABLE test CHANGE c1 c1 DATETIME NOT NULL DEFAULT '2020-04-01 16:16:16'" - dolt sql -q "ALTER TABLE test CHANGE c2 c2 TIMESTAMP NULL DEFAULT '2008-04-22 16:16:16'" - run dolt sql -q "SELECT column_name, is_nullable, column_default FROM information_schema.columns WHERE table_name = 'test'" -r=csv - [ "$status" -eq "0" ] - [[ "$output" =~ "COLUMN_NAME,IS_NULLABLE,COLUMN_DEFAULT" ]] || false - [[ "$output" =~ "pk,NO," ]] || false - [[ "$output" =~ "c1,NO,2020-04-01 16:16:16" ]] || false - [[ "$output" =~ "c2,YES,2008-04-22 16:16:16" ]] || false - [[ "$output" =~ "c3,YES,0" ]] || false -} - -@test "default-values: Function default values are correctly represented in the information_schema.columns table" { - dolt sql -q "CREATE TABLE test(pk BIGINT PRIMARY KEY, v1 SMALLINT DEFAULT (GREATEST(pk, 2)))" - run dolt sql -q "SELECT column_name, is_nullable, column_default FROM information_schema.columns WHERE table_name = 'test'" - [ "$status" -eq "0" ] - [[ "$output" =~ "| pk | NO | NULL |" ]] || false - [[ "$output" =~ "| v1 | YES | greatest(pk,2) |" ]] || false -} - -@test "default-values: Additional test with function defaults" { - dolt sql -q "CREATE TABLE test_table (pk int primary key, fname varchar(20), lname varchar(20), height int)" - dolt sql -q "ALTER TABLE test_table CHANGE fname col2 float NOT NULL DEFAULT (length('hello'))" - dolt sql -q "ALTER TABLE test_table CHANGE lname col3 double NOT NULL DEFAULT (ROUND(-1.58))" - dolt sql -q "ALTER TABLE test_table CHANGE height col4 float NULL DEFAULT (RAND())" - - run dolt sql -q "SELECT column_name, column_default FROM information_schema.columns WHERE table_name = 'test_table'" - [ "$status" -eq "0" ] - [[ "$output" =~ "| pk | NULL |" ]] || false - [[ "$output" =~ "| col2 | length('hello') |" ]] || false - [[ "$output" =~ "| col3 | round(-1.58,0) |" ]] || false - [[ "$output" =~ "| col4 | rand() |" ]] || false -} - -@test "default-values: Outputting the string version of a more complex default value works" { - dolt sql -q "CREATE TABLE test_table (pk int primary key, fname varchar(20), lname varchar(20))" - dolt sql -q "ALTER TABLE test_table CHANGE fname col2 INT NULL DEFAULT (RAND()+RAND());" - dolt sql -q "ALTER TABLE test_table CHANGE lname col3 BOOLEAN NULL DEFAULT (CASE pk WHEN 1 THEN false ELSE true END);" - - run dolt sql -q "SELECT column_name, column_default FROM information_schema.columns where table_name='test_table'" -r csv - [ "$status" -eq "0" ] - [[ "$output" =~ "COLUMN_NAME,COLUMN_DEFAULT" ]] || false - [[ "$output" =~ "pk," ]] || false - [[ "$output" =~ "col2,(rand() + rand())" ]] || false - [[ "$output" =~ "col3,CASE pk WHEN 1 THEN false ELSE true END" ]] || false -} +#!/usr/bin/env bats +load $BATS_TEST_DIRNAME/helper/common.bash + +setup() { + setup_common + + dolt sql < bad-update.csv +pk,v1,v2 +5,,5 +6,,6 +DELIM + + run dolt table import -u test bad-update.csv + [ "$status" -eq "1" ] + [[ "$output" =~ "bad row" ]] || false + [[ "$output" =~ "pk: 5" ]] || false + [[ "$output" =~ "v1: " ]] || false + [[ "$output" =~ "v2: 5" ]] || false + [[ "$output" =~ "column name 'v1' is non-nullable but attempted to set a value of null" ]] || false +} + +@test "default-values: Defining literal default value to NULL or EMPTY value in ALTER TABLE" { + dolt sql -q "CREATE TABLE test(pk BIGINT PRIMARY KEY, c1 BIGINT, c2 BIGINT, c3 INT)" + run dolt sql -q "SELECT column_name, is_nullable, column_default FROM information_schema.columns WHERE table_name = 'test'" -r=csv + [ "$status" -eq "0" ] + [[ "$output" =~ "COLUMN_NAME,IS_NULLABLE,COLUMN_DEFAULT" ]] || false + [[ "$output" =~ "pk,NO," ]] || false + [[ "$output" =~ "c1,YES," ]] || false + [[ "$output" =~ "c2,YES," ]] || false + [[ "$output" =~ "c3,YES," ]] || false + + dolt sql -q "ALTER TABLE test CHANGE c1 c1 INT NULL DEFAULT NULL" + dolt sql -q "ALTER TABLE test CHANGE c2 c2 varchar(4) NOT NULL DEFAULT ''" + dolt sql -q "ALTER TABLE test CHANGE c3 c3 varchar(4) NOT NULL DEFAULT 'ln'" + run dolt sql -q "SELECT column_name, is_nullable, column_default FROM information_schema.columns WHERE table_name = 'test'" -r=csv + [ "$status" -eq "0" ] + [[ "$output" =~ "COLUMN_NAME,IS_NULLABLE,COLUMN_DEFAULT" ]] || false + [[ "$output" =~ "pk,NO," ]] || false + [[ "$output" =~ "c1,YES," ]] || false + [[ "$output" =~ "c2,NO," ]] || false + [[ "$output" =~ "c3,NO,ln" ]] || false + + dolt sql -q "ALTER TABLE test CHANGE c1 c1 FLOAT NOT NULL DEFAULT '4.44'" + dolt sql -q "ALTER TABLE test CHANGE c2 c2 DOUBLE NOT NULL DEFAULT '3.333'" + dolt sql -q "ALTER TABLE test CHANGE c3 c3 BOOLEAN NULL DEFAULT FALSE" + run dolt sql -q "SELECT column_name, is_nullable, column_default FROM information_schema.columns WHERE table_name = 'test'" -r=csv + [ "$status" -eq "0" ] + [[ "$output" =~ "COLUMN_NAME,IS_NULLABLE,COLUMN_DEFAULT" ]] || false + [[ "$output" =~ "pk,NO," ]] || false + [[ "$output" =~ "c1,NO,4.44" ]] || false + [[ "$output" =~ "c2,NO,3.333" ]] || false + [[ "$output" =~ "c3,YES,0" ]] || false + + dolt sql -q "ALTER TABLE test CHANGE c1 c1 DATETIME NOT NULL DEFAULT '2020-04-01 16:16:16'" + dolt sql -q "ALTER TABLE test CHANGE c2 c2 TIMESTAMP NULL DEFAULT '2008-04-22 16:16:16'" + run dolt sql -q "SELECT column_name, is_nullable, column_default FROM information_schema.columns WHERE table_name = 'test'" -r=csv + [ "$status" -eq "0" ] + [[ "$output" =~ "COLUMN_NAME,IS_NULLABLE,COLUMN_DEFAULT" ]] || false + [[ "$output" =~ "pk,NO," ]] || false + [[ "$output" =~ "c1,NO,2020-04-01 16:16:16" ]] || false + [[ "$output" =~ "c2,YES,2008-04-22 16:16:16" ]] || false + [[ "$output" =~ "c3,YES,0" ]] || false +} + +@test "default-values: Function default values are correctly represented in the information_schema.columns table" { + dolt sql -q "CREATE TABLE test(pk BIGINT PRIMARY KEY, v1 SMALLINT DEFAULT (GREATEST(pk, 2)))" + run dolt sql -q "SELECT column_name, is_nullable, column_default FROM information_schema.columns WHERE table_name = 'test'" + [ "$status" -eq "0" ] + [[ "$output" =~ "| pk | NO | NULL |" ]] || false + [[ "$output" =~ "| v1 | YES | greatest(pk,2) |" ]] || false +} + +@test "default-values: Additional test with function defaults" { + dolt sql -q "CREATE TABLE test_table (pk int primary key, fname varchar(20), lname varchar(20), height int)" + dolt sql -q "ALTER TABLE test_table CHANGE fname col2 float NOT NULL DEFAULT (length('hello'))" + dolt sql -q "ALTER TABLE test_table CHANGE lname col3 double NOT NULL DEFAULT (ROUND(-1.58))" + dolt sql -q "ALTER TABLE test_table CHANGE height col4 float NULL DEFAULT (RAND())" + + run dolt sql -q "SELECT column_name, column_default FROM information_schema.columns WHERE table_name = 'test_table'" + [ "$status" -eq "0" ] + [[ "$output" =~ "| pk | NULL |" ]] || false + [[ "$output" =~ "| col2 | length('hello') |" ]] || false + [[ "$output" =~ "| col3 | round(-1.58,0) |" ]] || false + [[ "$output" =~ "| col4 | rand() |" ]] || false +} + +@test "default-values: Outputting the string version of a more complex default value works" { + dolt sql -q "CREATE TABLE test_table (pk int primary key, fname varchar(20), lname varchar(20))" + dolt sql -q "ALTER TABLE test_table CHANGE fname col2 INT NULL DEFAULT (RAND()+RAND());" + dolt sql -q "ALTER TABLE test_table CHANGE lname col3 BOOLEAN NULL DEFAULT (CASE pk WHEN 1 THEN false ELSE true END);" + + run dolt sql -q "SELECT column_name, column_default FROM information_schema.columns where table_name='test_table'" -r csv + [ "$status" -eq "0" ] + [[ "$output" =~ "COLUMN_NAME,COLUMN_DEFAULT" ]] || false + [[ "$output" =~ "pk," ]] || false + [[ "$output" =~ "col2,(rand() + rand())" ]] || false + [[ "$output" =~ "col3,CASE pk WHEN 1 THEN false ELSE true END" ]] || false +}