Use available method to compare tables (vitessio#11056) (vitessio#984)

* Use available method to compare tables We already have a method that compares while ignoring the name, so use that here too. This avoids a copy and custom logic here. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * Embed pointer instead of direct object We always have this already heap allocated so we don't save anything with storing the embedded struct here. In fact, we generate more copies this way because we have to copy the heap object this way. The cleanup of pointer references and deferences also shows this better matches how we use this. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
planetscale · Aug 21, 2022 · 39efe82 · 39efe82
1 parent bbc12c7
commit 39efe82
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 38 deletions.
diff --git a/go/vt/schemadiff/schema.go b/go/vt/schemadiff/schema.go
@@ -212,7 +212,7 @@ func (s *Schema) normalize() error {
 				continue
 			}
 			// Not handled. Is this view dependent on already handled objects?
-			dependentNames, err := getViewDependentTableNames(&v.CreateView)
+			dependentNames, err := getViewDependentTableNames(v.CreateView)
 			if err != nil {
 				return err
 			}
@@ -497,15 +497,15 @@ func (s *Schema) apply(diffs []EntityDiff) error {
 			if _, ok := s.named[name]; ok {
 				return &ApplyDuplicateEntityError{Entity: name}
 			}
-			s.tables = append(s.tables, &CreateTableEntity{CreateTable: *diff.createTable})
+			s.tables = append(s.tables, &CreateTableEntity{CreateTable: diff.createTable})
 			_, s.named[name] = diff.Entities()
 		case *CreateViewEntityDiff:
 			// We expect the view to not exist
 			name := diff.createView.ViewName.Name.String()
 			if _, ok := s.named[name]; ok {
 				return &ApplyDuplicateEntityError{Entity: name}
 			}
-			s.views = append(s.views, &CreateViewEntity{CreateView: *diff.createView})
+			s.views = append(s.views, &CreateViewEntity{CreateView: diff.createView})
 			_, s.named[name] = diff.Entities()
 		case *DropTableEntityDiff:
 			// We expect the table to exist

diff --git a/go/vt/schemadiff/table.go b/go/vt/schemadiff/table.go
@@ -121,7 +121,7 @@ func (d *CreateTableEntityDiff) IsEmpty() bool {
 
 // Entities implements EntityDiff
 func (d *CreateTableEntityDiff) Entities() (from Entity, to Entity) {
-	return nil, &CreateTableEntity{CreateTable: *d.createTable}
+	return nil, &CreateTableEntity{CreateTable: d.createTable}
 }
 
 // Statement implements EntityDiff
@@ -282,14 +282,14 @@ func (d *RenameTableEntityDiff) SetSubsequentDiff(EntityDiff) {
 
 // CreateTableEntity stands for a TABLE construct. It contains the table's CREATE statement.
 type CreateTableEntity struct {
-	sqlparser.CreateTable
+	*sqlparser.CreateTable
 }
 
 func NewCreateTableEntity(c *sqlparser.CreateTable) (*CreateTableEntity, error) {
 	if !c.IsFullyParsed() {
 		return nil, &NotFullyParsedError{Entity: c.Table.Name.String(), Statement: sqlparser.CanonicalString(c)}
 	}
-	entity := &CreateTableEntity{CreateTable: *c}
+	entity := &CreateTableEntity{CreateTable: c}
 	entity.normalize()
 	return entity, nil
 }
@@ -329,7 +329,7 @@ func (c *CreateTableEntity) normalizeTableOptions() {
 }
 
 func (c *CreateTableEntity) Clone() Entity {
-	return &CreateTableEntity{CreateTable: *sqlparser.CloneRefOfCreateTable(&c.CreateTable)}
+	return &CreateTableEntity{CreateTable: sqlparser.CloneRefOfCreateTable(c.CreateTable)}
 }
 
 // Right now we assume MySQL 8.0 for the collation normalization handling.
@@ -618,22 +618,19 @@ func (c *CreateTableEntity) Diff(other Entity, hints *DiffHints) (EntityDiff, er
 // It returns an AlterTable statement if changes are found, or nil if not.
 // the other table may be of different name; its name is ignored.
 func (c *CreateTableEntity) TableDiff(other *CreateTableEntity, hints *DiffHints) (*AlterTableEntityDiff, error) {
-	otherStmt := other.CreateTable
-	otherStmt.Table = c.CreateTable.Table
-
 	if !c.CreateTable.IsFullyParsed() {
-		return nil, &NotFullyParsedError{Entity: c.Name(), Statement: sqlparser.CanonicalString(&c.CreateTable)}
+		return nil, &NotFullyParsedError{Entity: c.Name(), Statement: sqlparser.CanonicalString(c.CreateTable)}
 	}
-	if !otherStmt.IsFullyParsed() {
-		return nil, &NotFullyParsedError{Entity: other.Name(), Statement: sqlparser.CanonicalString(&otherStmt)}
+	if !other.CreateTable.IsFullyParsed() {
+		return nil, &NotFullyParsedError{Entity: other.Name(), Statement: sqlparser.CanonicalString(other.CreateTable)}
 	}
 
-	if sqlparser.EqualsRefOfCreateTable(&c.CreateTable, &otherStmt) {
+	if c.identicalOtherThanName(other) {
 		return nil, nil
 	}
 
 	alterTable := &sqlparser.AlterTable{
-		Table: otherStmt.Table,
+		Table: c.CreateTable.Table,
 	}
 	diffedTableCharset := ""
 	var parentAlterTableEntityDiff *AlterTableEntityDiff
@@ -691,7 +688,7 @@ func (c *CreateTableEntity) TableDiff(other *CreateTableEntity, hints *DiffHints
 	}
 	for _, superfluousFulltextKey := range superfluousFulltextKeys {
 		alterTable := &sqlparser.AlterTable{
-			Table:        otherStmt.Table,
+			Table:        c.CreateTable.Table,
 			AlterOptions: []sqlparser.AlterOption{superfluousFulltextKey},
 		}
 		diff := &AlterTableEntityDiff{alterTable: alterTable, from: c, to: other}
@@ -701,7 +698,7 @@ func (c *CreateTableEntity) TableDiff(other *CreateTableEntity, hints *DiffHints
 	}
 	for _, partitionSpec := range partitionSpecs {
 		alterTable := &sqlparser.AlterTable{
-			Table:         otherStmt.Table,
+			Table:         c.CreateTable.Table,
 			PartitionSpec: partitionSpec,
 		}
 		diff := &AlterTableEntityDiff{alterTable: alterTable, from: c, to: other}
@@ -1480,7 +1477,7 @@ func heuristicallyDetectColumnRenames(
 
 // Create implements Entity interface
 func (c *CreateTableEntity) Create() EntityDiff {
-	return &CreateTableEntityDiff{to: c, createTable: &c.CreateTable}
+	return &CreateTableEntityDiff{to: c, createTable: c.CreateTable}
 }
 
 // Drop implements Entity interface

diff --git a/go/vt/schemadiff/view.go b/go/vt/schemadiff/view.go
@@ -91,7 +91,7 @@ func (d *CreateViewEntityDiff) IsEmpty() bool {
 
 // Entities implements EntityDiff
 func (d *CreateViewEntityDiff) Entities() (from Entity, to Entity) {
-	return nil, &CreateViewEntity{CreateView: *d.createView}
+	return nil, &CreateViewEntity{CreateView: d.createView}
 }
 
 // Statement implements EntityDiff
@@ -193,14 +193,14 @@ func (d *DropViewEntityDiff) SetSubsequentDiff(EntityDiff) {
 
 // CreateViewEntity stands for a VIEW construct. It contains the view's CREATE statement.
 type CreateViewEntity struct {
-	sqlparser.CreateView
+	*sqlparser.CreateView
 }
 
 func NewCreateViewEntity(c *sqlparser.CreateView) (*CreateViewEntity, error) {
 	if !c.IsFullyParsed() {
 		return nil, &NotFullyParsedError{Entity: c.ViewName.Name.String(), Statement: sqlparser.CanonicalString(c)}
 	}
-	entity := &CreateViewEntity{CreateView: *c}
+	entity := &CreateViewEntity{CreateView: c}
 	entity.normalize()
 	return entity, nil
 }
@@ -235,35 +235,32 @@ func (c *CreateViewEntity) Diff(other Entity, hints *DiffHints) (EntityDiff, err
 // It returns an AlterView statement if changes are found, or nil if not.
 // the other view may be of different name; its name is ignored.
 func (c *CreateViewEntity) ViewDiff(other *CreateViewEntity, _ *DiffHints) (*AlterViewEntityDiff, error) {
-	otherStmt := other.CreateView
-	otherStmt.ViewName = c.CreateView.ViewName
-
-	if !c.CreateView.IsFullyParsed() {
-		return nil, &NotFullyParsedError{Entity: c.Name(), Statement: sqlparser.CanonicalString(&c.CreateView)}
+	if !c.IsFullyParsed() {
+		return nil, &NotFullyParsedError{Entity: c.Name(), Statement: sqlparser.CanonicalString(c.CreateView)}
 	}
-	if !otherStmt.IsFullyParsed() {
-		return nil, &NotFullyParsedError{Entity: c.Name(), Statement: sqlparser.CanonicalString(&otherStmt)}
+	if !other.CreateView.IsFullyParsed() {
+		return nil, &NotFullyParsedError{Entity: c.Name(), Statement: sqlparser.CanonicalString(other.CreateView)}
 	}
 
-	if sqlparser.EqualsRefOfCreateView(&c.CreateView, &otherStmt) {
+	if c.identicalOtherThanName(other) {
 		return nil, nil
 	}
 
 	alterView := &sqlparser.AlterView{
-		ViewName:    otherStmt.ViewName,
-		Algorithm:   otherStmt.Algorithm,
-		Definer:     otherStmt.Definer,
-		Security:    otherStmt.Security,
-		Columns:     otherStmt.Columns,
-		Select:      otherStmt.Select,
-		CheckOption: otherStmt.CheckOption,
+		ViewName:    c.CreateView.ViewName,
+		Algorithm:   other.CreateView.Algorithm,
+		Definer:     other.CreateView.Definer,
+		Security:    other.CreateView.Security,
+		Columns:     other.CreateView.Columns,
+		Select:      other.CreateView.Select,
+		CheckOption: other.CreateView.CheckOption,
 	}
 	return &AlterViewEntityDiff{alterView: alterView, from: c, to: other}, nil
 }
 
 // Create implements Entity interface
 func (c *CreateViewEntity) Create() EntityDiff {
-	return &CreateViewEntityDiff{createView: &c.CreateView}
+	return &CreateViewEntityDiff{createView: c.CreateView}
 }
 
 // Drop implements Entity interface
@@ -297,5 +294,19 @@ func (c *CreateViewEntity) Apply(diff EntityDiff) (Entity, error) {
 }
 
 func (c *CreateViewEntity) Clone() Entity {
-	return &CreateViewEntity{CreateView: *sqlparser.CloneRefOfCreateView(&c.CreateView)}
+	return &CreateViewEntity{CreateView: sqlparser.CloneRefOfCreateView(c.CreateView)}
+}
+
+func (c *CreateViewEntity) identicalOtherThanName(other *CreateViewEntity) bool {
+	if other == nil {
+		return false
+	}
+	return c.Algorithm == other.Algorithm &&
+		c.Security == other.Security &&
+		c.CheckOption == other.CheckOption &&
+		c.IsReplace == other.IsReplace &&
+		sqlparser.EqualsRefOfDefiner(c.Definer, other.Definer) &&
+		sqlparser.EqualsColumns(c.Columns, other.Columns) &&
+		sqlparser.EqualsSelectStatement(c.Select, other.Select) &&
+		sqlparser.EqualsRefOfParsedComments(c.Comments, other.Comments)
 }