Repository: ariga/atlas
Pull request #1264 (2022-11-06 13:42:35)

sql/postgres: add support for diffing operator classes


Patch:
diff --git a/doc/md/atlas-schema/sql.mdx b/doc/md/atlas-schema/sql.mdx
index b52819e7..28946d7e 100644
--- a/doc/md/atlas-schema/sql.mdx
+++ b/doc/md/atlas-schema/sql.mdx
@@ -409,6 +409,14 @@ index "idx_include" {
   columns = [column.range]
   include = [column.version]
 }
+
+index "idx_operator_class" {
+  type = GIN
+  on {
+    column = column.j
+    ops    = jsonb_path_ops
+  }
+}
 ```
 
 #### Properties
diff --git a/sql/internal/sqlx/diff.go b/sql/internal/sqlx/diff.go
index fa5c00ca..39f70a3d 100644
--- a/sql/internal/sqlx/diff.go
+++ b/sql/internal/sqlx/diff.go
@@ -42,12 +42,12 @@ type (
 		// For example, an index type or predicate (for partial indexes).
 		IndexAttrChanged(from, to []schema.Attr) bool
 
-		// IndexPartAttrChanged reports if the index-part attributes were
-		// changed. For example, an index-part collation.
-		IndexPartAttrChanged(from, to *schema.IndexPart) bool
+		// IndexPartAttrChanged reports if the part's attributes at position "i"
+		// were changed. For example, an index-part collation.
+		IndexPartAttrChanged(from, to *schema.Index, i int) bool
 
 		// IsGeneratedIndexName reports if the index name was generated by the database
-		// for unnamed INDEX or UNIQUE constraints. In such cases, the differ will look
+		// for unnamed INDEX or UNIQUE constraints. In such cases, the Differ will look
 		// for unnamed schema.Indexes on the desired state, before tagging the index as
 		// a candidate for deletion.
 		IsGeneratedIndexName(*schema.Table, *schema.Index) bool
@@ -280,12 +280,13 @@ func (d *Diff) indexChange(from, to *schema.Index) schema.ChangeKind {
 	if d.IndexAttrChanged(from.Attrs, to.Attrs) {
 		change |= schema.ChangeAttr
 	}
-	change |= d.partsChange(from.Parts, to.Parts)
+	change |= d.partsChange(from, to)
 	change |= CommentChange(from.Attrs, to.Attrs)
 	return change
 }
 
-func (d *Diff) partsChange(from, to []*schema.IndexPart) schema.ChangeKind {
+func (d *Diff) partsChange(fromI, toI *schema.Index) schema.ChangeKind {
+	from, to := fromI.Parts, toI.Parts
 	if len(from) != len(to) {
 		return schema.ChangeParts
 	}
@@ -293,7 +294,7 @@ func (d *Diff) partsChange(from, to []*schema.IndexPart) schema.ChangeKind {
 	sort.Slice(from, func(i, j int) bool { return from[i].SeqNo < from[j].SeqNo })
 	for i := range from {
 		switch {
-		case from[i].Desc != to[i].Desc || d.IndexPartAttrChanged(from[i], to[i]):
+		case from[i].Desc != to[i].Desc || d.IndexPartAttrChanged(fromI, toI, i):
 			return schema.ChangeParts
 		case from[i].C != nil && to[i].C != nil:
 			if from[i].C.Name != to[i].C.Name {
@@ -351,7 +352,7 @@ func (d *Diff) similarUnnamedIndex(t *schema.Table, idx1 *schema.Index) (*schema
 		if idx2.Name != "" || len(idx2.Parts) != len(idx1.Parts) || idx2.Unique != idx1.Unique {
 			continue
 		}
-		if d.partsChange(idx1.Parts, idx2.Parts) == schema.NoChange {
+		if d.partsChange(idx1, idx2) == schema.NoChange {
 			return idx2, true
 		}
 	}
diff --git a/sql/mysql/diff.go b/sql/mysql/diff.go
index 043eaf1a..758c23bd 100644
--- a/sql/mysql/diff.go
+++ b/sql/mysql/diff.go
@@ -162,9 +162,9 @@ func (*diff) IndexAttrChanged(from, to []schema.Attr) bool {
 }
 
 // IndexPartAttrChanged reports if the index-part attributes (collation or prefix) were changed.
-func (*diff) IndexPartAttrChanged(from, to *schema.IndexPart) bool {
+func (*diff) IndexPartAttrChanged(fromI, toI *schema.Index, i int) bool {
 	var s1, s2 SubPart
-	return sqlx.Has(from.Attrs, &s1) != sqlx.Has(to.Attrs, &s2) || s1.Len != s2.Len
+	return sqlx.Has(fromI.Parts[i].Attrs, &s1) != sqlx.Has(toI.Parts[i].Attrs, &s2) || s1.Len != s2.Len
 }
 
 // ReferenceChanged reports if the foreign key referential action was changed.
diff --git a/sql/postgres/diff.go b/sql/postgres/diff.go
index a1cf5fe9..8cf6fd0e 100644
--- a/sql/postgres/diff.go
+++ b/sql/postgres/diff.go
@@ -173,12 +173,30 @@ func (*diff) IndexAttrChanged(from, to []schema.Attr) bool {
 }
 
 // IndexPartAttrChanged reports if the index-part attributes were changed.
-func (*diff) IndexPartAttrChanged(from, to *schema.IndexPart) bool {
+func (*diff) IndexPartAttrChanged(fromI, toI *schema.Index, i int) bool {
+	from, to := fromI.Parts[i], toI.Parts[i]
 	p1 := &IndexColumnProperty{NullsFirst: from.Desc, NullsLast: !from.Desc}
 	sqlx.Has(from.Attrs, p1)
 	p2 := &IndexColumnProperty{NullsFirst: to.Desc, NullsLast: !to.Desc}
 	sqlx.Has(to.Attrs, p2)
-	return p1.NullsFirst != p2.NullsFirst || p1.NullsLast != p2.NullsLast
+	if p1.NullsFirst != p2.NullsFirst || p1.NullsLast != p2.NullsLast {
+		return true
+	}
+	var fromOp, toOp IndexOpClass
+	switch fromHas, toHas := sqlx.Has(from.Attrs, &fromOp), sqlx.Has(to.Attrs, &toOp); {
+	case fromHas && toHas:
+		return !fromOp.Equal(&toOp)
+	case toHas:
+		// Report a change if a non-default operator class was added.
+		d, err := toOp.DefaultFor(toI, toI.Parts[i])
+		return !d && err == nil
+	case fromHas:
+		// Report a change if a non-default operator class was removed.
+		d, err := fromOp.DefaultFor(fromI, fromI.Parts[i])
+		return !d && err == nil
+	default:
+		return false
+	}
 }
 
 // ReferenceChanged reports if the foreign key referential action was changed.
diff --git a/sql/postgres/inspect.go b/sql/postgres/inspect.go
index c4789178..8ece5326 100644
--- a/sql/postgres/inspect.go
+++ b/sql/postgres/inspect.go
@@ -904,13 +904,16 @@ var (
 
 // DefaultFor reports if the operator_class is the default for the index part.
 func (o *IndexOpClass) DefaultFor(idx *schema.Index, part *schema.IndexPart) (bool, error) {
-	if len(o.Params) > 0 {
-		return false, nil
+	// Explicitly defined as the default (Usually, it comes from the inspection).
+	if o.Default && len(o.Params) == 0 {
+		return true, nil
+	}
+	it := &IndexType{T: IndexTypeBTree}
+	if sqlx.Has(idx.Attrs, it) {
+		it.T = strings.ToUpper(it.T)
 	}
-	var it IndexType
-	// The type of the key need to be known in order to check
-	// if it is the default operator class.
-	if part.X != nil || !sqlx.Has(idx.Attrs, &it) || o.Default {
+	// The key type must be known to check if it is the default op_class.
+	if part.X != nil || len(o.Params) > 0 {
 		return false, nil
 	}
 	opsOnce.Do(func() {
@@ -936,7 +939,20 @@ func (o *IndexOpClass) DefaultFor(idx *schema.Index, part *schema.IndexPart) (bo
 			return false, fmt.Errorf("postgres: format operator-class type %T: %w", typ, err)
 		}
 	}
-	return defaultOps[postgresop.Class{Name: o.Name, Method: strings.ToUpper(it.T), Type: t}], nil
+	return defaultOps[postgresop.Class{Name: o.Name, Method: it.T, Type: t}], nil
+}
+
+// Equal reports whether o and x are the same operator class.
+func (o *IndexOpClass) Equal(x *IndexOpClass) bool {
+	if o.Name != x.Name || o.Default != x.Default || len(o.Params) != len(x.Params) {
+		return false
+	}
+	for i := range o.Params {
+		if o.Params[i].N != x.Params[i].N || o.Params[i].V != x.Params[i].V {
+			return false
+		}
+	}
+	return true
 }
 
 // String returns the string representation of the operator class.
diff --git a/sql/postgres/sqlspec.go b/sql/postgres/sqlspec.go
index 08a21847..0b27f0e6 100644
--- a/sql/postgres/sqlspec.go
+++ b/sql/postgres/sqlspec.go
@@ -112,7 +112,7 @@ func MarshalSpec(v any, marshaler schemahcl.Marshaler) ([]byte, error) {
 var (
 	hclState = schemahcl.New(
 		schemahcl.WithTypes(TypeRegistry.Specs()),
-		schemahcl.WithScopedEnums("table.index.type", IndexTypeBTree, IndexTypeBRIN, IndexTypeHash, IndexTypeGIN, IndexTypeGiST, IndexTypeSPGiST),
+		schemahcl.WithScopedEnums("table.index.type", IndexTypeBTree, IndexTypeBRIN, IndexTypeHash, IndexTypeGIN, IndexTypeGiST, "GiST", IndexTypeSPGiST, "SPGiST"),
 		schemahcl.WithScopedEnums("table.partition.type", PartitionTypeRange, PartitionTypeList, PartitionTypeHash),
 		schemahcl.WithScopedEnums("table.column.identity.generated", GeneratedTypeAlways, GeneratedTypeByDefault),
 		schemahcl.WithScopedEnums("table.column.as.type", "STORED"),
@@ -311,7 +311,7 @@ func convertIndex(spec *sqlspec.Index, t *schema.Table) (*schema.Index, error) {
 		if err != nil {
 			return nil, err
 		}
-		idx.Attrs = append(idx.Attrs, &IndexType{T: t})
+		idx.Attrs = append(idx.Attrs, &IndexType{T: strings.ToUpper(t)})
 	}
 	if attr, ok := spec.Attr("where"); ok {
 		p, err := attr.String()
@@ -560,9 +560,6 @@ func partAttr(idx *schema.Index, part *schema.IndexPart, spec *sqlspec.IndexPart
 	if !sqlx.Has(part.Attrs, &op) {
 		return nil
 	}
-	if d, err := op.DefaultFor(idx, part); err != nil || d {
-		return err
-	}
 	switch d, err := op.DefaultFor(idx, part); {
 	case err != nil:
 		return err
diff --git a/sql/schema/dsl.go b/sql/schema/dsl.go
index c534ac33..4d937f6f 100644
--- a/sql/schema/dsl.go
+++ b/sql/schema/dsl.go
@@ -652,10 +652,10 @@ func (i *Index) AddParts(parts ...*IndexPart) *Index {
 func NewIndexPart() *IndexPart { return &IndexPart{} }
 
 // NewColumnPart creates a new index part with the given column.
-func NewColumnPart(c *Column) *IndexPart { return &IndexPart{C: c} }
+func NewColumnPart(c *Column) *IndexPart { return NewIndexPart().SetColumn(c) }
 
 // NewExprPart creates a new index part with the given expression.
-func NewExprPart(x Expr) *IndexPart { return &IndexPart{X: x} }
+func NewExprPart(x Expr) *IndexPart { return NewIndexPart().SetExpr(x) }
 
 // SetDesc configures the "DESC" attribute of the key part.
 func (p *IndexPart) SetDesc(b bool) *IndexPart {
diff --git a/sql/sqlite/diff.go b/sql/sqlite/diff.go
index df377ac8..17072064 100644
--- a/sql/sqlite/diff.go
+++ b/sql/sqlite/diff.go
@@ -113,7 +113,7 @@ func (*Diff) IndexAttrChanged(from, to []schema.Attr) bool {
 }
 
 // IndexPartAttrChanged reports if the index-part attributes were changed.
-func (*Diff) IndexPartAttrChanged(_, _ *schema.IndexPart) bool {
+func (*Diff) IndexPartAttrChanged(_, _ *schema.Index, _ int) bool {
 	return false
 }
 


Test patch:
diff --git a/internal/integration/testdata/postgres/index-operator-class.txt b/internal/integration/testdata/postgres/index-operator-class.txt
new file mode 100644
index 00000000..6dbb17b2
--- /dev/null
+++ b/internal/integration/testdata/postgres/index-operator-class.txt
@@ -0,0 +1,197 @@
+# Run this test only on 14 because tsvector_ops
+# parameters are not supported in old versions.
+only postgres14
+
+apply 1.hcl
+cmpshow pets 1.sql
+
+apply 2.hcl
+cmpshow pets 2.sql
+cmphcl 2.inspect.hcl
+
+apply 3.hcl
+cmpshow logs 3.sql
+
+apply 4.hcl
+cmpshow tsv 4.sql
+
+apply 5.hcl
+cmpshow tsv 5.sql
+
+-- 1.hcl --
+schema "script_index_operator_class" {}
+
+table "pets" {
+  schema = schema.$db
+  column "name" {
+    null = false
+    type = char(10)
+  }
+  index "name_idx1" {
+    on {
+      column = column.name
+      ops    = bpchar_ops
+    }
+  }
+  index "name_idx2" {
+    on {
+      column = column.name
+      // Set a non-default operator class.
+      ops    = bpchar_pattern_ops
+    }
+  }
+}
+
+-- 1.sql --
+        Table "script_index_operator_class.pets"
+ Column |     Type      | Collation | Nullable | Default
+--------+---------------+-----------+----------+---------
+ name   | character(10) |           | not null |
+Indexes:
+    "name_idx1" btree (name)
+    "name_idx2" btree (name bpchar_pattern_ops)
+
+
+-- 2.hcl --
+schema "script_index_operator_class" {}
+
+table "pets" {
+  schema = schema.$db
+  column "name" {
+    null = false
+    type = char(10)
+  }
+  // Flip operator classes.
+  index "name_idx1" {
+    on {
+      column = column.name
+      ops    = bpchar_pattern_ops
+    }
+  }
+  index "name_idx2" {
+    on {
+      column = column.name
+      ops    = bpchar_ops
+    }
+  }
+}
+
+-- 2.sql --
+        Table "script_index_operator_class.pets"
+ Column |     Type      | Collation | Nullable | Default
+--------+---------------+-----------+----------+---------
+ name   | character(10) |           | not null |
+Indexes:
+    "name_idx1" btree (name bpchar_pattern_ops)
+    "name_idx2" btree (name)
+
+-- 2.inspect.hcl --
+table "pets" {
+  schema = schema.script_index_operator_class
+  column "name" {
+    null = false
+    type = character(10)
+  }
+  index "name_idx1" {
+    type = BTREE
+    on {
+      column = column.name
+      ops    = bpchar_pattern_ops
+    }
+  }
+  index "name_idx2" {
+    columns = [column.name]
+    type    = BTREE
+  }
+}
+schema "script_index_operator_class" {
+}
+
+-- 3.hcl --
+schema "script_index_operator_class" {}
+
+table "logs" {
+  schema = schema.$db
+  column "j" {
+    null = false
+    type = jsonb
+  }
+  index "j_idx" {
+    type = GIN
+    on {
+      column = column.j
+      ops    = jsonb_path_ops
+    }
+  }
+}
+
+-- 3.sql --
+  Table "script_index_operator_class.logs"
+ Column | Type  | Collation | Nullable | Default
+--------+-------+-----------+----------+---------
+ j      | jsonb |           | not null |
+Indexes:
+    "j_idx" gin (j jsonb_path_ops)
+
+-- 4.hcl --
+schema "script_index_operator_class" {}
+
+table "tsv" {
+  schema = schema.$db
+  column "t" {
+    null = false
+    type = text
+  }
+  column "a" {
+    null = false
+    type = tsvector
+  }
+  index "a_idx" {
+    type = GiST
+    on {
+      column = column.a
+      ops    = sql("tsvector_ops(siglen=8)")
+    }
+  }
+}
+
+-- 4.sql --
+      Table "script_index_operator_class.tsv"
+ Column |   Type   | Collation | Nullable | Default
+--------+----------+-----------+----------+---------
+ t      | text     |           | not null |
+ a      | tsvector |           | not null |
+Indexes:
+    "a_idx" gist (a tsvector_ops (siglen='8'))
+
+
+-- 5.hcl --
+schema "script_index_operator_class" {}
+
+table "tsv" {
+  schema = schema.$db
+  column "t" {
+    null = false
+    type = text
+  }
+  column "a" {
+    null = false
+    type = tsvector
+  }
+  index "a_idx" {
+    type = GiST
+    on {
+      column = column.a
+      ops    = sql("tsvector_ops(siglen=9)")
+    }
+  }
+}
+
+-- 5.sql --
+      Table "script_index_operator_class.tsv"
+ Column |   Type   | Collation | Nullable | Default
+--------+----------+-----------+----------+---------
+ t      | text     |           | not null |
+ a      | tsvector |           | not null |
+Indexes:
+    "a_idx" gist (a tsvector_ops (siglen='9'))
\ No newline at end of file
diff --git a/sql/postgres/diff_test.go b/sql/postgres/diff_test.go
index e687a9e3..73424613 100644
--- a/sql/postgres/diff_test.go
+++ b/sql/postgres/diff_test.go
@@ -375,6 +375,43 @@ func TestDiff_TableDiff(t *testing.T) {
 				},
 			}
 		}(),
+		func() testcase {
+			var (
+				from = schema.NewTable("t1").
+					SetSchema(schema.New("public")).
+					AddColumns(schema.NewIntColumn("c1", "int8"))
+				to = schema.NewTable("t1").
+					SetSchema(schema.New("public")).
+					AddColumns(schema.NewIntColumn("c1", "int8"))
+			)
+			from.Indexes = []*schema.Index{
+				schema.NewIndex("idx1").AddParts(schema.NewColumnPart(from.Columns[0])),
+				schema.NewIndex("idx2").AddParts(schema.NewColumnPart(from.Columns[0])),
+				schema.NewIndex("idx3").AddParts(schema.NewColumnPart(from.Columns[0])),
+				schema.NewIndex("idx4").AddParts(schema.NewColumnPart(to.Columns[0]).AddAttrs(&IndexOpClass{Name: "int8_ops"})),
+				schema.NewIndex("idx5").AddParts(schema.NewColumnPart(to.Columns[0]).AddAttrs(&IndexOpClass{Name: "int8_ops"})),
+			}
+			to.Indexes = []*schema.Index{
+				// A default operator class was added.
+				schema.NewIndex("idx1").AddParts(schema.NewColumnPart(to.Columns[0]).AddAttrs(&IndexOpClass{Name: "int8_ops"})),
+				// Unrecognized operator class with explicit default.
+				schema.NewIndex("idx2").AddParts(schema.NewColumnPart(to.Columns[0]).AddAttrs(&IndexOpClass{Name: "int8_custom", Default: true})),
+				// A default operator class but with custom parameters.
+				schema.NewIndex("idx3").AddParts(schema.NewColumnPart(to.Columns[0]).AddAttrs(&IndexOpClass{Name: "int8_ops", Params: []struct{ N, V string }{{"signlen", "1"}}})),
+				// A default operator class was dropped.
+				schema.NewIndex("idx4").AddParts(schema.NewColumnPart(from.Columns[0])),
+				// Equal operators.
+				schema.NewIndex("idx5").AddParts(schema.NewColumnPart(to.Columns[0]).AddAttrs(&IndexOpClass{Name: "int8_ops"})),
+			}
+			return testcase{
+				name: "operator class",
+				from: from,
+				to:   to,
+				wantChanges: []schema.Change{
+					&schema.ModifyIndex{From: from.Indexes[2], To: to.Indexes[2], Change: schema.ChangeParts},
+				},
+			}
+		}(),
 		func() testcase {
 			var (
 				ref = &schema.Table{
diff --git a/sql/postgres/sqlspec_test.go b/sql/postgres/sqlspec_test.go
index 71723bb0..59a930e5 100644
--- a/sql/postgres/sqlspec_test.go
+++ b/sql/postgres/sqlspec_test.go
@@ -284,12 +284,17 @@ table "t" {
 	t.Run("Valid", func(t *testing.T) {
 		var (
 			s schema.Schema
-			f = fmt.Sprintf(f, "HASH")
+			r = fmt.Sprintf(f, "HASH")
 		)
-		err := EvalHCLBytes([]byte(f), &s, nil)
-		require.NoError(t, err)
+		require.NoError(t, EvalHCLBytes([]byte(r), &s, nil))
 		idx := s.Tables[0].Indexes[0]
 		require.Equal(t, IndexTypeHash, idx.Attrs[0].(*IndexType).T)
+
+		s = schema.Schema{}
+		r = fmt.Sprintf(f, "GiST")
+		require.NoError(t, EvalHCLBytes([]byte(r), &s, nil))
+		idx = s.Tables[0].Indexes[0]
+		require.Equal(t, IndexTypeGiST, idx.Attrs[0].(*IndexType).T)
 	})
 }
 
@@ -732,7 +737,7 @@ func TestMarshalSpec_IndexOpClass(t *testing.T) {
 			Table:  s.Tables[0],
 			Unique: true,
 			Parts: []*schema.IndexPart{
-				{SeqNo: 0, C: s.Tables[0].Columns[1], Attrs: []schema.Attr{&IndexOpClass{Name: "tsvector_ops", Default: true, Params: []struct{ N, V string }{{"siglen", "1"}}}}},
+				{SeqNo: 0, C: s.Tables[0].Columns[1], Attrs: []schema.Attr{&IndexOpClass{Name: "tsvector_ops", Params: []struct{ N, V string }{{"siglen", "1"}}}}},
 			},
 			Attrs: []schema.Attr{
 				&IndexType{T: IndexTypeGiST},