refactor(diff): add renamed filter and removed alias

Add DiffTypeRenamed constant and "removed" as user-friendly alias for "dropped" filter option. This provides more granular filtering and improves user experience with familiar terminology. List of changes: - Add DiffTypeRenamed constant for renamed tables - Revert GetSummary to use DiffTypeRenamed instead of treating renames as modified (table_deltas.go:742) - Add "removed" as alias that maps to "dropped" internally for user convenience - Update filter validation to include renamed option - Update CLI help text to document all filter options including renamed and removed alias - Handle renamed tables in merge stats alongside modified - Update getDiffSummariesBetweenRefs and getSchemaDiffSummariesBetweenRefs to handle renamed diff type - Update all tests to use new constants and test renamed filtering Filter options now available: added, modified, renamed, dropped, removed (alias for dropped). Refs: #1430
2026-01-10 08:49:43 -06:00 · 2025-11-28 19:29:44 -08:00
parent 4a3ff0e883
commit 3c9fb41e10
4 changed files with 57 additions and 51 deletions
--- a/go/cmd/dolt/commands/diff.go
+++ b/go/cmd/dolt/commands/diff.go
@@ -87,7 +87,7 @@ The diffs displayed can be limited to show the first N by providing the paramete

 To filter which data rows are displayed, use {{.EmphasisLeft}}--where <SQL expression>{{.EmphasisRight}}. Table column names in the filter expression must be prefixed with {{.EmphasisLeft}}from_{{.EmphasisRight}} or {{.EmphasisLeft}}to_{{.EmphasisRight}}, e.g. {{.EmphasisLeft}}to_COLUMN_NAME > 100{{.EmphasisRight}} or {{.EmphasisLeft}}from_COLUMN_NAME + to_COLUMN_NAME = 0{{.EmphasisRight}}.

-To filter diff output by change type, use {{.EmphasisLeft}}--filter <type>{{.EmphasisRight}} where {{.EmphasisLeft}}<type>{{.EmphasisRight}} is one of {{.EmphasisLeft}}added{{.EmphasisRight}}, {{.EmphasisLeft}}modified{{.EmphasisRight}}, or {{.EmphasisLeft}}removed{{.EmphasisRight}}. The {{.EmphasisLeft}}added{{.EmphasisRight}} filter shows only additions (new tables or rows), {{.EmphasisLeft}}modified{{.EmphasisRight}} shows only modifications (schema changes, renames, or row updates), and {{.EmphasisLeft}}removed{{.EmphasisRight}} shows only deletions (dropped tables or deleted rows). For example, {{.EmphasisLeft}}dolt diff --filter=removed{{.EmphasisRight}} shows only deleted rows and dropped tables.
+To filter diff output by change type, use {{.EmphasisLeft}}--filter <type>{{.EmphasisRight}} where {{.EmphasisLeft}}<type>{{.EmphasisRight}} is one of {{.EmphasisLeft}}added{{.EmphasisRight}}, {{.EmphasisLeft}}modified{{.EmphasisRight}}, {{.EmphasisLeft}}renamed{{.EmphasisRight}}, or {{.EmphasisLeft}}dropped{{.EmphasisRight}}. The {{.EmphasisLeft}}added{{.EmphasisRight}} filter shows only additions (new tables or rows), {{.EmphasisLeft}}modified{{.EmphasisRight}} shows only schema modifications or row updates, {{.EmphasisLeft}}renamed{{.EmphasisRight}} shows only renamed tables, and {{.EmphasisLeft}}dropped{{.EmphasisRight}} shows only deletions (dropped tables or deleted rows). You can also use {{.EmphasisLeft}}removed{{.EmphasisRight}} as an alias for {{.EmphasisLeft}}dropped{{.EmphasisRight}}. For example, {{.EmphasisLeft}}dolt diff --filter=dropped{{.EmphasisRight}} shows only deleted rows and dropped tables.

 The {{.EmphasisLeft}}--diff-mode{{.EmphasisRight}} argument controls how modified rows are presented when the format output is set to {{.EmphasisLeft}}tabular{{.EmphasisRight}}. When set to {{.EmphasisLeft}}row{{.EmphasisRight}}, modified rows are presented as old and new rows. When set to {{.EmphasisLeft}}line{{.EmphasisRight}}, modified rows are presented as a single row, and changes are presented using "+" and "-" within the column. When set to {{.EmphasisLeft}}in-place{{.EmphasisRight}}, modified rows are presented as a single row, and changes are presented side-by-side with a color distinction (requires a color-enabled terminal). When set to {{.EmphasisLeft}}context{{.EmphasisRight}}, rows that contain at least one column that spans multiple lines uses {{.EmphasisLeft}}line{{.EmphasisRight}}, while all other rows use {{.EmphasisLeft}}row{{.EmphasisRight}}. The default value is {{.EmphasisLeft}}context{{.EmphasisRight}}.
 `,
@@ -144,14 +144,21 @@ type diffTypeFilter struct {

 // newDiffTypeFilter creates a filter for the specified diff type.
 // Pass diff.DiffTypeAll or empty string to include all types.
+// Accepts "removed" as an alias for "dropped" for user convenience.
 func newDiffTypeFilter(filterType string) *diffTypeFilter {
 	if filterType == "" || filterType == diff.DiffTypeAll {
 		return &diffTypeFilter{filters: nil} // nil means include all
 	}

+	// Map "removed" to "dropped" (alias for user convenience)
+	internalFilterType := filterType
+	if filterType == "removed" {
+		internalFilterType = diff.DiffTypeDropped
+	}
+
 	return &diffTypeFilter{
 		filters: map[string]bool{
-			filterType: true,
+			internalFilterType: true,
 		},
 	}
 }
@@ -176,7 +183,8 @@ func (df *diffTypeFilter) isValid() bool {
 	for filterType := range df.filters {
 		if filterType != diff.DiffTypeAdded &&
 			filterType != diff.DiffTypeModified &&
-			filterType != diff.DiffTypeRemoved {
+			filterType != diff.DiffTypeRenamed &&
+			filterType != diff.DiffTypeDropped {
 			return false
 		}
 	}
@@ -355,8 +363,8 @@ func (cmd DiffCmd) validateArgs(apr *argparser.ArgParseResults) errhand.VerboseE
 	if hasFilter {
 		filter := newDiffTypeFilter(filterValue)
 		if !filter.isValid() {
-			return errhand.BuildDError("invalid filter: %s. Valid values are: %s, %s, %s",
-				filterValue, diff.DiffTypeAdded, diff.DiffTypeModified, diff.DiffTypeRemoved).Build()
+			return errhand.BuildDError("invalid filter: %s. Valid values are: %s, %s, %s, %s (or %s)",
+				filterValue, diff.DiffTypeAdded, diff.DiffTypeModified, diff.DiffTypeRenamed, diff.DiffTypeDropped, "removed").Build()
 		}
 	}

@@ -816,10 +824,10 @@ func getSchemaDiffSummariesBetweenRefs(queryist cli.Queryist, sqlCtx *sql.Contex
 			diffType = diff.DiffTypeAdded
 			tableName = toTable
 		case toTable == "":
-			diffType = diff.DiffTypeRemoved
+			diffType = diff.DiffTypeDropped
 			tableName = fromTable
 		case fromTable != "" && toTable != "" && fromTable != toTable:
-			diffType = diff.DiffTypeModified // Renamed tables are treated as modified
+			diffType = diff.DiffTypeRenamed
 			tableName = toTable
 		default:
 			return nil, fmt.Errorf("error: unexpected schema diff case: fromTable='%s', toTable='%s'", fromTable, toTable)
@@ -881,17 +889,14 @@ func getDiffSummariesBetweenRefs(queryist cli.Queryist, sqlCtx *sql.Context, fro
 		}

 		switch summary.DiffType {
-		case diff.DiffTypeRemoved:
+		case diff.DiffTypeDropped:
 			summary.TableName = summary.FromTableName
 		case diff.DiffTypeAdded:
 			summary.TableName = summary.ToTableName
+		case diff.DiffTypeRenamed:
+			summary.TableName = summary.ToTableName
 		case diff.DiffTypeModified:
-			// For renamed tables, use ToTableName; for other modifications, use FromTableName
-			if summary.FromTableName.Name != summary.ToTableName.Name {
-				summary.TableName = summary.ToTableName
-			} else {
-				summary.TableName = summary.FromTableName
-			}
+			summary.TableName = summary.FromTableName
 		default:
 			return nil, fmt.Errorf("error: unexpected diff type '%s'", summary.DiffType)
 		}
--- a/go/cmd/dolt/commands/diff_filter_test.go
+++ b/go/cmd/dolt/commands/diff_filter_test.go
@@ -33,7 +33,7 @@ func TestDiffTypeFilter_IsValid(t *testing.T) {
 	}{
 		{"valid: added", diff.DiffTypeAdded, true},
 		{"valid: modified", diff.DiffTypeModified, true},
-		{"valid: removed", diff.DiffTypeRemoved, true},
+		{"valid: removed", diff.DiffTypeDropped, true},
 		{"valid: all", diff.DiffTypeAll, true},
 		{"invalid: empty string with nil filter", "", true}, // nil filter is valid
 		{"invalid: random string", "invalid", false},
@@ -68,27 +68,27 @@ func TestDiffTypeFilter_ShouldInclude(t *testing.T) {
 		// Testing with filter=added
 		{"filter=added, check added", diff.DiffTypeAdded, diff.DiffTypeAdded, true},
 		{"filter=added, check modified", diff.DiffTypeAdded, diff.DiffTypeModified, false},
-		{"filter=added, check removed", diff.DiffTypeAdded, diff.DiffTypeRemoved, false},
+		{"filter=added, check removed", diff.DiffTypeAdded, diff.DiffTypeDropped, false},

 		// Testing with filter=modified
 		{"filter=modified, check added", diff.DiffTypeModified, diff.DiffTypeAdded, false},
 		{"filter=modified, check modified", diff.DiffTypeModified, diff.DiffTypeModified, true},
-		{"filter=modified, check removed", diff.DiffTypeModified, diff.DiffTypeRemoved, false},
+		{"filter=modified, check removed", diff.DiffTypeModified, diff.DiffTypeDropped, false},

-		// Testing with filter=removed
-		{"filter=removed, check added", diff.DiffTypeRemoved, diff.DiffTypeAdded, false},
-		{"filter=removed, check modified", diff.DiffTypeRemoved, diff.DiffTypeModified, false},
-		{"filter=removed, check removed", diff.DiffTypeRemoved, diff.DiffTypeRemoved, true},
+		// Testing with filter=dropped
+		{"filter=dropped, check added", diff.DiffTypeDropped, diff.DiffTypeAdded, false},
+		{"filter=dropped, check modified", diff.DiffTypeDropped, diff.DiffTypeModified, false},
+		{"filter=dropped, check removed", diff.DiffTypeDropped, diff.DiffTypeDropped, true},

 		// Testing with filter=all
 		{"filter=all, check added", diff.DiffTypeAll, diff.DiffTypeAdded, true},
 		{"filter=all, check modified", diff.DiffTypeAll, diff.DiffTypeModified, true},
-		{"filter=all, check removed", diff.DiffTypeAll, diff.DiffTypeRemoved, true},
+		{"filter=all, check removed", diff.DiffTypeAll, diff.DiffTypeDropped, true},

 		// Testing with empty filter (nil filters map)
 		{"filter=empty, check added", "", diff.DiffTypeAdded, true},
 		{"filter=empty, check modified", "", diff.DiffTypeModified, true},
-		{"filter=empty, check removed", "", diff.DiffTypeRemoved, true},
+		{"filter=empty, check removed", "", diff.DiffTypeDropped, true},
 	}

 	for _, tt := range tests {
@@ -110,7 +110,7 @@ func TestDiffTypeFilter_ConsistencyAcrossMethods(t *testing.T) {
 		if !df.shouldInclude(diff.DiffTypeAdded) {
 			t.Error("filter=all should include added")
 		}
-		if !df.shouldInclude(diff.DiffTypeRemoved) {
+		if !df.shouldInclude(diff.DiffTypeDropped) {
 			t.Error("filter=all should include removed")
 		}
 		if !df.shouldInclude(diff.DiffTypeModified) {
@@ -125,7 +125,7 @@ func TestDiffTypeFilter_ConsistencyAcrossMethods(t *testing.T) {
 		if !df.shouldInclude(diff.DiffTypeAdded) {
 			t.Error("filter=added should include added")
 		}
-		if df.shouldInclude(diff.DiffTypeRemoved) {
+		if df.shouldInclude(diff.DiffTypeDropped) {
 			t.Error("filter=added should not include removed")
 		}
 		if df.shouldInclude(diff.DiffTypeModified) {
@@ -133,17 +133,17 @@ func TestDiffTypeFilter_ConsistencyAcrossMethods(t *testing.T) {
 		}
 	})

-	t.Run("filter=removed only includes removed", func(t *testing.T) {
-		df := newDiffTypeFilter(diff.DiffTypeRemoved)
+	t.Run("filter=dropped only includes removed", func(t *testing.T) {
+		df := newDiffTypeFilter(diff.DiffTypeDropped)

 		if df.shouldInclude(diff.DiffTypeAdded) {
-			t.Error("filter=removed should not include added")
+			t.Error("filter=dropped should not include added")
 		}
-		if !df.shouldInclude(diff.DiffTypeRemoved) {
-			t.Error("filter=removed should include removed")
+		if !df.shouldInclude(diff.DiffTypeDropped) {
+			t.Error("filter=dropped should include removed")
 		}
 		if df.shouldInclude(diff.DiffTypeModified) {
-			t.Error("filter=removed should not include modified")
+			t.Error("filter=dropped should not include modified")
 		}
 	})

@@ -153,7 +153,7 @@ func TestDiffTypeFilter_ConsistencyAcrossMethods(t *testing.T) {
 		if df.shouldInclude(diff.DiffTypeAdded) {
 			t.Error("filter=modified should not include added")
 		}
-		if df.shouldInclude(diff.DiffTypeRemoved) {
+		if df.shouldInclude(diff.DiffTypeDropped) {
 			t.Error("filter=modified should not include removed")
 		}
 		if !df.shouldInclude(diff.DiffTypeModified) {
@@ -186,7 +186,7 @@ func TestFilterConstants(t *testing.T) {
 	}{
 		{"DiffTypeAdded value", diff.DiffTypeAdded, "added"},
 		{"DiffTypeModified value", diff.DiffTypeModified, "modified"},
-		{"DiffTypeRemoved value", diff.DiffTypeRemoved, "removed"},
+		{"DiffTypeDropped value", diff.DiffTypeDropped, "dropped"},
 		{"DiffTypeAll value", diff.DiffTypeAll, "all"},
 	}

@@ -201,7 +201,7 @@ func TestFilterConstants(t *testing.T) {

 func TestFilterConstants_AreUnique(t *testing.T) {
 	// Test that all filter constants are unique
-	constants := []string{diff.DiffTypeAdded, diff.DiffTypeModified, diff.DiffTypeRemoved, diff.DiffTypeAll}
+	constants := []string{diff.DiffTypeAdded, diff.DiffTypeModified, diff.DiffTypeDropped, diff.DiffTypeAll}
 	seen := make(map[string]bool)

 	for _, c := range constants {
@@ -218,7 +218,7 @@ func TestFilterConstants_AreUnique(t *testing.T) {

 func TestFilterConstants_AreLowercase(t *testing.T) {
 	// Test that filter constants are lowercase (convention)
-	constants := []string{diff.DiffTypeAdded, diff.DiffTypeModified, diff.DiffTypeRemoved, diff.DiffTypeAll}
+	constants := []string{diff.DiffTypeAdded, diff.DiffTypeModified, diff.DiffTypeDropped, diff.DiffTypeAll}

 	for _, c := range constants {
 		if c != strings.ToLower(c) {
@@ -265,7 +265,7 @@ func TestShouldUseLazyHeader(t *testing.T) {
 		},
 		{
 			name:           "don't use lazy: table renamed",
-			filterType:     diff.DiffTypeRemoved,
+			filterType:     diff.DiffTypeDropped,
 			schemaChange:   false,
 			isRename:       true,
 			expectedResult: false,
@@ -285,8 +285,8 @@ func TestShouldUseLazyHeader(t *testing.T) {
 			expectedResult: true,
 		},
 		{
-			name:           "use lazy: filter=removed, data-only",
-			filterType:     diff.DiffTypeRemoved,
+			name:           "use lazy: filter=dropped, data-only",
+			filterType:     diff.DiffTypeDropped,
 			schemaChange:   false,
 			isRename:       false,
 			expectedResult: true,
@@ -498,25 +498,25 @@ func TestShouldSkipRow(t *testing.T) {
 		expectedResult bool
 	}{
 		{"filter=added, row=Added", diff.DiffTypeAdded, diff.Added, false},
-		{"filter=added, row=Removed", diff.DiffTypeAdded, diff.Removed, true},
+		{"filter=added, row=Dropped", diff.DiffTypeAdded, diff.Removed, true},
 		{"filter=added, row=ModifiedOld", diff.DiffTypeAdded, diff.ModifiedOld, true},
 		{"filter=added, row=ModifiedNew", diff.DiffTypeAdded, diff.ModifiedNew, true},

-		{"filter=removed, row=Added", diff.DiffTypeRemoved, diff.Added, true},
-		{"filter=removed, row=Removed", diff.DiffTypeRemoved, diff.Removed, false},
-		{"filter=removed, row=ModifiedOld", diff.DiffTypeRemoved, diff.ModifiedOld, true},
+		{"filter=dropped, row=Added", diff.DiffTypeDropped, diff.Added, true},
+		{"filter=dropped, row=Dropped", diff.DiffTypeDropped, diff.Removed, false},
+		{"filter=dropped, row=ModifiedOld", diff.DiffTypeDropped, diff.ModifiedOld, true},

 		{"filter=modified, row=Added", diff.DiffTypeModified, diff.Added, true},
-		{"filter=modified, row=Removed", diff.DiffTypeModified, diff.Removed, true},
+		{"filter=modified, row=Dropped", diff.DiffTypeModified, diff.Removed, true},
 		{"filter=modified, row=ModifiedOld", diff.DiffTypeModified, diff.ModifiedOld, false},
 		{"filter=modified, row=ModifiedNew", diff.DiffTypeModified, diff.ModifiedNew, false},

 		{"filter=all, row=Added", diff.DiffTypeAll, diff.Added, false},
-		{"filter=all, row=Removed", diff.DiffTypeAll, diff.Removed, false},
+		{"filter=all, row=Dropped", diff.DiffTypeAll, diff.Removed, false},
 		{"filter=all, row=ModifiedOld", diff.DiffTypeAll, diff.ModifiedOld, false},

 		{"nil filter, row=Added", "", diff.Added, false},
-		{"nil filter, row=Removed", "", diff.Removed, false},
+		{"nil filter, row=Dropped", "", diff.Removed, false},
 	}

 	for _, tt := range tests {
--- a/go/cmd/dolt/commands/merge.go
+++ b/go/cmd/dolt/commands/merge.go
@@ -485,12 +485,12 @@ func calculateMergeStats(queryist cli.Queryist, sqlCtx *sql.Context, mergeStats
 			mergeStats[summary.TableName.Name] = &merge.MergeStats{
 				Operation: merge.TableAdded,
 			}
-		} else if summary.DiffType == diff.DiffTypeRemoved {
+		} else if summary.DiffType == diff.DiffTypeDropped {
 			allUnmodified = false
 			mergeStats[summary.TableName.Name] = &merge.MergeStats{
 				Operation: merge.TableRemoved,
 			}
-		} else if summary.DiffType == diff.DiffTypeModified {
+		} else if summary.DiffType == diff.DiffTypeModified || summary.DiffType == diff.DiffTypeRenamed {
 			allUnmodified = false
 			mergeStats[summary.TableName.Name] = &merge.MergeStats{
 				Operation: merge.TableModified,
--- a/go/libraries/doltcore/diff/table_deltas.go
+++ b/go/libraries/doltcore/diff/table_deltas.go
@@ -45,7 +45,8 @@ const (
 const (
 	DiffTypeAdded    = "added"
 	DiffTypeModified = "modified"
-	DiffTypeRemoved  = "removed"
+	DiffTypeRenamed  = "renamed"
+	DiffTypeDropped  = "dropped"
 	DiffTypeAll      = "all"
 )

@@ -114,7 +115,7 @@ func ChangeTypeToDiffType(ct ChangeType) string {
 	case Added:
 		return DiffTypeAdded
 	case Removed:
-		return DiffTypeRemoved
+		return DiffTypeDropped
 	case ModifiedOld, ModifiedNew:
 		// Both ModifiedOld and ModifiedNew represent the same logical change: modified
 		return DiffTypeModified
@@ -715,7 +716,7 @@ func (td TableDelta) GetSummary(ctx context.Context) (*TableDeltaSummary, error)
 			FromTableName: td.FromName,
 			DataChange:    dataChange,
 			SchemaChange:  true,
-			DiffType:      DiffTypeRemoved,
+			DiffType:      DiffTypeDropped,
 		}, nil
 	}

@@ -738,7 +739,7 @@ func (td TableDelta) GetSummary(ctx context.Context) (*TableDeltaSummary, error)
 			ToTableName:   td.ToName,
 			DataChange:    dataChange,
 			SchemaChange:  true,
-			DiffType:      DiffTypeModified,
+			DiffType:      DiffTypeRenamed,
 		}, nil
 	}