Merge pull request #8578 from dolthub/taylor/fix-rebase

Fix dolt_rebase and dolt_ignore for doltgres

go/cmd/dolt/commands/stashcmds/pop.go

@@ -169,15 +169,15 @@ func applyStashAtIdx(ctx *sql.Context, dEnv *env.DoltEnv, curWorkingRoot doltdb.
 		return false, err
 	}
 
-	var tablesWithConflict []string
+	var tablesWithConflict []doltdb.TableName
 	for tbl, stats := range result.Stats {
 		if stats.HasConflicts() {
 			tablesWithConflict = append(tablesWithConflict, tbl)
 		}
 	}
 
 	if len(tablesWithConflict) > 0 {
-		tblNames := strings.Join(tablesWithConflict, "', '")
+		tblNames := strings.Join(doltdb.FlattenTableNames(tablesWithConflict), "', '")
 		cli.Printf("error: Your local changes to the following tables would be overwritten by applying stash %d:\n"+
 			"\t{'%s'}\n"+
 			"Please commit your changes or stash them before you merge.\nAborting\n", idx, tblNames)

go/libraries/doltcore/cherry_pick/cherry_pick.go

@@ -391,16 +391,16 @@ func rootsEqual(root1, root2 doltdb.RootValue) (bool, error) {
 
 // stageCherryPickedTables stages the tables from |mergeStats| that don't have any merge artifacts – i.e.
 // tables that don't have any data or schema conflicts and don't have any constraint violations.
-func stageCherryPickedTables(ctx *sql.Context, mergeStats map[string]*merge.MergeStats) (err error) {
-	tablesToAdd := make([]string, 0, len(mergeStats))
+func stageCherryPickedTables(ctx *sql.Context, mergeStats map[doltdb.TableName]*merge.MergeStats) (err error) {
+	tablesToAdd := make([]doltdb.TableName, 0, len(mergeStats))
 	for tableName, mergeStats := range mergeStats {
 		if mergeStats.HasArtifacts() {
 			continue
 		}
 
 		// Find any tables being deleted and make sure we stage those tables first
 		if mergeStats.Operation == merge.TableRemoved {
-			tablesToAdd = append([]string{tableName}, tablesToAdd...)
+			tablesToAdd = append([]doltdb.TableName{tableName}, tablesToAdd...)
 		} else {
 			tablesToAdd = append(tablesToAdd, tableName)
 		}
@@ -413,8 +413,7 @@ func stageCherryPickedTables(ctx *sql.Context, mergeStats map[string]*merge.Merg
 		return fmt.Errorf("unable to get roots for database '%s' from session", dbName)
 	}
 
-	// TODO: schema name
-	roots, err = actions.StageTables(ctx, roots, doltdb.ToTableNames(tablesToAdd, doltdb.DefaultSchemaName), true)
+	roots, err = actions.StageTables(ctx, roots, tablesToAdd, true)
 	if err != nil {
 		return err
 	}

go/libraries/doltcore/doltdb/ignore.go

@@ -54,6 +54,20 @@ const (
 
 type IgnorePatterns []IgnorePattern
 
+// ConvertTupleToIgnoreBoolean is a function that converts a Tuple to a boolean for the ignore field. This is used to handle the Doltgres extended boolean type.
+var ConvertTupleToIgnoreBoolean = convertTupleToIgnoreBoolean
+
+func convertTupleToIgnoreBoolean(valueDesc val.TupleDesc, valueTuple val.Tuple) (bool, error) {
+	if !valueDesc.Equals(val.NewTupleDescriptor(val.Type{Enc: val.Int8Enc, Nullable: false})) {
+		return false, fmt.Errorf("dolt_ignore had unexpected value type, this should never happen")
+	}
+	ignore, ok := valueDesc.GetBool(0, valueTuple)
+	if !ok {
+		return false, fmt.Errorf("could not read boolean")
+	}
+	return ignore, nil
+}
+
 func GetIgnoredTablePatterns(ctx context.Context, roots Roots) (IgnorePatterns, error) {
 	var ignorePatterns []IgnorePattern
 	workingSet := roots.Working
@@ -82,9 +96,6 @@ func GetIgnoredTablePatterns(ctx context.Context, roots Roots) (IgnorePatterns,
 	if !keyDesc.Equals(val.NewTupleDescriptor(val.Type{Enc: val.StringEnc})) {
 		return nil, fmt.Errorf("dolt_ignore had unexpected key type, this should never happen")
 	}
-	if !valueDesc.Equals(val.NewTupleDescriptor(val.Type{Enc: val.Int8Enc, Nullable: true})) {
-		return nil, fmt.Errorf("dolt_ignore had unexpected value type, this should never happen")
-	}
 
 	ignoreTableMap, err := durable.ProllyMapFromIndex(index).IterAll(ctx)
 	if err != nil {
@@ -103,7 +114,11 @@ func GetIgnoredTablePatterns(ctx context.Context, roots Roots) (IgnorePatterns,
 		if !ok {
 			return nil, fmt.Errorf("could not read pattern")
 		}
-		ignore, ok := valueDesc.GetBool(0, valueTuple)
+		ignore, err := ConvertTupleToIgnoreBoolean(valueDesc, valueTuple)
+		if err != nil {
+			return nil, err
+		}
+
 		ignorePatterns = append(ignorePatterns, NewIgnorePattern(pattern, ignore))
 	}
 	return ignorePatterns, nil
@@ -125,8 +140,6 @@ func ExcludeIgnoredTables(ctx context.Context, roots Roots, tables []TableName)
 		}
 		if conflict := AsDoltIgnoreInConflict(err); conflict != nil {
 			// no-op
-		} else if err != nil {
-			return nil, err
 		} else if ignored == DontIgnore {
 			// no-op
 		} else if ignored == Ignore {

go/libraries/doltcore/merge/merge.go

@@ -84,7 +84,7 @@ func MergeCommits(ctx *sql.Context, commit, mergeCommit *doltdb.Commit, opts edi
 type Result struct {
 	Root            doltdb.RootValue
 	SchemaConflicts []SchemaConflict
-	Stats           map[string]*MergeStats
+	Stats           map[doltdb.TableName]*MergeStats
 }
 
 func (r Result) HasSchemaConflicts() bool {
@@ -231,7 +231,7 @@ func MergeRoots(
 		return nil, err
 	}
 
-	tblToStats := make(map[string]*MergeStats)
+	tblToStats := make(map[doltdb.TableName]*MergeStats)
 
 	// Merge tables one at a time. This is done based on name. With table names from ourRoot being merged first,
 	// renaming a table will return delete/modify conflict error consistently.
@@ -257,7 +257,7 @@ func MergeRoots(
 			// If there's a true conflict, then the parent table will catch the conflict.
 			stats = &MergeStats{Operation: TableModified}
 		} else if errors.Is(ErrTableDeletedAndSchemaModified, err) {
-			tblToStats[tblName.Name] = &MergeStats{
+			tblToStats[tblName] = &MergeStats{
 				Operation:       TableModified,
 				SchemaConflicts: 1,
 			}
@@ -292,7 +292,7 @@ func MergeRoots(
 		}
 
 		if mergedTable.table != nil {
-			tblToStats[tblName.Name] = stats
+			tblToStats[tblName] = stats
 
 			// edge case: if we're merging a table with a schema name to a root that doesn't have that schema,
 			// we implicitly create that schema on the destination root in addition to updating the list of schemas
@@ -320,7 +320,7 @@ func MergeRoots(
 
 		if mergedRootHasTable {
 			// Merge root deleted this table
-			tblToStats[tblName.Name] = &MergeStats{Operation: TableRemoved}
+			tblToStats[tblName] = &MergeStats{Operation: TableRemoved}
 
 			// TODO: drop schemas as necessary
 			mergedRoot, err = mergedRoot.RemoveTables(ctx, false, false, tblName)
@@ -367,7 +367,7 @@ func MergeRoots(
 	var tableSet *doltdb.TableNameSet = nil
 	if mergeOpts.RecordViolationsForTables != nil {
 		tableSet = doltdb.NewCaseInsensitiveTableNameSet(nil)
-		for tableName, _ := range mergeOpts.RecordViolationsForTables {
+		for tableName := range mergeOpts.RecordViolationsForTables {
 			tableSet.Add(tableName)
 		}
 	}
@@ -456,7 +456,7 @@ func mergeCVsWithStash(ctx context.Context, root doltdb.RootValue, stash *violat
 }
 
 // checks if a conflict occurred during the merge
-func checkForConflicts(tblToStats map[string]*MergeStats) bool {
+func checkForConflicts(tblToStats map[doltdb.TableName]*MergeStats) bool {
 	for _, stat := range tblToStats {
 		if stat.HasConflicts() {
 			return true
@@ -466,9 +466,9 @@ func checkForConflicts(tblToStats map[string]*MergeStats) bool {
 }
 
 // populates tblToStats with violation statistics
-func getConstraintViolationStats(ctx context.Context, root doltdb.RootValue, tblToStats map[string]*MergeStats) error {
+func getConstraintViolationStats(ctx context.Context, root doltdb.RootValue, tblToStats map[doltdb.TableName]*MergeStats) error {
 	for tblName, stats := range tblToStats {
-		tbl, ok, err := root.GetTable(ctx, doltdb.TableName{Name: tblName})
+		tbl, ok, err := root.GetTable(ctx, tblName)
 		if err != nil {
 			return err
 		}
@@ -523,7 +523,7 @@ func MergeWouldStompChanges(ctx context.Context, roots doltdb.Roots, mergeCommit
 	mergedHeadDiffs := diffTableHashes(headTableHashes, mergeTableHashes)
 
 	stompedTables := make([]doltdb.TableName, 0, len(headWorkingDiffs))
-	for tName, _ := range headWorkingDiffs {
+	for tName := range headWorkingDiffs {
 		if _, ok := mergedHeadDiffs[tName]; ok {
 			// even if the working changes match the merge changes, don't allow (matches git behavior).
 			stompedTables = append(stompedTables, tName)

go/libraries/doltcore/sqle/database.go

@@ -2331,6 +2331,29 @@ func (db Database) SetCollation(ctx *sql.Context, collation sql.CollationID) err
 	return db.SetRoot(ctx, newRoot)
 }
 
+// ConvertRowToRebasePlanStep converts a sql.Row to RebasePlanStep. This is used by Doltgres to convert
+// from a sql.Row considering the correct types.
+var ConvertRowToRebasePlanStep = convertRowToRebasePlanStep
+
+func convertRowToRebasePlanStep(row sql.Row) (rebase.RebasePlanStep, error) {
+	i, ok := row[1].(uint16)
+	if !ok {
+		return rebase.RebasePlanStep{}, fmt.Errorf("invalid enum value in rebase plan: %v (%T)", row[1], row[1])
+	}
+
+	rebaseAction, ok := dprocedures.RebaseActionEnumType.At(int(i))
+	if !ok {
+		return rebase.RebasePlanStep{}, fmt.Errorf("invalid enum value in rebase plan: %v (%T)", row[1], row[1])
+	}
+
+	return rebase.RebasePlanStep{
+		RebaseOrder: row[0].(decimal.Decimal),
+		Action:      rebaseAction,
+		CommitHash:  row[2].(string),
+		CommitMsg:   row[3].(string),
+	}, nil
+}
+
 // LoadRebasePlan implements the rebase.RebasePlanDatabase interface
 func (db Database) LoadRebasePlan(ctx *sql.Context) (*rebase.RebasePlan, error) {
 	table, ok, err := db.GetTableInsensitive(ctx, doltdb.RebaseTableName)
@@ -2341,8 +2364,9 @@ func (db Database) LoadRebasePlan(ctx *sql.Context) (*rebase.RebasePlan, error)
 		return nil, fmt.Errorf("unable to find dolt_rebase table")
 	}
 	resolvedTable := plan.NewResolvedTable(table, db, nil)
+	rebaseSchema := dprocedures.GetDoltRebaseSystemTableSchema()
 	sort := plan.NewSort([]sql.SortField{{
-		Column: expression.NewGetField(0, types.MustCreateDecimalType(6, 2), "rebase_order", false),
+		Column: expression.NewGetField(0, rebaseSchema[0].Type, "rebase_order", false),
 		Order:  sql.Ascending,
 	}}, resolvedTable)
 	iter, err := rowexec.DefaultBuilder.Build(ctx, sort, nil)
@@ -2359,29 +2383,37 @@ func (db Database) LoadRebasePlan(ctx *sql.Context) (*rebase.RebasePlan, error)
 			return nil, err
 		}
 
-		i, ok := row[1].(uint16)
-		if !ok {
-			return nil, fmt.Errorf("invalid enum value in rebase plan: %v (%T)", row[1], row[1])
-		}
-		rebaseAction, ok := dprocedures.RebaseActionEnumType.At(int(i))
-		if !ok {
-			return nil, fmt.Errorf("invalid enum value in rebase plan: %v (%T)", row[1], row[1])
+		newRebasePlan, err := ConvertRowToRebasePlanStep(row)
+		if err != nil {
+			return nil, err
 		}
 
-		rebasePlan.Steps = append(rebasePlan.Steps, rebase.RebasePlanStep{
-			RebaseOrder: row[0].(decimal.Decimal),
-			Action:      rebaseAction,
-			CommitHash:  row[2].(string),
-			CommitMsg:   row[3].(string),
-		})
+		rebasePlan.Steps = append(rebasePlan.Steps, newRebasePlan)
 	}
 
 	return &rebasePlan, nil
 }
 
+// ConvertRebasePlanStepToRow converts a RebasePlanStep to sql.Row. This is used by Doltgres to convert
+// to a sql.Row with the correct types.
+var ConvertRebasePlanStepToRow = convertRebasePlanStepToRow
+
+func convertRebasePlanStepToRow(planMember rebase.RebasePlanStep) (sql.Row, error) {
+	actionEnumValue := dprocedures.RebaseActionEnumType.IndexOf(strings.ToLower(planMember.Action))
+	if actionEnumValue == -1 {
+		return nil, fmt.Errorf("invalid rebase action: %s", planMember.Action)
+	}
+	return sql.Row{
+		planMember.RebaseOrder,
+		uint16(actionEnumValue),
+		planMember.CommitHash,
+		planMember.CommitMsg,
+	}, nil
+}
+
 // SaveRebasePlan implements the rebase.RebasePlanDatabase interface
 func (db Database) SaveRebasePlan(ctx *sql.Context, plan *rebase.RebasePlan) error {
-	pkSchema := sql.NewPrimaryKeySchema(dprocedures.DoltRebaseSystemTableSchema)
+	pkSchema := sql.NewPrimaryKeySchema(dprocedures.GetDoltRebaseSystemTableSchema())
 	// we use createSqlTable, instead of CreateTable to avoid the "dolt_" reserved prefix table name check
 	err := db.createSqlTable(ctx, doltdb.RebaseTableName, "", pkSchema, sql.Collation_Default, "")
 	if err != nil {
@@ -2403,16 +2435,12 @@ func (db Database) SaveRebasePlan(ctx *sql.Context, plan *rebase.RebasePlan) err
 
 	inserter := writeableDoltTable.Inserter(ctx)
 	for _, planMember := range plan.Steps {
-		actionEnumValue := dprocedures.RebaseActionEnumType.IndexOf(strings.ToLower(planMember.Action))
-		if actionEnumValue == -1 {
-			return fmt.Errorf("invalid rebase action: %s", planMember.Action)
-		}
-		err = inserter.Insert(ctx, sql.Row{
-			planMember.RebaseOrder,
-			uint16(actionEnumValue),
-			planMember.CommitHash,
-			planMember.CommitMsg,
-		})
+		row, err := ConvertRebasePlanStepToRow(planMember)
+		if err != nil {
+			return err
+		}
+
+		err = inserter.Insert(ctx, row)
 		if err != nil {
 			return err
 		}

go/libraries/doltcore/sqle/dprocedures/dolt_merge.go

@@ -396,7 +396,7 @@ func executeNoFFMerge(
 	if err != nil {
 		return nil, nil, err
 	}
-	result := &merge.Result{Root: mergeRoot, Stats: make(map[string]*merge.MergeStats)}
+	result := &merge.Result{Root: mergeRoot, Stats: make(map[doltdb.TableName]*merge.MergeStats)}
 
 	ws, err = mergeRootToWorking(ctx, dSess, dbName, false, spec.Force, ws, result, spec.WorkingDiffs, spec.MergeC, spec.MergeCSpecStr)
 	if err != nil {

go/libraries/doltcore/sqle/dprocedures/dolt_rebase.go

@@ -55,28 +55,34 @@ var RebaseActionEnumType = types.MustCreateEnumType([]string{
 	rebase.RebaseActionSquash,
 	rebase.RebaseActionFixup}, sql.Collation_Default)
 
-var DoltRebaseSystemTableSchema = []*sql.Column{
-	{
-		Name:       "rebase_order",
-		Type:       types.MustCreateDecimalType(6, 2),
-		Nullable:   false,
-		PrimaryKey: true,
-	},
-	{
-		Name:     "action",
-		Type:     RebaseActionEnumType,
-		Nullable: false,
-	},
-	{
-		Name:     "commit_hash",
-		Type:     types.Text,
-		Nullable: false,
-	},
-	{
-		Name:     "commit_message",
-		Type:     types.Text,
-		Nullable: false,
-	},
+// GetDoltRebaseSystemTableSchema returns the schema for the dolt_rebase system table.
+// This is used by Doltgres to update the dolt_rebase schema using Doltgres types.
+var GetDoltRebaseSystemTableSchema = getDoltRebaseSystemTableSchema
+
+func getDoltRebaseSystemTableSchema() sql.Schema {
+	return []*sql.Column{
+		{
+			Name:       "rebase_order",
+			Type:       types.MustCreateDecimalType(6, 2),
+			Nullable:   false,
+			PrimaryKey: true,
+		},
+		{
+			Name:     "action",
+			Type:     RebaseActionEnumType,
+			Nullable: false,
+		},
+		{
+			Name:     "commit_hash",
+			Type:     types.Text,
+			Nullable: false,
+		},
+		{
+			Name:     "commit_message",
+			Type:     types.Text,
+			Nullable: false,
+		},
+	}
 }
 
 // ErrRebaseUncommittedChanges is used when a rebase is started, but there are uncommitted (and not