Repository: ariga/atlas
Pull request #2934 (2024-07-08 19:55:03)

sql/internal/sqlx: filter diff changes before returning changes


Patch:
diff --git a/schemahcl/extension.go b/schemahcl/extension.go
index 09d01399..16ac86c9 100644
--- a/schemahcl/extension.go
+++ b/schemahcl/extension.go
@@ -221,6 +221,11 @@ func (r *Resource) as(target any) error {
 		children := childrenOfType(r, childType)
 		extras.Children = append(extras.Children, children...)
 	}
+	// In case the resource contains a remain (DefaultExtension)
+	// field, attach to it the position.
+	if r.rang != nil && rem.Remain() != nil {
+		rem.Remain().SetRange(r.rang)
+	}
 	return nil
 }
 
diff --git a/sql/internal/sqlx/diff.go b/sql/internal/sqlx/diff.go
index fa072a8d..8b393e13 100644
--- a/sql/internal/sqlx/diff.go
+++ b/sql/internal/sqlx/diff.go
@@ -378,12 +378,12 @@ func addViewChange(v *schema.View) []schema.Change {
 
 // columnDiff returns the schema changes (if any) for migrating table columns.
 func (d *Diff) columnDiff(from, to *schema.Table, opts *schema.DiffOptions) ([]schema.Change, error) {
-	var changes []schema.Change
+	var all []schema.Change
 	// Drop or modify columns.
 	for _, c1 := range from.Columns {
 		c2, ok := to.Column(c1.Name)
 		if !ok {
-			changes = opts.AddOrSkip(changes, &schema.DropColumn{C: c1})
+			all = append(all, &schema.DropColumn{C: c1})
 			continue
 		}
 		change, err := d.ColumnChange(from, c1, c2)
@@ -391,7 +391,7 @@ func (d *Diff) columnDiff(from, to *schema.Table, opts *schema.DiffOptions) ([]s
 			return nil, err
 		}
 		if change != schema.NoChange {
-			changes = opts.AddOrSkip(changes, &schema.ModifyColumn{
+			all = append(all, &schema.ModifyColumn{
 				From:   c1,
 				To:     c2,
 				Change: change,
@@ -401,12 +401,22 @@ func (d *Diff) columnDiff(from, to *schema.Table, opts *schema.DiffOptions) ([]s
 	// Add columns.
 	for _, c1 := range to.Columns {
 		if _, ok := from.Column(c1.Name); !ok {
-			changes = opts.AddOrSkip(changes, &schema.AddColumn{
+			all = append(all, &schema.AddColumn{
 				C: c1,
 			})
 		}
 	}
-	return d.askForColumns(from, changes, opts)
+	var (
+		err     error
+		changes = make([]schema.Change, 0, len(all))
+	)
+	if all, err = d.askForColumns(from, all, opts); err != nil {
+		return nil, err
+	}
+	for _, c := range all {
+		changes = opts.AddOrSkip(changes, c)
+	}
+	return changes, nil
 }
 
 // pkDiff returns the schema changes (if any) for migrating table
@@ -435,8 +445,8 @@ func (d *Diff) pkDiff(from, to *schema.Table, opts *schema.DiffOptions) (changes
 // indexes from current state to the desired state.
 func (d *Diff) indexDiffT(from, to *schema.Table, opts *schema.DiffOptions) ([]schema.Change, error) {
 	var (
-		changes []schema.Change
-		exists  = make(map[*schema.Index]bool)
+		all    []schema.Change
+		exists = make(map[*schema.Index]bool)
 	)
 	// Drop or modify indexes.
 	for _, idx1 := range from.Indexes {
@@ -444,7 +454,7 @@ func (d *Diff) indexDiffT(from, to *schema.Table, opts *schema.DiffOptions) ([]s
 		// Found directly.
 		if ok {
 			if change := d.indexChange(idx1, idx2); change != schema.NoChange {
-				changes = opts.AddOrSkip(changes, &schema.ModifyIndex{
+				all = append(all, &schema.ModifyIndex{
 					From:   idx1,
 					To:     idx2,
 					Change: change,
@@ -461,7 +471,7 @@ func (d *Diff) indexDiffT(from, to *schema.Table, opts *schema.DiffOptions) ([]s
 			}
 		}
 		// Not found.
-		changes = opts.AddOrSkip(changes, &schema.DropIndex{I: idx1})
+		all = append(all, &schema.DropIndex{I: idx1})
 	}
 	// Add indexes.
 	for _, idx := range to.Indexes {
@@ -469,10 +479,20 @@ func (d *Diff) indexDiffT(from, to *schema.Table, opts *schema.DiffOptions) ([]s
 			continue
 		}
 		if _, ok := from.Index(idx.Name); !ok {
-			changes = opts.AddOrSkip(changes, &schema.AddIndex{I: idx})
+			all = append(all, &schema.AddIndex{I: idx})
 		}
 	}
-	return d.askForIndexes(from.Name, changes, opts)
+	var (
+		err     error
+		changes = make([]schema.Change, 0, len(all))
+	)
+	if all, err = d.askForIndexes(from.Name, all, opts); err != nil {
+		return nil, err
+	}
+	for _, c := range all {
+		changes = opts.AddOrSkip(changes, c)
+	}
+	return changes, nil
 }
 
 // viewDefChanged checks if the view definition has changed.


Test patch:
diff --git a/schemahcl/schemahcl_test.go b/schemahcl/schemahcl_test.go
index ad00c17e..315547f3 100644
--- a/schemahcl/schemahcl_test.go
+++ b/schemahcl/schemahcl_test.go
@@ -1101,6 +1101,10 @@ mixed3 = ["string1", "string1"]
 func Test_WithPos(t *testing.T) {
 	var (
 		doc struct {
+			Bar struct {
+				A int `spec:"a"`
+				DefaultExtension
+			} `spec:"bar"`
 			DefaultExtension
 		}
 		b = []byte(`
@@ -1111,6 +1115,9 @@ foo {
   }
 }
 baz = 1
+bar {
+  a = 1
+}
 `)
 	)
 	require.NoError(t, New(WithPos()).EvalBytes(b, &doc, nil))
@@ -1123,6 +1130,7 @@ baz = 1
 	require.Equal(t, 3, rs[1].Range().Start.Line)
 	require.Equal(t, 4, rs[1].Children[0].Range().Start.Line)
 	require.Equal(t, 5, rs[1].Children[0].Attrs[0].Range().Start.Line)
+	require.NotNil(t, doc.Bar.Extra.Range(), "position should be attached to the resource")
 }
 
 func TestExtendedBlockDef(t *testing.T) {