From cccf9611bf3ffc71a634564868662fcef87a980f Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Sun, 25 Feb 2024 20:10:46 +0200 Subject: [PATCH 01/22] Annotating textual diffs Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/annotations.go | 137 ++++++++++++++++++++++++++++++++ go/vt/schemadiff/table.go | 86 +++++++++++++++----- go/vt/schemadiff/types.go | 28 +++++++ 3 files changed, 232 insertions(+), 19 deletions(-) create mode 100644 go/vt/schemadiff/annotations.go diff --git a/go/vt/schemadiff/annotations.go b/go/vt/schemadiff/annotations.go new file mode 100644 index 00000000000..0783c1f873d --- /dev/null +++ b/go/vt/schemadiff/annotations.go @@ -0,0 +1,137 @@ +/* +Copyright 2024 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package schemadiff + +import ( + "strings" +) + +var ( + annotationAddedHint = " -- schemadiff:added" + annotationRemovedHint = " -- schemadiff:removed" +) + +func annotatedStatement(stmt string, annotationType TextualAnnotationType, annotationHint TextualAnnotationHint, annotations []string) string { + anyLineAnnotated := false + stmtLines := strings.Split(stmt, "\n") + for i := range stmtLines { + lineAnnotated := false + trimmedLine := stmtLines[i] + trimmedLine = strings.TrimSpace(trimmedLine) + trimmedLine = strings.TrimRight(trimmedLine, ",") + trimmedLine = strings.TrimLeft(trimmedLine, "(") + trimmedLine = strings.TrimLeft(trimmedLine, ") ") + trimmedLine = strings.TrimSpace(trimmedLine) + if trimmedLine == "" { + continue + } + for _, annotation := range annotations { + annotation = strings.TrimSpace(annotation) + annotationLines := strings.Split(annotation, "\n") + for _, annTrimmedLine := range annotationLines { + if lineAnnotated { + break + } + + annTrimmedLine = strings.TrimSpace(annTrimmedLine) + possibleMutations := map[string]bool{ + trimmedLine: true, + ") " + trimmedLine: true, + "(" + trimmedLine: true, + "(" + trimmedLine + ",": true, + trimmedLine + ",": true, + trimmedLine + ")": true, + } + if possibleMutations[annTrimmedLine] { + // Annotate this line! + switch annotationHint { + case PlusMinusSpaceTextualAnnotationHint, + PlusMinusEqualTextualAnnotationHint, + PlusMinusTextualAnnotationHint: + switch annotationType { + case AddedTextualAnnotationType: + stmtLines[i] = "+" + stmtLines[i] + case RemovedTextualAnnotationType: + stmtLines[i] = "-" + stmtLines[i] + } + case SchemadiffSuffixTextualAnnotationHint: + switch annotationType { + case AddedTextualAnnotationType: + stmtLines[i] = stmtLines[i] + annotationAddedHint + case RemovedTextualAnnotationType: + stmtLines[i] = stmtLines[i] + annotationRemovedHint + } + } + lineAnnotated = true + anyLineAnnotated = true + } + } + } + if !lineAnnotated { + switch annotationHint { + case PlusMinusSpaceTextualAnnotationHint: + stmtLines[i] = " " + stmtLines[i] + case PlusMinusEqualTextualAnnotationHint: + stmtLines[i] = "=" + stmtLines[i] + case PlusMinusTextualAnnotationHint: + // line unchanged + case SchemadiffSuffixTextualAnnotationHint: + // line unchanged + } + } + } + if !anyLineAnnotated { + return stmt + } + return strings.Join(stmtLines, "\n") +} + +func unifiedAnnotated(annotatedFrom string, annotatedTo string) string { + fromLines := strings.Split(annotatedFrom, "\n") + toLines := strings.Split(annotatedTo, "\n") + unified := []string{} + + fromIndex := 0 + toIndex := 0 + for fromIndex < len(fromLines) || toIndex < len(toLines) { + matchingLine := "" + if fromIndex < len(fromLines) { + fromLine := fromLines[fromIndex] + if strings.HasSuffix(fromLine, annotationRemovedHint) { + unified = append(unified, "- "+fromLine) + fromIndex++ + continue + } + matchingLine = fromLine + } + if toIndex < len(toLines) { + toLine := toLines[toIndex] + if strings.HasSuffix(toLine, annotationAddedHint) { + unified = append(unified, "+ "+toLine) + toIndex++ + continue + } + if matchingLine == "" { + matchingLine = toLine + } + } + unified = append(unified, matchingLine) + fromIndex++ + toIndex++ + } + return strings.Join(unified, "\n") +} diff --git a/go/vt/schemadiff/table.go b/go/vt/schemadiff/table.go index aab697c2bf0..87faa5fb455 100644 --- a/go/vt/schemadiff/table.go +++ b/go/vt/schemadiff/table.go @@ -35,9 +35,10 @@ type charsetCollate struct { } type AlterTableEntityDiff struct { - from *CreateTableEntity - to *CreateTableEntity - alterTable *sqlparser.AlterTable + from *CreateTableEntity + to *CreateTableEntity + alterTable *sqlparser.AlterTable + annotations *TextualAnnotations canonicalStatementString string subsequentDiff *AlterTableEntityDiff @@ -59,6 +60,15 @@ func (d *AlterTableEntityDiff) Entities() (from Entity, to Entity) { return d.from, d.to } +func (d *AlterTableEntityDiff) Annotated() (from string, to string, unified string) { + annotationHint := SchemadiffSuffixTextualAnnotationHint + fromStatementString := d.from.Create().CanonicalStatementString() + from = annotatedStatement(fromStatementString, RemovedTextualAnnotationType, annotationHint, d.annotations.removed) + toStatementString := d.to.Create().CanonicalStatementString() + to = annotatedStatement(toStatementString, AddedTextualAnnotationType, annotationHint, d.annotations.added) + return from, to, unifiedAnnotated(from, to) +} + // Statement implements EntityDiff func (d *AlterTableEntityDiff) Statement() sqlparser.Statement { if d == nil { @@ -837,13 +847,14 @@ func (c *CreateTableEntity) TableDiff(other *CreateTableEntity, hints *DiffHints var parentAlterTableEntityDiff *AlterTableEntityDiff var partitionSpecs []*sqlparser.PartitionSpec var superfluousFulltextKeys []*sqlparser.AddIndexDefinition + annotations := NewTextualAnnotations() { // diff columns // ordered columns for both tables: t1Columns := c.CreateTable.TableSpec.Columns t2Columns := other.CreateTable.TableSpec.Columns - if err := c.diffColumns(alterTable, t1Columns, t2Columns, hints, t1cc, t2cc); err != nil { + if err := c.diffColumns(alterTable, annotations, t1Columns, t2Columns, hints, t1cc, t2cc); err != nil { return nil, err } } @@ -852,14 +863,14 @@ func (c *CreateTableEntity) TableDiff(other *CreateTableEntity, hints *DiffHints // ordered keys for both tables: t1Keys := c.CreateTable.TableSpec.Indexes t2Keys := other.CreateTable.TableSpec.Indexes - superfluousFulltextKeys = c.diffKeys(alterTable, t1Keys, t2Keys, hints) + superfluousFulltextKeys = c.diffKeys(alterTable, annotations, t1Keys, t2Keys, hints) } { // diff constraints // ordered constraints for both tables: t1Constraints := c.CreateTable.TableSpec.Constraints t2Constraints := other.CreateTable.TableSpec.Constraints - c.diffConstraints(alterTable, c.Name(), t1Constraints, other.Name(), t2Constraints, hints) + c.diffConstraints(alterTable, annotations, c.Name(), t1Constraints, other.Name(), t2Constraints, hints) } { // diff partitions @@ -867,7 +878,7 @@ func (c *CreateTableEntity) TableDiff(other *CreateTableEntity, hints *DiffHints t1Partitions := c.CreateTable.TableSpec.PartitionOption t2Partitions := other.CreateTable.TableSpec.PartitionOption var err error - partitionSpecs, err = c.diffPartitions(alterTable, t1Partitions, t2Partitions, hints) + partitionSpecs, err = c.diffPartitions(alterTable, annotations, t1Partitions, t2Partitions, hints) if err != nil { return nil, err } @@ -877,14 +888,14 @@ func (c *CreateTableEntity) TableDiff(other *CreateTableEntity, hints *DiffHints // ordered keys for both tables: t1Options := c.CreateTable.TableSpec.Options t2Options := other.CreateTable.TableSpec.Options - if err := c.diffOptions(alterTable, t1Options, t2Options, hints); err != nil { + if err := c.diffOptions(alterTable, annotations, t1Options, t2Options, hints); err != nil { return nil, err } } tableSpecHasChanged := len(alterTable.AlterOptions) > 0 || alterTable.PartitionOption != nil || alterTable.PartitionSpec != nil newAlterTableEntityDiff := func(alterTable *sqlparser.AlterTable) *AlterTableEntityDiff { - d := &AlterTableEntityDiff{alterTable: alterTable, from: c, to: other} + d := &AlterTableEntityDiff{alterTable: alterTable, from: c, to: other, annotations: annotations} var algorithmValue sqlparser.AlgorithmValue @@ -986,6 +997,7 @@ func isDefaultTableOptionValue(option *sqlparser.TableOption) bool { } func (c *CreateTableEntity) diffOptions(alterTable *sqlparser.AlterTable, + annotations *TextualAnnotations, t1Options sqlparser.TableOptions, t2Options sqlparser.TableOptions, hints *DiffHints, @@ -1071,11 +1083,16 @@ func (c *CreateTableEntity) diffOptions(alterTable *sqlparser.AlterTable, if tableOption != nil { tableOption.Name = t1Option.Name alterTableOptions = append(alterTableOptions, tableOption) + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(sqlparser.TableOptions{t1Option})) } } - } // changed options + modifyTableOption := func(option1, option2 *sqlparser.TableOption) { + alterTableOptions = append(alterTableOptions, option2) + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(sqlparser.TableOptions{option1})) + annotations.added = append(annotations.added, sqlparser.CanonicalString(sqlparser.TableOptions{option2})) + } for _, t2Option := range t2Options { if t1Option, ok := t1OptionsMap[t2Option.Name]; ok { options1 := sqlparser.TableOptions{t1Option} @@ -1087,10 +1104,10 @@ func (c *CreateTableEntity) diffOptions(alterTable *sqlparser.AlterTable, case "CHARSET", "COLLATE": switch hints.TableCharsetCollateStrategy { case TableCharsetCollateStrict: - alterTableOptions = append(alterTableOptions, t2Option) + modifyTableOption(t1Option, t2Option) case TableCharsetCollateIgnoreEmpty: if t1Option.String != "" && t2Option.String != "" { - alterTableOptions = append(alterTableOptions, t2Option) + modifyTableOption(t1Option, t2Option) } // if one is empty, we ignore case TableCharsetCollateIgnoreAlways: @@ -1099,7 +1116,7 @@ func (c *CreateTableEntity) diffOptions(alterTable *sqlparser.AlterTable, case "AUTO_INCREMENT": switch hints.AutoIncrementStrategy { case AutoIncrementApplyAlways: - alterTableOptions = append(alterTableOptions, t2Option) + modifyTableOption(t1Option, t2Option) case AutoIncrementApplyHigher: option1AutoIncrement, err := strconv.ParseInt(t1Option.Value.Val, 10, 64) if err != nil { @@ -1111,18 +1128,22 @@ func (c *CreateTableEntity) diffOptions(alterTable *sqlparser.AlterTable, } if option2AutoIncrement > option1AutoIncrement { // never decrease AUTO_INCREMENT. Only increase - alterTableOptions = append(alterTableOptions, t2Option) + modifyTableOption(t1Option, t2Option) } case AutoIncrementIgnore: // do not apply } default: // Apply the new options - alterTableOptions = append(alterTableOptions, t2Option) + modifyTableOption(t1Option, t2Option) } } } } + addTableOption := func(option *sqlparser.TableOption) { + alterTableOptions = append(alterTableOptions, option) + annotations.added = append(annotations.added, sqlparser.CanonicalString(sqlparser.TableOptions{option})) + } // added options for _, t2Option := range t2Options { if _, ok := t1OptionsMap[t2Option.Name]; !ok { @@ -1130,18 +1151,18 @@ func (c *CreateTableEntity) diffOptions(alterTable *sqlparser.AlterTable, case "CHARSET", "COLLATE": switch hints.TableCharsetCollateStrategy { case TableCharsetCollateStrict: - alterTableOptions = append(alterTableOptions, t2Option) + addTableOption(t2Option) // in all other strategies we ignore the charset } case "AUTO_INCREMENT": switch hints.AutoIncrementStrategy { case AutoIncrementApplyAlways, AutoIncrementApplyHigher: - alterTableOptions = append(alterTableOptions, t2Option) + addTableOption(t2Option) case AutoIncrementIgnore: // do not apply } default: - alterTableOptions = append(alterTableOptions, t2Option) + addTableOption(t2Option) } } } @@ -1158,6 +1179,7 @@ func (c *CreateTableEntity) diffOptions(alterTable *sqlparser.AlterTable, // - table1 may have non-empty list of partitions _preceding_ this sequence, and table2 may not // - table2 may have non-empty list of partitions _following_ this sequence, and table1 may not func (c *CreateTableEntity) isRangePartitionsRotation( + annotations *TextualAnnotations, t1Partitions *sqlparser.PartitionOption, t2Partitions *sqlparser.PartitionOption, ) (bool, []*sqlparser.PartitionSpec, error) { @@ -1212,6 +1234,7 @@ func (c *CreateTableEntity) isRangePartitionsRotation( Names: []sqlparser.IdentifierCI{p.Name}, } partitionSpecs = append(partitionSpecs, partitionSpec) + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(p)) } for _, p := range addedPartitions2 { partitionSpec := &sqlparser.PartitionSpec{ @@ -1219,11 +1242,13 @@ func (c *CreateTableEntity) isRangePartitionsRotation( Definitions: []*sqlparser.PartitionDefinition{p}, } partitionSpecs = append(partitionSpecs, partitionSpec) + annotations.added = append(annotations.added, sqlparser.CanonicalString(p)) } return true, partitionSpecs, nil } func (c *CreateTableEntity) diffPartitions(alterTable *sqlparser.AlterTable, + annotations *TextualAnnotations, t1Partitions *sqlparser.PartitionOption, t2Partitions *sqlparser.PartitionOption, hints *DiffHints, @@ -1234,6 +1259,7 @@ func (c *CreateTableEntity) diffPartitions(alterTable *sqlparser.AlterTable, case t1Partitions == nil: // add partitioning alterTable.PartitionOption = t2Partitions + annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Partitions)) case t2Partitions == nil: // remove partitioning partitionSpec := &sqlparser.PartitionSpec{ @@ -1241,6 +1267,7 @@ func (c *CreateTableEntity) diffPartitions(alterTable *sqlparser.AlterTable, IsAll: true, } alterTable.PartitionSpec = partitionSpec + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Partitions)) case sqlparser.Equals.RefOfPartitionOption(t1Partitions, t2Partitions): // identical partitioning return nil, nil @@ -1256,7 +1283,7 @@ func (c *CreateTableEntity) diffPartitions(alterTable *sqlparser.AlterTable, // Having said that, we _do_ analyze the scenario of a RANGE partitioning rotation of partitions: // where zero or more partitions may have been dropped from the earlier range, and zero or more // partitions have been added with a later range: - isRotation, partitionSpecs, err := c.isRangePartitionsRotation(t1Partitions, t2Partitions) + isRotation, partitionSpecs, err := c.isRangePartitionsRotation(annotations, t1Partitions, t2Partitions) if err != nil { return nil, err } @@ -1271,11 +1298,14 @@ func (c *CreateTableEntity) diffPartitions(alterTable *sqlparser.AlterTable, } } alterTable.PartitionOption = t2Partitions + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Partitions)) + annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Partitions)) } return nil, nil } func (c *CreateTableEntity) diffConstraints(alterTable *sqlparser.AlterTable, + annotations *TextualAnnotations, t1Name string, t1Constraints []*sqlparser.ConstraintDefinition, t2Name string, @@ -1328,6 +1358,7 @@ func (c *CreateTableEntity) diffConstraints(alterTable *sqlparser.AlterTable, // constraint exists in t1 but not in t2, hence it is dropped dropConstraint := dropConstraintStatement(t1Constraint) alterTable.AlterOptions = append(alterTable.AlterOptions, dropConstraint) + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Constraint)) } else { t2ConstraintsCountMap[constraintName]-- } @@ -1354,6 +1385,8 @@ func (c *CreateTableEntity) diffConstraints(alterTable *sqlparser.AlterTable, Enforced: check2Details.Enforced, } alterTable.AlterOptions = append(alterTable.AlterOptions, alterConstraint) + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Constraint)) + annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Constraint)) continue } @@ -1364,6 +1397,8 @@ func (c *CreateTableEntity) diffConstraints(alterTable *sqlparser.AlterTable, } alterTable.AlterOptions = append(alterTable.AlterOptions, dropConstraint) alterTable.AlterOptions = append(alterTable.AlterOptions, addConstraint) + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Constraint)) + annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Constraint)) } } else { // constraint exists in t2 but not in t1, hence it is added @@ -1371,11 +1406,13 @@ func (c *CreateTableEntity) diffConstraints(alterTable *sqlparser.AlterTable, ConstraintDefinition: t2Constraint, } alterTable.AlterOptions = append(alterTable.AlterOptions, addConstraint) + annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Constraint)) } } } func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable, + annotations *TextualAnnotations, t1Keys []*sqlparser.IndexDefinition, t2Keys []*sqlparser.IndexDefinition, hints *DiffHints, @@ -1407,6 +1444,7 @@ func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable, // column exists in t1 but not in t2, hence it is dropped dropKey := dropKeyStatement(t1Key.Info) alterTable.AlterOptions = append(alterTable.AlterOptions, dropKey) + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Key)) } } @@ -1425,6 +1463,8 @@ func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable, Name: t2Key.Info.Name, Invisible: newVisibility, }) + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Key)) + annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Key)) continue } @@ -1435,6 +1475,8 @@ func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable, } alterTable.AlterOptions = append(alterTable.AlterOptions, dropKey) alterTable.AlterOptions = append(alterTable.AlterOptions, addKey) + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Key)) + annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Key)) } } else { // key exists in t2 but not in t1, hence it is added @@ -1452,6 +1494,7 @@ func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable, } if !addedAsSuperfluousStatement { alterTable.AlterOptions = append(alterTable.AlterOptions, addKey) + annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Key)) } } } @@ -1528,6 +1571,7 @@ func evaluateColumnReordering(t1SharedColumns, t2SharedColumns []*sqlparser.Colu // 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) diffColumns(alterTable *sqlparser.AlterTable, + annotations *TextualAnnotations, t1Columns []*sqlparser.ColumnDefinition, t2Columns []*sqlparser.ColumnDefinition, hints *DiffHints, @@ -1570,6 +1614,7 @@ func (c *CreateTableEntity) diffColumns(alterTable *sqlparser.AlterTable, Name: getColName(&t1Col.Name), } dropColumns = append(dropColumns, dropColumn) + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Col)) } } @@ -1625,6 +1670,8 @@ func (c *CreateTableEntity) diffColumns(alterTable *sqlparser.AlterTable, if modifyColumnDiff != nil { // column definition or ordering has changed modifyColumns = append(modifyColumns, modifyColumnDiff.modifyColumn) + annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Col.col)) + annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Col)) } } // Evaluate added columns @@ -1650,6 +1697,7 @@ func (c *CreateTableEntity) diffColumns(alterTable *sqlparser.AlterTable, } expectAppendIndex++ addColumns = append(addColumns, addColumn) + annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Col)) } } dropColumns, addColumns, renameColumns := heuristicallyDetectColumnRenames(dropColumns, addColumns, t1ColumnsMap, t2ColumnsMap, hints) diff --git a/go/vt/schemadiff/types.go b/go/vt/schemadiff/types.go index 30814dfc26c..c78eb57342b 100644 --- a/go/vt/schemadiff/types.go +++ b/go/vt/schemadiff/types.go @@ -29,6 +29,32 @@ const ( InstantDDLCapabilityPossible ) +type TextualAnnotationType int + +const ( + UnchangedTextualAnnotationType TextualAnnotationType = iota + AddedTextualAnnotationType + RemovedTextualAnnotationType +) + +type TextualAnnotationHint int + +const ( + PlusMinusSpaceTextualAnnotationHint TextualAnnotationHint = iota + PlusMinusEqualTextualAnnotationHint + PlusMinusTextualAnnotationHint + SchemadiffSuffixTextualAnnotationHint +) + +type TextualAnnotations struct { + removed []string + added []string +} + +func NewTextualAnnotations() *TextualAnnotations { + return &TextualAnnotations{} +} + // Entity stands for a database object we can diff: // - A table // - A view @@ -66,6 +92,8 @@ type EntityDiff interface { SetSubsequentDiff(EntityDiff) // InstantDDLCapability returns the ability of this diff to run with ALGORITHM=INSTANT InstantDDLCapability() InstantDDLCapability + + // TextualAnnotation() (from string, to string) } const ( From d1d2bd4318c7a916292564f5c4d8a4fecf0d3e3b Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 07:48:10 +0200 Subject: [PATCH 02/22] precompute annotations Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/annotations.go | 89 +++++++++++++++++---------------- 1 file changed, 47 insertions(+), 42 deletions(-) diff --git a/go/vt/schemadiff/annotations.go b/go/vt/schemadiff/annotations.go index 0783c1f873d..08cee594f6b 100644 --- a/go/vt/schemadiff/annotations.go +++ b/go/vt/schemadiff/annotations.go @@ -28,57 +28,62 @@ var ( func annotatedStatement(stmt string, annotationType TextualAnnotationType, annotationHint TextualAnnotationHint, annotations []string) string { anyLineAnnotated := false stmtLines := strings.Split(stmt, "\n") + annotationLines := map[string]bool{} + for _, annotation := range annotations { + annotation = strings.TrimSpace(annotation) + lines := strings.Split(annotation, "\n") + for _, line := range lines { + line = strings.TrimSpace(line) + annotationLines[line] = true + } + } for i := range stmtLines { lineAnnotated := false trimmedLine := stmtLines[i] trimmedLine = strings.TrimSpace(trimmedLine) - trimmedLine = strings.TrimRight(trimmedLine, ",") - trimmedLine = strings.TrimLeft(trimmedLine, "(") - trimmedLine = strings.TrimLeft(trimmedLine, ") ") - trimmedLine = strings.TrimSpace(trimmedLine) + // trimmedLine = strings.TrimRight(trimmedLine, ",") + // trimmedLine = strings.TrimLeft(trimmedLine, "(") + // trimmedLine = strings.TrimLeft(trimmedLine, ") ") + // trimmedLine = strings.TrimSpace(trimmedLine) if trimmedLine == "" { continue } - for _, annotation := range annotations { - annotation = strings.TrimSpace(annotation) - annotationLines := strings.Split(annotation, "\n") - for _, annTrimmedLine := range annotationLines { - if lineAnnotated { - break - } - - annTrimmedLine = strings.TrimSpace(annTrimmedLine) - possibleMutations := map[string]bool{ - trimmedLine: true, - ") " + trimmedLine: true, - "(" + trimmedLine: true, - "(" + trimmedLine + ",": true, - trimmedLine + ",": true, - trimmedLine + ")": true, - } - if possibleMutations[annTrimmedLine] { - // Annotate this line! - switch annotationHint { - case PlusMinusSpaceTextualAnnotationHint, - PlusMinusEqualTextualAnnotationHint, - PlusMinusTextualAnnotationHint: - switch annotationType { - case AddedTextualAnnotationType: - stmtLines[i] = "+" + stmtLines[i] - case RemovedTextualAnnotationType: - stmtLines[i] = "-" + stmtLines[i] - } - case SchemadiffSuffixTextualAnnotationHint: - switch annotationType { - case AddedTextualAnnotationType: - stmtLines[i] = stmtLines[i] + annotationAddedHint - case RemovedTextualAnnotationType: - stmtLines[i] = stmtLines[i] + annotationRemovedHint - } + for annotationLine := range annotationLines { + if lineAnnotated { + break + } + possibleMutations := map[string]bool{ + annotationLine: true, + ") " + annotationLine: true, + ") " + annotationLine + ",": true, + "(" + annotationLine: true, + "(" + annotationLine + ",": true, + annotationLine + ",": true, + annotationLine + ")": true, + } + if possibleMutations[trimmedLine] { + // Annotate this line! + switch annotationHint { + case PlusMinusSpaceTextualAnnotationHint, + PlusMinusEqualTextualAnnotationHint, + PlusMinusTextualAnnotationHint: + switch annotationType { + case AddedTextualAnnotationType: + stmtLines[i] = "+" + stmtLines[i] + case RemovedTextualAnnotationType: + stmtLines[i] = "-" + stmtLines[i] + } + case SchemadiffSuffixTextualAnnotationHint: + switch annotationType { + case AddedTextualAnnotationType: + stmtLines[i] = stmtLines[i] + annotationAddedHint + case RemovedTextualAnnotationType: + stmtLines[i] = stmtLines[i] + annotationRemovedHint } - lineAnnotated = true - anyLineAnnotated = true } + lineAnnotated = true + anyLineAnnotated = true + delete(annotationLines, annotationLine) // no need to iterate it in the future } } if !lineAnnotated { From 4985414837285a4c322fb99c59416decec0f9796 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 09:06:34 +0200 Subject: [PATCH 03/22] formalize TextualAnnotations Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/annotations.go | 185 +++++++++++++++++++++----------- go/vt/schemadiff/table.go | 59 +++++----- go/vt/schemadiff/table_test.go | 40 +++++++ go/vt/schemadiff/types.go | 26 ----- 4 files changed, 189 insertions(+), 121 deletions(-) diff --git a/go/vt/schemadiff/annotations.go b/go/vt/schemadiff/annotations.go index 08cee594f6b..1baecdd9f09 100644 --- a/go/vt/schemadiff/annotations.go +++ b/go/vt/schemadiff/annotations.go @@ -20,31 +20,118 @@ import ( "strings" ) -var ( - annotationAddedHint = " -- schemadiff:added" - annotationRemovedHint = " -- schemadiff:removed" +type TextualAnnotationType int + +const ( + UnchangedTextualAnnotationType TextualAnnotationType = iota + AddedTextualAnnotationType + RemovedTextualAnnotationType ) -func annotatedStatement(stmt string, annotationType TextualAnnotationType, annotationHint TextualAnnotationHint, annotations []string) string { - anyLineAnnotated := false +type TextualAnnotatedLine struct { + line string + typ TextualAnnotationType +} + +type TextualAnnotations struct { + lines []*TextualAnnotatedLine + hasChanges bool +} + +func NewTextualAnnotations() *TextualAnnotations { + return &TextualAnnotations{} +} + +func (a *TextualAnnotations) Len() int { + return len(a.lines) +} + +func (a *TextualAnnotations) mark(line string, typ TextualAnnotationType) { + a.lines = append(a.lines, &TextualAnnotatedLine{line: line, typ: typ}) + if typ != UnchangedTextualAnnotationType { + a.hasChanges = true + } +} + +func (a *TextualAnnotations) MarkAdded(line string) { + a.mark(line, AddedTextualAnnotationType) +} + +func (a *TextualAnnotations) MarkRemoved(line string) { + a.mark(line, RemovedTextualAnnotationType) +} + +func (a *TextualAnnotations) MarkUnchanged(line string) { + a.mark(line, UnchangedTextualAnnotationType) +} + +func (a *TextualAnnotations) ByType(typ TextualAnnotationType) (r []*TextualAnnotatedLine) { + for _, line := range a.lines { + if line.typ == typ { + r = append(r, line) + } + } + return r +} + +func (a *TextualAnnotations) Added() (r []*TextualAnnotatedLine) { + return a.ByType(AddedTextualAnnotationType) +} + +func (a *TextualAnnotations) Removed() (r []*TextualAnnotatedLine) { + return a.ByType(RemovedTextualAnnotationType) +} + +func (a *TextualAnnotations) Export(format TextualAnnotationFormat) string { + textLines := make([]string, 0, len(a.lines)) + for _, annotatedLine := range a.lines { + switch annotatedLine.typ { + case AddedTextualAnnotationType: + annotatedLine.line = "+" + annotatedLine.line + case RemovedTextualAnnotationType: + annotatedLine.line = "-" + annotatedLine.line + default: + // line unchanged + switch format { + case PlusMinusSpaceTextualAnnotationFormat: + if a.hasChanges { + annotatedLine.line = " " + annotatedLine.line + } + case PlusMinusEqualTextualAnnotationFormat: + annotatedLine.line = "=" + annotatedLine.line + case PlusMinusTextualAnnotationFormat: + } + } + textLines = append(textLines, annotatedLine.line) + } + return strings.Join(textLines, "\n") +} + +type TextualAnnotationFormat int + +const ( + PlusMinusSpaceTextualAnnotationFormat TextualAnnotationFormat = iota + PlusMinusEqualTextualAnnotationFormat + PlusMinusTextualAnnotationFormat + SchemadiffSuffixTextualAnnotationFormat +) + +func annotatedStatement(stmt string, annotationType TextualAnnotationType, annotations *TextualAnnotations) *TextualAnnotations { stmtLines := strings.Split(stmt, "\n") + result := NewTextualAnnotations() annotationLines := map[string]bool{} - for _, annotation := range annotations { - annotation = strings.TrimSpace(annotation) - lines := strings.Split(annotation, "\n") + for _, annotation := range annotations.ByType(annotationType) { + lines := strings.Split(annotation.line, "\n") for _, line := range lines { line = strings.TrimSpace(line) - annotationLines[line] = true + if line != "" { + annotationLines[line] = true + } } } for i := range stmtLines { lineAnnotated := false - trimmedLine := stmtLines[i] - trimmedLine = strings.TrimSpace(trimmedLine) - // trimmedLine = strings.TrimRight(trimmedLine, ",") - // trimmedLine = strings.TrimLeft(trimmedLine, "(") - // trimmedLine = strings.TrimLeft(trimmedLine, ") ") - // trimmedLine = strings.TrimSpace(trimmedLine) + trimmedLine := strings.TrimSpace(stmtLines[i]) if trimmedLine == "" { continue } @@ -63,80 +150,48 @@ func annotatedStatement(stmt string, annotationType TextualAnnotationType, annot } if possibleMutations[trimmedLine] { // Annotate this line! - switch annotationHint { - case PlusMinusSpaceTextualAnnotationHint, - PlusMinusEqualTextualAnnotationHint, - PlusMinusTextualAnnotationHint: - switch annotationType { - case AddedTextualAnnotationType: - stmtLines[i] = "+" + stmtLines[i] - case RemovedTextualAnnotationType: - stmtLines[i] = "-" + stmtLines[i] - } - case SchemadiffSuffixTextualAnnotationHint: - switch annotationType { - case AddedTextualAnnotationType: - stmtLines[i] = stmtLines[i] + annotationAddedHint - case RemovedTextualAnnotationType: - stmtLines[i] = stmtLines[i] + annotationRemovedHint - } - } + result.mark(stmtLines[i], annotationType) lineAnnotated = true - anyLineAnnotated = true delete(annotationLines, annotationLine) // no need to iterate it in the future } } if !lineAnnotated { - switch annotationHint { - case PlusMinusSpaceTextualAnnotationHint: - stmtLines[i] = " " + stmtLines[i] - case PlusMinusEqualTextualAnnotationHint: - stmtLines[i] = "=" + stmtLines[i] - case PlusMinusTextualAnnotationHint: - // line unchanged - case SchemadiffSuffixTextualAnnotationHint: - // line unchanged - } + result.MarkUnchanged(stmtLines[i]) } } - if !anyLineAnnotated { - return stmt - } - return strings.Join(stmtLines, "\n") + return result } -func unifiedAnnotated(annotatedFrom string, annotatedTo string) string { - fromLines := strings.Split(annotatedFrom, "\n") - toLines := strings.Split(annotatedTo, "\n") - unified := []string{} +func unifiedAnnotated(from *TextualAnnotations, to *TextualAnnotations) *TextualAnnotations { + unified := NewTextualAnnotations() fromIndex := 0 toIndex := 0 - for fromIndex < len(fromLines) || toIndex < len(toLines) { + for fromIndex < from.Len() || toIndex < to.Len() { matchingLine := "" - if fromIndex < len(fromLines) { - fromLine := fromLines[fromIndex] - if strings.HasSuffix(fromLine, annotationRemovedHint) { - unified = append(unified, "- "+fromLine) + if fromIndex < from.Len() { + fromLine := from.lines[fromIndex] + if fromLine.typ == RemovedTextualAnnotationType { + unified.MarkRemoved(fromLine.line) fromIndex++ continue } - matchingLine = fromLine + matchingLine = fromLine.line } - if toIndex < len(toLines) { - toLine := toLines[toIndex] - if strings.HasSuffix(toLine, annotationAddedHint) { - unified = append(unified, "+ "+toLine) + if toIndex < to.Len() { + toLine := to.lines[toIndex] + if toLine.typ == AddedTextualAnnotationType { + unified.MarkAdded(toLine.line) toIndex++ continue } if matchingLine == "" { - matchingLine = toLine + matchingLine = toLine.line } } - unified = append(unified, matchingLine) + unified.MarkUnchanged(matchingLine) fromIndex++ toIndex++ } - return strings.Join(unified, "\n") + return unified } diff --git a/go/vt/schemadiff/table.go b/go/vt/schemadiff/table.go index 87faa5fb455..0b1b39f1677 100644 --- a/go/vt/schemadiff/table.go +++ b/go/vt/schemadiff/table.go @@ -60,12 +60,11 @@ func (d *AlterTableEntityDiff) Entities() (from Entity, to Entity) { return d.from, d.to } -func (d *AlterTableEntityDiff) Annotated() (from string, to string, unified string) { - annotationHint := SchemadiffSuffixTextualAnnotationHint +func (d *AlterTableEntityDiff) Annotated() (from *TextualAnnotations, to *TextualAnnotations, unified *TextualAnnotations) { fromStatementString := d.from.Create().CanonicalStatementString() - from = annotatedStatement(fromStatementString, RemovedTextualAnnotationType, annotationHint, d.annotations.removed) + from = annotatedStatement(fromStatementString, RemovedTextualAnnotationType, d.annotations) toStatementString := d.to.Create().CanonicalStatementString() - to = annotatedStatement(toStatementString, AddedTextualAnnotationType, annotationHint, d.annotations.added) + to = annotatedStatement(toStatementString, AddedTextualAnnotationType, d.annotations) return from, to, unifiedAnnotated(from, to) } @@ -1083,15 +1082,15 @@ func (c *CreateTableEntity) diffOptions(alterTable *sqlparser.AlterTable, if tableOption != nil { tableOption.Name = t1Option.Name alterTableOptions = append(alterTableOptions, tableOption) - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(sqlparser.TableOptions{t1Option})) + annotations.MarkRemoved(sqlparser.CanonicalString(sqlparser.TableOptions{t1Option})) } } } // changed options modifyTableOption := func(option1, option2 *sqlparser.TableOption) { alterTableOptions = append(alterTableOptions, option2) - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(sqlparser.TableOptions{option1})) - annotations.added = append(annotations.added, sqlparser.CanonicalString(sqlparser.TableOptions{option2})) + annotations.MarkRemoved(sqlparser.CanonicalString(sqlparser.TableOptions{option1})) + annotations.MarkAdded(sqlparser.CanonicalString(sqlparser.TableOptions{option2})) } for _, t2Option := range t2Options { if t1Option, ok := t1OptionsMap[t2Option.Name]; ok { @@ -1142,7 +1141,7 @@ func (c *CreateTableEntity) diffOptions(alterTable *sqlparser.AlterTable, } addTableOption := func(option *sqlparser.TableOption) { alterTableOptions = append(alterTableOptions, option) - annotations.added = append(annotations.added, sqlparser.CanonicalString(sqlparser.TableOptions{option})) + annotations.MarkAdded(sqlparser.CanonicalString(sqlparser.TableOptions{option})) } // added options for _, t2Option := range t2Options { @@ -1234,7 +1233,7 @@ func (c *CreateTableEntity) isRangePartitionsRotation( Names: []sqlparser.IdentifierCI{p.Name}, } partitionSpecs = append(partitionSpecs, partitionSpec) - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(p)) + annotations.MarkRemoved(sqlparser.CanonicalString(p)) } for _, p := range addedPartitions2 { partitionSpec := &sqlparser.PartitionSpec{ @@ -1242,7 +1241,7 @@ func (c *CreateTableEntity) isRangePartitionsRotation( Definitions: []*sqlparser.PartitionDefinition{p}, } partitionSpecs = append(partitionSpecs, partitionSpec) - annotations.added = append(annotations.added, sqlparser.CanonicalString(p)) + annotations.MarkAdded(sqlparser.CanonicalString(p)) } return true, partitionSpecs, nil } @@ -1259,7 +1258,7 @@ func (c *CreateTableEntity) diffPartitions(alterTable *sqlparser.AlterTable, case t1Partitions == nil: // add partitioning alterTable.PartitionOption = t2Partitions - annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Partitions)) + annotations.MarkAdded(sqlparser.CanonicalString(t2Partitions)) case t2Partitions == nil: // remove partitioning partitionSpec := &sqlparser.PartitionSpec{ @@ -1267,7 +1266,7 @@ func (c *CreateTableEntity) diffPartitions(alterTable *sqlparser.AlterTable, IsAll: true, } alterTable.PartitionSpec = partitionSpec - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Partitions)) + annotations.MarkRemoved(sqlparser.CanonicalString(t1Partitions)) case sqlparser.Equals.RefOfPartitionOption(t1Partitions, t2Partitions): // identical partitioning return nil, nil @@ -1298,8 +1297,8 @@ func (c *CreateTableEntity) diffPartitions(alterTable *sqlparser.AlterTable, } } alterTable.PartitionOption = t2Partitions - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Partitions)) - annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Partitions)) + annotations.MarkRemoved(sqlparser.CanonicalString(t1Partitions)) + annotations.MarkAdded(sqlparser.CanonicalString(t2Partitions)) } return nil, nil } @@ -1358,7 +1357,7 @@ func (c *CreateTableEntity) diffConstraints(alterTable *sqlparser.AlterTable, // constraint exists in t1 but not in t2, hence it is dropped dropConstraint := dropConstraintStatement(t1Constraint) alterTable.AlterOptions = append(alterTable.AlterOptions, dropConstraint) - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Constraint)) + annotations.MarkRemoved(sqlparser.CanonicalString(t1Constraint)) } else { t2ConstraintsCountMap[constraintName]-- } @@ -1385,8 +1384,8 @@ func (c *CreateTableEntity) diffConstraints(alterTable *sqlparser.AlterTable, Enforced: check2Details.Enforced, } alterTable.AlterOptions = append(alterTable.AlterOptions, alterConstraint) - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Constraint)) - annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Constraint)) + annotations.MarkRemoved(sqlparser.CanonicalString(t1Constraint)) + annotations.MarkAdded(sqlparser.CanonicalString(t2Constraint)) continue } @@ -1397,8 +1396,8 @@ func (c *CreateTableEntity) diffConstraints(alterTable *sqlparser.AlterTable, } alterTable.AlterOptions = append(alterTable.AlterOptions, dropConstraint) alterTable.AlterOptions = append(alterTable.AlterOptions, addConstraint) - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Constraint)) - annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Constraint)) + annotations.MarkRemoved(sqlparser.CanonicalString(t1Constraint)) + annotations.MarkAdded(sqlparser.CanonicalString(t2Constraint)) } } else { // constraint exists in t2 but not in t1, hence it is added @@ -1406,7 +1405,7 @@ func (c *CreateTableEntity) diffConstraints(alterTable *sqlparser.AlterTable, ConstraintDefinition: t2Constraint, } alterTable.AlterOptions = append(alterTable.AlterOptions, addConstraint) - annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Constraint)) + annotations.MarkAdded(sqlparser.CanonicalString(t2Constraint)) } } } @@ -1444,7 +1443,7 @@ func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable, // column exists in t1 but not in t2, hence it is dropped dropKey := dropKeyStatement(t1Key.Info) alterTable.AlterOptions = append(alterTable.AlterOptions, dropKey) - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Key)) + annotations.MarkRemoved(sqlparser.CanonicalString(t1Key)) } } @@ -1463,8 +1462,8 @@ func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable, Name: t2Key.Info.Name, Invisible: newVisibility, }) - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Key)) - annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Key)) + annotations.MarkRemoved(sqlparser.CanonicalString(t1Key)) + annotations.MarkAdded(sqlparser.CanonicalString(t2Key)) continue } @@ -1475,8 +1474,8 @@ func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable, } alterTable.AlterOptions = append(alterTable.AlterOptions, dropKey) alterTable.AlterOptions = append(alterTable.AlterOptions, addKey) - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Key)) - annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Key)) + annotations.MarkRemoved(sqlparser.CanonicalString(t1Key)) + annotations.MarkAdded(sqlparser.CanonicalString(t2Key)) } } else { // key exists in t2 but not in t1, hence it is added @@ -1494,7 +1493,7 @@ func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable, } if !addedAsSuperfluousStatement { alterTable.AlterOptions = append(alterTable.AlterOptions, addKey) - annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Key)) + annotations.MarkAdded(sqlparser.CanonicalString(t2Key)) } } } @@ -1614,7 +1613,7 @@ func (c *CreateTableEntity) diffColumns(alterTable *sqlparser.AlterTable, Name: getColName(&t1Col.Name), } dropColumns = append(dropColumns, dropColumn) - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Col)) + annotations.MarkRemoved(sqlparser.CanonicalString(t1Col)) } } @@ -1670,8 +1669,8 @@ func (c *CreateTableEntity) diffColumns(alterTable *sqlparser.AlterTable, if modifyColumnDiff != nil { // column definition or ordering has changed modifyColumns = append(modifyColumns, modifyColumnDiff.modifyColumn) - annotations.removed = append(annotations.removed, sqlparser.CanonicalString(t1Col.col)) - annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Col)) + annotations.MarkRemoved(sqlparser.CanonicalString(t1Col.col)) + annotations.MarkAdded(sqlparser.CanonicalString(t2Col)) } } // Evaluate added columns @@ -1697,7 +1696,7 @@ func (c *CreateTableEntity) diffColumns(alterTable *sqlparser.AlterTable, } expectAppendIndex++ addColumns = append(addColumns, addColumn) - annotations.added = append(annotations.added, sqlparser.CanonicalString(t2Col)) + annotations.MarkAdded(sqlparser.CanonicalString(t2Col)) } } dropColumns, addColumns, renameColumns := heuristicallyDetectColumnRenames(dropColumns, addColumns, t1ColumnsMap, t2ColumnsMap, hints) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index af72f15776b..8bd6671e36e 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -17,6 +17,7 @@ limitations under the License. package schemadiff import ( + "fmt" "strings" "testing" @@ -1465,6 +1466,45 @@ func TestCreateTableDiff(t *testing.T) { applied.Create().CanonicalStatementString(), ) } + { // Validate annotations + alterEntityDiff, ok := alter.(*AlterTableEntityDiff) + require.True(t, ok) + annotatedFrom, annotatedTo, annotatedUnified := alterEntityDiff.Annotated() + annotatedFromString := annotatedFrom.Export(PlusMinusSpaceTextualAnnotationFormat) + annotatedToString := annotatedTo.Export(PlusMinusSpaceTextualAnnotationFormat) + annotatedUnifiedString := annotatedUnified.Export(PlusMinusSpaceTextualAnnotationFormat) + { + eFromStatementString := eFrom.Create().CanonicalStatementString() + for _, annotation := range alterEntityDiff.annotations.Removed() { + require.NotEmpty(t, annotation.line) + assert.Contains(t, eFromStatementString, annotation.line) + } + if len(alterEntityDiff.annotations.Removed()) == 0 { + assert.Equal(t, eFromStatementString, annotatedFromString) + } else { + assert.NotEqual(t, eFromStatementString, annotatedFromString) + } + } + { + eToStatementString := eTo.Create().CanonicalStatementString() + for _, annotation := range alterEntityDiff.annotations.Added() { + require.NotEmpty(t, annotation.line) + assert.Contains(t, eToStatementString, annotation.line) + } + if len(alterEntityDiff.annotations.Added()) == 0 { + assert.Equal(t, eToStatementString, annotatedToString) + } else { + assert.NotEqual(t, eToStatementString, annotatedToString) + } + } + fmt.Println("============== ") + fmt.Println(">>>>>>", alter.CanonicalStatementString()) + fmt.Println(annotatedFromString) + fmt.Println(annotatedToString) + fmt.Println("-------------- unified") + fmt.Println(annotatedUnifiedString) + fmt.Println("-------------- / ") + } } { cdiff := alter.CanonicalStatementString() diff --git a/go/vt/schemadiff/types.go b/go/vt/schemadiff/types.go index c78eb57342b..4f044badeae 100644 --- a/go/vt/schemadiff/types.go +++ b/go/vt/schemadiff/types.go @@ -29,32 +29,6 @@ const ( InstantDDLCapabilityPossible ) -type TextualAnnotationType int - -const ( - UnchangedTextualAnnotationType TextualAnnotationType = iota - AddedTextualAnnotationType - RemovedTextualAnnotationType -) - -type TextualAnnotationHint int - -const ( - PlusMinusSpaceTextualAnnotationHint TextualAnnotationHint = iota - PlusMinusEqualTextualAnnotationHint - PlusMinusTextualAnnotationHint - SchemadiffSuffixTextualAnnotationHint -) - -type TextualAnnotations struct { - removed []string - added []string -} - -func NewTextualAnnotations() *TextualAnnotations { - return &TextualAnnotations{} -} - // Entity stands for a database object we can diff: // - A table // - A view From ebe2caf475c2a9800ebe52f279e5f2f9da27e19f Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 10:34:22 +0200 Subject: [PATCH 04/22] adding tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/annotations.go | 72 +++++----- go/vt/schemadiff/table_test.go | 243 +++++++++++++++++++++++++++++++- 2 files changed, 274 insertions(+), 41 deletions(-) diff --git a/go/vt/schemadiff/annotations.go b/go/vt/schemadiff/annotations.go index 1baecdd9f09..bf5c3136140 100644 --- a/go/vt/schemadiff/annotations.go +++ b/go/vt/schemadiff/annotations.go @@ -28,13 +28,13 @@ const ( RemovedTextualAnnotationType ) -type TextualAnnotatedLine struct { - line string +type AnnotatedText struct { + text string typ TextualAnnotationType } type TextualAnnotations struct { - lines []*TextualAnnotatedLine + texts []*AnnotatedText hasChanges bool } @@ -43,66 +43,65 @@ func NewTextualAnnotations() *TextualAnnotations { } func (a *TextualAnnotations) Len() int { - return len(a.lines) + return len(a.texts) } -func (a *TextualAnnotations) mark(line string, typ TextualAnnotationType) { - a.lines = append(a.lines, &TextualAnnotatedLine{line: line, typ: typ}) +func (a *TextualAnnotations) mark(text string, typ TextualAnnotationType) { + a.texts = append(a.texts, &AnnotatedText{text: text, typ: typ}) if typ != UnchangedTextualAnnotationType { a.hasChanges = true } } -func (a *TextualAnnotations) MarkAdded(line string) { - a.mark(line, AddedTextualAnnotationType) +func (a *TextualAnnotations) MarkAdded(text string) { + a.mark(text, AddedTextualAnnotationType) } -func (a *TextualAnnotations) MarkRemoved(line string) { - a.mark(line, RemovedTextualAnnotationType) +func (a *TextualAnnotations) MarkRemoved(text string) { + a.mark(text, RemovedTextualAnnotationType) } -func (a *TextualAnnotations) MarkUnchanged(line string) { - a.mark(line, UnchangedTextualAnnotationType) +func (a *TextualAnnotations) MarkUnchanged(text string) { + a.mark(text, UnchangedTextualAnnotationType) } -func (a *TextualAnnotations) ByType(typ TextualAnnotationType) (r []*TextualAnnotatedLine) { - for _, line := range a.lines { - if line.typ == typ { - r = append(r, line) +func (a *TextualAnnotations) ByType(typ TextualAnnotationType) (r []*AnnotatedText) { + for _, text := range a.texts { + if text.typ == typ { + r = append(r, text) } } return r } -func (a *TextualAnnotations) Added() (r []*TextualAnnotatedLine) { +func (a *TextualAnnotations) Added() (r []*AnnotatedText) { return a.ByType(AddedTextualAnnotationType) } -func (a *TextualAnnotations) Removed() (r []*TextualAnnotatedLine) { +func (a *TextualAnnotations) Removed() (r []*AnnotatedText) { return a.ByType(RemovedTextualAnnotationType) } func (a *TextualAnnotations) Export(format TextualAnnotationFormat) string { - textLines := make([]string, 0, len(a.lines)) - for _, annotatedLine := range a.lines { - switch annotatedLine.typ { + textLines := make([]string, 0, len(a.texts)) + for _, annotatedText := range a.texts { + switch annotatedText.typ { case AddedTextualAnnotationType: - annotatedLine.line = "+" + annotatedLine.line + annotatedText.text = "+" + annotatedText.text case RemovedTextualAnnotationType: - annotatedLine.line = "-" + annotatedLine.line + annotatedText.text = "-" + annotatedText.text default: - // line unchanged + // text unchanged switch format { case PlusMinusSpaceTextualAnnotationFormat: if a.hasChanges { - annotatedLine.line = " " + annotatedLine.line + // If there is absolutely no change, we don't add a space anywhere + annotatedText.text = " " + annotatedText.text } - case PlusMinusEqualTextualAnnotationFormat: - annotatedLine.line = "=" + annotatedLine.line case PlusMinusTextualAnnotationFormat: } } - textLines = append(textLines, annotatedLine.line) + textLines = append(textLines, annotatedText.text) } return strings.Join(textLines, "\n") } @@ -111,9 +110,7 @@ type TextualAnnotationFormat int const ( PlusMinusSpaceTextualAnnotationFormat TextualAnnotationFormat = iota - PlusMinusEqualTextualAnnotationFormat PlusMinusTextualAnnotationFormat - SchemadiffSuffixTextualAnnotationFormat ) func annotatedStatement(stmt string, annotationType TextualAnnotationType, annotations *TextualAnnotations) *TextualAnnotations { @@ -121,7 +118,8 @@ func annotatedStatement(stmt string, annotationType TextualAnnotationType, annot result := NewTextualAnnotations() annotationLines := map[string]bool{} for _, annotation := range annotations.ByType(annotationType) { - lines := strings.Split(annotation.line, "\n") + // An annotated text could be multiline. Partition specs are such. + lines := strings.Split(annotation.text, "\n") for _, line := range lines { line = strings.TrimSpace(line) if line != "" { @@ -170,23 +168,23 @@ func unifiedAnnotated(from *TextualAnnotations, to *TextualAnnotations) *Textual for fromIndex < from.Len() || toIndex < to.Len() { matchingLine := "" if fromIndex < from.Len() { - fromLine := from.lines[fromIndex] + fromLine := from.texts[fromIndex] if fromLine.typ == RemovedTextualAnnotationType { - unified.MarkRemoved(fromLine.line) + unified.MarkRemoved(fromLine.text) fromIndex++ continue } - matchingLine = fromLine.line + matchingLine = fromLine.text } if toIndex < to.Len() { - toLine := to.lines[toIndex] + toLine := to.texts[toIndex] if toLine.typ == AddedTextualAnnotationType { - unified.MarkAdded(toLine.line) + unified.MarkAdded(toLine.text) toIndex++ continue } if matchingLine == "" { - matchingLine = toLine.line + matchingLine = toLine.text } } unified.MarkUnchanged(matchingLine) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index 8bd6671e36e..fe2437cc508 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -47,6 +47,7 @@ func TestCreateTableDiff(t *testing.T) { charset int algorithm int enumreorder int + textdiffs []string }{ { name: "identical", @@ -71,6 +72,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t (Id int not null, primary key(id))", diff: "alter table t modify column Id int not null", cdiff: "ALTER TABLE `t` MODIFY COLUMN `Id` int NOT NULL", + textdiffs: []string{ + "- `id` int NOT NULL,", + "+ `Id` int NOT NULL,", + }, }, { name: "identical, name change", @@ -108,6 +113,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, `i` int not null default 0)", diff: "alter table t1 add column i int not null default 0", cdiff: "ALTER TABLE `t1` ADD COLUMN `i` int NOT NULL DEFAULT 0", + textdiffs: []string{ + "+ `i` int NOT NULL DEFAULT 0,", + }, }, { name: "dropped column", @@ -117,6 +125,9 @@ func TestCreateTableDiff(t *testing.T) { cdiff: "ALTER TABLE `t1` DROP COLUMN `i`", fromName: "t1", toName: "t2", + textdiffs: []string{ + "- `i` int NOT NULL DEFAULT 0,", + }, }, { name: "modified column", @@ -124,6 +135,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, `i` bigint unsigned default null)", diff: "alter table t1 modify column i bigint unsigned", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `i` bigint unsigned", + textdiffs: []string{ + "- `i` int NOT NULL DEFAULT 0,", + "+ `i` bigint unsigned,", + }, }, { name: "added column, dropped column, modified column", @@ -131,6 +146,12 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, ts timestamp null, `i` bigint unsigned default null)", diff: "alter table t1 drop column c, modify column i bigint unsigned, add column ts timestamp null after id", cdiff: "ALTER TABLE `t1` DROP COLUMN `c`, MODIFY COLUMN `i` bigint unsigned, ADD COLUMN `ts` timestamp NULL AFTER `id`", + textdiffs: []string{ + "- `c` char(3) DEFAULT '',", + "- `i` int NOT NULL DEFAULT 0,", + "+ `i` bigint unsigned,", + "+ `ts` timestamp NULL,", + }, }, // columns, rename { @@ -139,6 +160,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, i2 int not null, c char(3) default '')", diff: "alter table t1 drop column i1, add column i2 int not null after id", cdiff: "ALTER TABLE `t1` DROP COLUMN `i1`, ADD COLUMN `i2` int NOT NULL AFTER `id`", + textdiffs: []string{ + "- `i1` int NOT NULL,", + "+ `i2` int NOT NULL,", + }, }, { name: "rename mid column. statement", @@ -147,6 +172,10 @@ func TestCreateTableDiff(t *testing.T) { colrename: ColumnRenameHeuristicStatement, diff: "alter table t1 rename column i1 to i2", cdiff: "ALTER TABLE `t1` RENAME COLUMN `i1` TO `i2`", + textdiffs: []string{ + "- `i1` int NOT NULL,", + "+ `i2` int NOT NULL,", + }, }, { name: "rename last column. statement", @@ -155,6 +184,10 @@ func TestCreateTableDiff(t *testing.T) { colrename: ColumnRenameHeuristicStatement, diff: "alter table t1 rename column i1 to i2", cdiff: "ALTER TABLE `t1` RENAME COLUMN `i1` TO `i2`", + textdiffs: []string{ + "- `i1` int NOT NULL,", + "+ `i2` int NOT NULL,", + }, }, { name: "rename two columns. statement", @@ -163,6 +196,12 @@ func TestCreateTableDiff(t *testing.T) { colrename: ColumnRenameHeuristicStatement, diff: "alter table t1 rename column i1 to i2, rename column v1 to v2", cdiff: "ALTER TABLE `t1` RENAME COLUMN `i1` TO `i2`, RENAME COLUMN `v1` TO `v2`", + textdiffs: []string{ + "- `i1` int NOT NULL,", + "- `v1` varchar(32),", + "+ `i2` int NOT NULL,", + "+ `v2` varchar(32),", + }, }, { name: "rename mid column and add an index. statement", @@ -171,6 +210,11 @@ func TestCreateTableDiff(t *testing.T) { colrename: ColumnRenameHeuristicStatement, diff: "alter table t1 rename column i1 to i2, add key i2_idx (i2)", cdiff: "ALTER TABLE `t1` RENAME COLUMN `i1` TO `i2`, ADD KEY `i2_idx` (`i2`)", + textdiffs: []string{ + "- `i1` int NOT NULL,", + "+ `i2` int NOT NULL,", + "+ KEY `i2_idx` (`i2`)", + }, }, { // in a future iteration, this will generate a RENAME for both column, like in the previous test. Until then, we do not RENAME two successive columns @@ -180,6 +224,12 @@ func TestCreateTableDiff(t *testing.T) { colrename: ColumnRenameHeuristicStatement, diff: "alter table t1 drop column i1, drop column v1, add column i2 int not null, add column v2 varchar(32)", cdiff: "ALTER TABLE `t1` DROP COLUMN `i1`, DROP COLUMN `v1`, ADD COLUMN `i2` int NOT NULL, ADD COLUMN `v2` varchar(32)", + textdiffs: []string{ + "- `i1` int NOT NULL,", + "- `v1` varchar(32),", + "+ `i2` int NOT NULL,", + "+ `v2` varchar(32),", + }, }, // columns, reordering { @@ -188,6 +238,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, a int, c int, b int, d int)", diff: "alter table t1 modify column c int after a", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `c` int AFTER `a`", + textdiffs: []string{ + "+ `c` int,", + "- `c` int,", + }, }, { name: "reorder column, far jump", @@ -195,6 +249,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (a int, b int, c int, d int, id int primary key)", diff: "alter table t1 modify column id int after d", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `id` int AFTER `d`", + textdiffs: []string{ + "- `id` int,", + "+ `id` int,", + }, }, { name: "reorder column, far jump with case sentivity", @@ -202,6 +260,12 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (a int, B int, c int, d int, id int primary key)", diff: "alter table t1 modify column B int, modify column id int after d", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `B` int, MODIFY COLUMN `id` int AFTER `d`", + textdiffs: []string{ + "- `id` int,", + "+ `id` int,", + "- `b` int,", + "+ `B` int,", + }, }, { name: "reorder column, far jump, another reorder", @@ -209,6 +273,12 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (a int, c int, b int, d int, id int primary key)", diff: "alter table t1 modify column c int after a, modify column id int after d", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `c` int AFTER `a`, MODIFY COLUMN `id` int AFTER `d`", + textdiffs: []string{ + "- `id` int,", + "+ `id` int,", + "- `c` int,", + "+ `c` int,", + }, }, { name: "reorder column, far jump, another reorder 2", @@ -216,6 +286,12 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (c int, a int, b int, d int, id int primary key)", diff: "alter table t1 modify column c int first, modify column id int after d", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `c` int FIRST, MODIFY COLUMN `id` int AFTER `d`", + textdiffs: []string{ + "- `id` int,", + "+ `id` int,", + "- `c` int,", + "+ `c` int,", + }, }, { name: "reorder column, far jump, another reorder 3", @@ -223,6 +299,12 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (a int, c int, b int, d int, id int primary key, e int, f int)", diff: "alter table t1 modify column c int after a, modify column id int after d", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `c` int AFTER `a`, MODIFY COLUMN `id` int AFTER `d`", + textdiffs: []string{ + "- `id` int,", + "+ `id` int,", + "- `c` int,", + "+ `c` int,", + }, }, { name: "reorder column, far jump, another reorder, removed columns", @@ -230,6 +312,14 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (a int, c int, f int, e int, id int primary key, g int)", diff: "alter table t1 drop column b, drop column d, modify column f int after c, modify column id int after e", cdiff: "ALTER TABLE `t1` DROP COLUMN `b`, DROP COLUMN `d`, MODIFY COLUMN `f` int AFTER `c`, MODIFY COLUMN `id` int AFTER `e`", + textdiffs: []string{ + "- `b` int,", + "- `d` int,", + "- `id` int,", + "+ `id` int,", + "- `f` int,", + "+ `f` int,", + }, }, { name: "two reorders", @@ -237,6 +327,12 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, b int, a int, c int, e int, d int, f int)", diff: "alter table t1 modify column b int after id, modify column e int after c", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `b` int AFTER `id`, MODIFY COLUMN `e` int AFTER `c`", + textdiffs: []string{ + "- `b` int,", + "+ `b` int,", + "- `e` int,", + "+ `e` int,", + }, }, { name: "two reorders, added and removed columns", @@ -244,6 +340,18 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (g int, id int primary key, h int, b int, a int, i int, e int, d int, j int, f int, k int)", diff: "alter table t1 drop column c, modify column b int after id, modify column e int after a, add column g int first, add column h int after id, add column i int after a, add column j int after d, add column k int", cdiff: "ALTER TABLE `t1` DROP COLUMN `c`, MODIFY COLUMN `b` int AFTER `id`, MODIFY COLUMN `e` int AFTER `a`, ADD COLUMN `g` int FIRST, ADD COLUMN `h` int AFTER `id`, ADD COLUMN `i` int AFTER `a`, ADD COLUMN `j` int AFTER `d`, ADD COLUMN `k` int", + textdiffs: []string{ + "- `c` int,", + "- `b` int,", + "+ `b` int,", + "- `e` int,", + "+ `e` int,", + "+ `g` int,", + "+ `h` int,", + "+ `i` int,", + "+ `j` int,", + "+ `k` int,", + }, }, { name: "reorder column and change data type", @@ -251,6 +359,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, a int, c bigint, b int, d int)", diff: "alter table t1 modify column c bigint after a", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `c` bigint AFTER `a`", + textdiffs: []string{ + "- `c` int,", + "+ `c` bigint,", + }, }, { name: "reorder column, first", @@ -258,6 +370,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (c int, id int primary key, a int, b int, d int)", diff: "alter table t1 modify column c int first", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `c` int FIRST", + textdiffs: []string{ + "- `c` int,", + "+ `c` int,", + }, }, { name: "add multiple columns", @@ -265,6 +381,11 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, a int, b int, c int, d int)", diff: "alter table t1 add column b int, add column c int, add column d int", cdiff: "ALTER TABLE `t1` ADD COLUMN `b` int, ADD COLUMN `c` int, ADD COLUMN `d` int", + textdiffs: []string{ + "+ `b` int,", + "+ `c` int,", + "+ `d` int,", + }, }, { name: "added column in middle", @@ -272,6 +393,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, a int, b int, x int, c int, d int)", diff: "alter table t1 add column x int after b", cdiff: "ALTER TABLE `t1` ADD COLUMN `x` int AFTER `b`", + textdiffs: []string{ + "+ `x` int,", + }, }, { name: "added multiple column in middle", @@ -279,6 +403,12 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (w int, x int, id int primary key, y int, a int, z int)", diff: "alter table t1 add column w int first, add column x int after w, add column y int after id, add column z int", cdiff: "ALTER TABLE `t1` ADD COLUMN `w` int FIRST, ADD COLUMN `x` int AFTER `w`, ADD COLUMN `y` int AFTER `id`, ADD COLUMN `z` int", + textdiffs: []string{ + "+ `w` int,", + "+ `x` int,", + "+ `y` int,", + "+ `z` int,", + }, }, { name: "added column first, reorder column", @@ -286,6 +416,11 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (x int, a int, id int primary key)", diff: "alter table t1 modify column a int first, add column x int first", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `a` int FIRST, ADD COLUMN `x` int FIRST", + textdiffs: []string{ + "- `a` int,", + "+ `a` int,", + "+ `x` int,", + }, }, { name: "added column in middle, add column on end, reorder column", @@ -293,6 +428,12 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, a int, b int, x int, d int, c int, y int)", diff: "alter table t1 modify column d int after b, add column x int after b, add column y int", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `d` int AFTER `b`, ADD COLUMN `x` int AFTER `b`, ADD COLUMN `y` int", + textdiffs: []string{ + "- `d` int,", + "+ `d` int,", + "+ `x` int,", + "+ `y` int,", + }, }, { name: "added column in middle, add column on end, reorder column 2", @@ -300,6 +441,12 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, a int, c int, x int, b int, d int, y int)", diff: "alter table t1 modify column c int after a, add column x int after c, add column y int", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `c` int AFTER `a`, ADD COLUMN `x` int AFTER `c`, ADD COLUMN `y` int", + textdiffs: []string{ + "- `c` int,", + "+ `c` int,", + "+ `x` int,", + "+ `y` int,", + }, }, // enum { @@ -308,6 +455,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, e enum('a', 'b', 'c', 'd'))", diff: "alter table t1 modify column e enum('a', 'b', 'c', 'd')", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `e` enum('a', 'b', 'c', 'd')", + textdiffs: []string{ + "- `e` enum('a', 'b', 'c'),", + "+ `e` enum('a', 'b', 'c', 'd'),", + }, }, { name: "truncate enum", @@ -315,6 +466,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, e enum('a', 'b'))", diff: "alter table t1 modify column e enum('a', 'b')", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `e` enum('a', 'b')", + textdiffs: []string{ + "- `e` enum('a', 'b', 'c'),", + "+ `e` enum('a', 'b'),", + }, }, { name: "rename enum value", @@ -322,6 +477,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, e enum('a', 'b', 'd'))", diff: "alter table t1 modify column e enum('a', 'b', 'd')", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `e` enum('a', 'b', 'd')", + textdiffs: []string{ + "- `e` enum('a', 'b', 'c'),", + "+ `e` enum('a', 'b', 'd'),", + }, }, { name: "reorder enum, fail", @@ -337,6 +496,10 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 modify column e enum('b', 'a', 'c')", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `e` enum('b', 'a', 'c')", enumreorder: EnumReorderStrategyAllow, + textdiffs: []string{ + "- `e` enum('a', 'b', 'c'),", + "+ `e` enum('b', 'a', 'c'),", + }, }, { name: "expand set", @@ -344,6 +507,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, e set('a', 'b', 'c', 'd'))", diff: "alter table t1 modify column e set('a', 'b', 'c', 'd')", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `e` set('a', 'b', 'c', 'd')", + textdiffs: []string{ + "- `e` set('a', 'b', 'c'),", + "+ `e` set('a', 'b', 'c', 'd'),", + }, }, { name: "truncate set", @@ -351,6 +518,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, e set('a', 'b'))", diff: "alter table t1 modify column e set('a', 'b')", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `e` set('a', 'b')", + textdiffs: []string{ + "- `e` set('a', 'b', 'c'),", + "+ `e` set('a', 'b'),", + }, }, { name: "rename set value", @@ -358,6 +529,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, e set('a', 'b', 'd'))", diff: "alter table t1 modify column e set('a', 'b', 'd')", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `e` set('a', 'b', 'd')", + textdiffs: []string{ + "- `e` set('a', 'b', 'c'),", + "+ `e` set('a', 'b', 'd'),", + }, }, { name: "reorder set, fail", @@ -373,6 +548,10 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 modify column e set('b', 'a', 'c')", cdiff: "ALTER TABLE `t1` MODIFY COLUMN `e` set('b', 'a', 'c')", enumreorder: EnumReorderStrategyAllow, + textdiffs: []string{ + "- `e` set('a', 'b', 'c'),", + "+ `e` set('b', 'a', 'c'),", + }, }, // keys @@ -382,6 +561,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, `i` int, key `i_idx` (i))", diff: "alter table t1 add key i_idx (i)", cdiff: "ALTER TABLE `t1` ADD KEY `i_idx` (`i`)", + textdiffs: []string{ + "+ KEY `i_idx` (`i`)", + }, }, { name: "added key without name", @@ -389,6 +571,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, `i` int, key (i))", diff: "alter table t1 add key i (i)", cdiff: "ALTER TABLE `t1` ADD KEY `i` (`i`)", + textdiffs: []string{ + "+ KEY `i` (`i`)", + }, }, { name: "added key without name, conflicting name", @@ -396,6 +581,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, `i` int, key i(i), key (i))", diff: "alter table t1 add key i_2 (i)", cdiff: "ALTER TABLE `t1` ADD KEY `i_2` (`i`)", + textdiffs: []string{ + "+ KEY `i_2` (`i`)", + }, }, { name: "added key without name, conflicting name 2", @@ -403,6 +591,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, `i` int, key i(i), key i_2(i), key (i))", diff: "alter table t1 add key i_3 (i)", cdiff: "ALTER TABLE `t1` ADD KEY `i_3` (`i`)", + textdiffs: []string{ + "+ KEY `i_3` (`i`)", + }, }, { name: "added column and key", @@ -410,6 +601,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, `i` int, key `i_idx` (i))", diff: "alter table t1 add column i int, add key i_idx (i)", cdiff: "ALTER TABLE `t1` ADD COLUMN `i` int, ADD KEY `i_idx` (`i`)", + textdiffs: []string{ + "+ `i` int", + "+ KEY `i_idx` (`i`)", + }, }, { name: "modify column primary key", @@ -417,6 +612,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key)", diff: "alter table t1 add primary key (id)", cdiff: "ALTER TABLE `t1` ADD PRIMARY KEY (`id`)", + textdiffs: []string{ + "+ PRIMARY KEY (`id`)", + }, }, { name: "added primary key", @@ -424,6 +622,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int, primary key(id))", diff: "alter table t1 add primary key (id)", cdiff: "ALTER TABLE `t1` ADD PRIMARY KEY (`id`)", + textdiffs: []string{ + "+ PRIMARY KEY (`id`)", + }, }, { name: "dropped primary key", @@ -431,6 +632,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int)", diff: "alter table t1 drop primary key", cdiff: "ALTER TABLE `t1` DROP PRIMARY KEY", + textdiffs: []string{ + "- PRIMARY KEY (`id`)", + }, }, { name: "dropped key", @@ -438,6 +642,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (`id` int primary key, i int)", diff: "alter table t1 drop key i_idx", cdiff: "ALTER TABLE `t1` DROP KEY `i_idx`", + textdiffs: []string{ + "- KEY `i_idx` (`i`)", + }, }, { name: "dropped key 2", @@ -445,6 +652,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (`id` int, i int, primary key (id))", diff: "alter table t1 drop key i_idx", cdiff: "ALTER TABLE `t1` DROP KEY `i_idx`", + textdiffs: []string{ + "- KEY `i_idx` (`i`)", + }, }, { name: "modified key", @@ -452,6 +662,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (`id` int primary key, i int, key i_idx(i, id))", diff: "alter table t1 drop key i_idx, add key i_idx (i, id)", cdiff: "ALTER TABLE `t1` DROP KEY `i_idx`, ADD KEY `i_idx` (`i`, `id`)", + textdiffs: []string{ + "- KEY `i_idx` (`i`)", + "+ KEY `i_idx` (`i`, `id`)", + }, }, { name: "modified primary key", @@ -459,6 +673,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (`id` int, i int, primary key(id, i),key i_idx(`i`))", diff: "alter table t1 drop primary key, add primary key (id, i)", cdiff: "ALTER TABLE `t1` DROP PRIMARY KEY, ADD PRIMARY KEY (`id`, `i`)", + textdiffs: []string{ + "- PRIMARY KEY (`id`)", + "+ PRIMARY KEY (`id`, `i`)", + }, }, { name: "alternative primary key definition, no diff", @@ -473,6 +691,12 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (the_id int primary key, info int not null);", diff: "alter table t1 drop primary key, drop column id, add column the_id int first, add primary key (the_id)", cdiff: "ALTER TABLE `t1` DROP PRIMARY KEY, DROP COLUMN `id`, ADD COLUMN `the_id` int FIRST, ADD PRIMARY KEY (`the_id`)", + textdiffs: []string{ + "- PRIMARY KEY (`id`)", + "- `id` int,", + "+ `the_id` int,", + "+ PRIMARY KEY (`the_id`)", + }, }, { name: "reordered key, no diff", @@ -1476,27 +1700,38 @@ func TestCreateTableDiff(t *testing.T) { { eFromStatementString := eFrom.Create().CanonicalStatementString() for _, annotation := range alterEntityDiff.annotations.Removed() { - require.NotEmpty(t, annotation.line) - assert.Contains(t, eFromStatementString, annotation.line) + require.NotEmpty(t, annotation.text) + assert.Contains(t, eFromStatementString, annotation.text) } if len(alterEntityDiff.annotations.Removed()) == 0 { + assert.Empty(t, annotatedFrom.Removed()) assert.Equal(t, eFromStatementString, annotatedFromString) } else { + assert.NotEmpty(t, annotatedFrom.Removed()) assert.NotEqual(t, eFromStatementString, annotatedFromString) } } { eToStatementString := eTo.Create().CanonicalStatementString() for _, annotation := range alterEntityDiff.annotations.Added() { - require.NotEmpty(t, annotation.line) - assert.Contains(t, eToStatementString, annotation.line) + require.NotEmpty(t, annotation.text) + assert.Contains(t, eToStatementString, annotation.text) } if len(alterEntityDiff.annotations.Added()) == 0 { + assert.Empty(t, annotatedTo.Added()) assert.Equal(t, eToStatementString, annotatedToString) } else { + assert.NotEmpty(t, annotatedTo.Added()) assert.NotEqual(t, eToStatementString, annotatedToString) } } + if len(ts.textdiffs) > 0 { + // For this test, we should validate the given diffs + for _, textdiff := range ts.textdiffs { + assert.Containsf(t, annotatedUnifiedString, textdiff, annotatedUnifiedString) + } + assert.Equal(t, len(annotatedUnified.Removed())+len(annotatedUnified.Added()), len(ts.textdiffs)) + } fmt.Println("============== ") fmt.Println(">>>>>>", alter.CanonicalStatementString()) fmt.Println(annotatedFromString) From 3d458a803c5ffe34d507cd0fa607fa74cf4e61d3 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 11:35:58 +0200 Subject: [PATCH 05/22] superfluous full text annotation Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table.go | 1 + 1 file changed, 1 insertion(+) diff --git a/go/vt/schemadiff/table.go b/go/vt/schemadiff/table.go index 0b1b39f1677..0f99ab14ca9 100644 --- a/go/vt/schemadiff/table.go +++ b/go/vt/schemadiff/table.go @@ -1488,6 +1488,7 @@ func (c *CreateTableEntity) diffKeys(alterTable *sqlparser.AlterTable, // Special case: MySQL does not support multiple ADD FULLTEXT KEY statements in a single ALTER superfluousFulltextKeys = append(superfluousFulltextKeys, addKey) addedAsSuperfluousStatement = true + annotations.MarkAdded(sqlparser.CanonicalString(t2Key)) } addedFulltextKeys++ } From fd497d1c7c67859ad1179d9132b8988157350311 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 11:36:12 +0200 Subject: [PATCH 06/22] adding tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table_test.go | 115 +++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index fe2437cc508..f0a8fd9cfa0 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -724,6 +724,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (`id` int primary key, i int, key i2_idx (`i`, id), key i_idx3(id), key i_idx ( i ) )", diff: "alter table t1 add key i_idx3 (id)", cdiff: "ALTER TABLE `t1` ADD KEY `i_idx3` (`id`)", + textdiffs: []string{ + "+ KEY `i_idx3` (`id`)", + }, }, { name: "key made visible", @@ -731,6 +734,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (`id` int primary key, i int, key i_idx(i))", diff: "alter table t1 alter index i_idx visible", cdiff: "ALTER TABLE `t1` ALTER INDEX `i_idx` VISIBLE", + textdiffs: []string{ + "- KEY `i_idx` (`i`) INVISIBLE", + "+ KEY `i_idx` (`i`)", + }, }, { name: "key made invisible", @@ -738,6 +745,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (`id` int primary key, i int, key i_idx(i) invisible)", diff: "alter table t1 alter index i_idx invisible", cdiff: "ALTER TABLE `t1` ALTER INDEX `i_idx` INVISIBLE", + textdiffs: []string{ + "- KEY `i_idx` (`i`)", + "+ KEY `i_idx` (`i`) INVISIBLE", + }, }, { name: "key made invisible with different case", @@ -745,6 +756,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (`id` int primary key, i int, key i_idx(i) INVISIBLE)", diff: "alter table t1 alter index i_idx invisible", cdiff: "ALTER TABLE `t1` ALTER INDEX `i_idx` INVISIBLE", + textdiffs: []string{ + "- KEY `i_idx` (`i`)", + "+ KEY `i_idx` (`i`) INVISIBLE", + }, }, // FULLTEXT keys { @@ -753,6 +768,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key, name tinytext not null, fulltext key name_ft(name))", diff: "alter table t1 add fulltext key name_ft (`name`)", cdiff: "ALTER TABLE `t1` ADD FULLTEXT KEY `name_ft` (`name`)", + textdiffs: []string{ + "+ FULLTEXT KEY `name_ft` (`name`)", + }, }, { name: "add one fulltext key with explicit parser", @@ -760,6 +778,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key, name tinytext not null, fulltext key name_ft(name) with parser ngram)", diff: "alter table t1 add fulltext key name_ft (`name`) with parser ngram", cdiff: "ALTER TABLE `t1` ADD FULLTEXT KEY `name_ft` (`name`) WITH PARSER ngram", + textdiffs: []string{ + "+ FULLTEXT KEY `name_ft` (`name`) WITH PARSER ngram", + }, }, { name: "add one fulltext key and one normal key", @@ -767,6 +788,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key, name tinytext not null, key name_idx(name(32)), fulltext key name_ft(name))", diff: "alter table t1 add key name_idx (`name`(32)), add fulltext key name_ft (`name`)", cdiff: "ALTER TABLE `t1` ADD KEY `name_idx` (`name`(32)), ADD FULLTEXT KEY `name_ft` (`name`)", + textdiffs: []string{ + "+ KEY `name_idx` (`name`(32)),", + "+ FULLTEXT KEY `name_ft` (`name`)", + }, }, { name: "add two fulltext keys, distinct statements", @@ -774,6 +799,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key, name1 tinytext not null, name2 tinytext not null, fulltext key name1_ft(name1), fulltext key name2_ft(name2))", diffs: []string{"alter table t1 add fulltext key name1_ft (name1)", "alter table t1 add fulltext key name2_ft (name2)"}, cdiffs: []string{"ALTER TABLE `t1` ADD FULLTEXT KEY `name1_ft` (`name1`)", "ALTER TABLE `t1` ADD FULLTEXT KEY `name2_ft` (`name2`)"}, + textdiffs: []string{ + "+ FULLTEXT KEY `name1_ft` (`name1`)", + "+ FULLTEXT KEY `name2_ft` (`name2`)", + }, }, { name: "add two fulltext keys, unify statements", @@ -782,6 +811,10 @@ func TestCreateTableDiff(t *testing.T) { fulltext: FullTextKeyUnifyStatements, diff: "alter table t1 add fulltext key name1_ft (name1), add fulltext key name2_ft (name2)", cdiff: "ALTER TABLE `t1` ADD FULLTEXT KEY `name1_ft` (`name1`), ADD FULLTEXT KEY `name2_ft` (`name2`)", + textdiffs: []string{ + "+ FULLTEXT KEY `name1_ft` (`name1`)", + "+ FULLTEXT KEY `name2_ft` (`name2`)", + }, }, { name: "no fulltext diff", @@ -824,6 +857,10 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 drop check check1, add constraint chk_abc123 check (i < 5)", cdiff: "ALTER TABLE `t1` DROP CHECK `check1`, ADD CONSTRAINT `chk_abc123` CHECK (`i` < 5)", constraint: ConstraintNamesStrict, + textdiffs: []string{ + "- CONSTRAINT `check1` CHECK (`i` < 5)", + "+ CONSTRAINT `chk_abc123` CHECK (`i` < 5)", + }, }, { name: "check constraints, different name, ignore vitess, non vitess names", @@ -832,6 +869,10 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 drop check check1, add constraint chk_abc123 check (i < 5)", cdiff: "ALTER TABLE `t1` DROP CHECK `check1`, ADD CONSTRAINT `chk_abc123` CHECK (`i` < 5)", constraint: ConstraintNamesIgnoreVitess, + textdiffs: []string{ + "- CONSTRAINT `check1` CHECK (`i` < 5)", + "+ CONSTRAINT `chk_abc123` CHECK (`i` < 5)", + }, }, { name: "check constraints, different name, ignore vitess, vitess names, no match", @@ -840,6 +881,10 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 drop check check1, add constraint check2_7fp024p4rxvr858tsaggvf9dw check (i < 5)", cdiff: "ALTER TABLE `t1` DROP CHECK `check1`, ADD CONSTRAINT `check2_7fp024p4rxvr858tsaggvf9dw` CHECK (`i` < 5)", constraint: ConstraintNamesIgnoreVitess, + textdiffs: []string{ + "- CONSTRAINT `check1` CHECK (`i` < 5)", + "+ CONSTRAINT `check2_7fp024p4rxvr858tsaggvf9dw` CHECK (`i` < 5)", + }, }, { name: "check constraints, different name, ignore vitess, vitess names match", @@ -897,6 +942,9 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 add constraint check3 check (i != 3)", cdiff: "ALTER TABLE `t1` ADD CONSTRAINT `check3` CHECK (`i` != 3)", constraint: ConstraintNamesIgnoreAll, + textdiffs: []string{ + "+ CONSTRAINT `check3` CHECK (`i` != 3)", + }, }, { name: "check constraints, remove", @@ -905,6 +953,9 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 drop check check3", cdiff: "ALTER TABLE `t1` DROP CHECK `check3`", constraint: ConstraintNamesIgnoreAll, + textdiffs: []string{ + "- CONSTRAINT `check3` CHECK (`i` != 3)", + }, }, { name: "check constraints, remove duplicate", @@ -913,6 +964,9 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 drop check check3", cdiff: "ALTER TABLE `t1` DROP CHECK `check3`", constraint: ConstraintNamesIgnoreAll, + textdiffs: []string{ + "- CONSTRAINT `check3` CHECK (`i` > 2)", + }, }, { name: "check constraints, remove, ignore vitess, no match", @@ -921,6 +975,13 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 drop check chk_123abc, drop check check3, drop check chk_789def, add constraint check1 check (i < 5), add constraint check2 check (i > 2)", cdiff: "ALTER TABLE `t1` DROP CHECK `chk_123abc`, DROP CHECK `check3`, DROP CHECK `chk_789def`, ADD CONSTRAINT `check1` CHECK (`i` < 5), ADD CONSTRAINT `check2` CHECK (`i` > 2)", constraint: ConstraintNamesIgnoreVitess, + textdiffs: []string{ + "- CONSTRAINT `chk_123abc` CHECK (`i` > 2)", + "- CONSTRAINT `check3` CHECK (`i` != 3)", + "- CONSTRAINT `chk_789def` CHECK (`i` < 5)", + "+ CONSTRAINT `check1` CHECK (`i` < 5)", + "+ CONSTRAINT `check2` CHECK (`i` > 2)", + }, }, { name: "check constraints, remove, ignore vitess, match", @@ -929,6 +990,9 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 drop check check3", cdiff: "ALTER TABLE `t1` DROP CHECK `check3`", constraint: ConstraintNamesIgnoreVitess, + textdiffs: []string{ + "- CONSTRAINT `check3` CHECK (`i` != 3)", + }, }, { name: "check constraints, remove, strict", @@ -937,6 +1001,13 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 drop check chk_123abc, drop check check3, drop check chk_789def, add constraint check1 check (i < 5), add constraint check2 check (i > 2)", cdiff: "ALTER TABLE `t1` DROP CHECK `chk_123abc`, DROP CHECK `check3`, DROP CHECK `chk_789def`, ADD CONSTRAINT `check1` CHECK (`i` < 5), ADD CONSTRAINT `check2` CHECK (`i` > 2)", constraint: ConstraintNamesStrict, + textdiffs: []string{ + "- CONSTRAINT `chk_123abc` CHECK (`i` > 2)", + "- CONSTRAINT `check3` CHECK (`i` != 3)", + "- CONSTRAINT `chk_789def` CHECK (`i` < 5)", + "+ CONSTRAINT `check1` CHECK (`i` < 5)", + "+ CONSTRAINT `check2` CHECK (`i` > 2)", + }, }, // foreign keys { @@ -945,6 +1016,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, i int, key i_idex (i))", diff: "alter table t1 drop foreign key f", cdiff: "ALTER TABLE `t1` DROP FOREIGN KEY `f`", + textdiffs: []string{ + "- CONSTRAINT `f` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", + }, }, { name: "add foreign key", @@ -952,6 +1026,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, i int, key ix(i), constraint f foreign key (i) references parent(id))", diff: "alter table t1 add constraint f foreign key (i) references parent (id)", cdiff: "ALTER TABLE `t1` ADD CONSTRAINT `f` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", + textdiffs: []string{ + "+ CONSTRAINT `f` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", + }, }, { name: "add foreign key and index", @@ -959,6 +1036,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, i int, key ix(i), constraint f foreign key (i) references parent(id))", diff: "alter table t1 add key ix (i), add constraint f foreign key (i) references parent (id)", cdiff: "ALTER TABLE `t1` ADD KEY `ix` (`i`), ADD CONSTRAINT `f` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", + textdiffs: []string{ + "+ KEY `ix` (`i`)", + "+ CONSTRAINT `f` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", + }, }, { name: "identical foreign key", @@ -971,6 +1052,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, i int, key ix(i), constraint f2 foreign key (i) references parent(id) on delete cascade)", diff: "alter table t1 drop foreign key f1, add constraint f2 foreign key (i) references parent (id) on delete cascade", cdiff: "ALTER TABLE `t1` DROP FOREIGN KEY `f1`, ADD CONSTRAINT `f2` FOREIGN KEY (`i`) REFERENCES `parent` (`id`) ON DELETE CASCADE", + textdiffs: []string{ + "- CONSTRAINT `f1` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", + "+ CONSTRAINT `f2` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", + }, }, { name: "similar foreign key under different name, ignore names", @@ -984,6 +1069,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, i int, key i_idex (i), constraint f1 foreign key (i) references parent(id))", diff: "alter table t1 drop foreign key f2", cdiff: "ALTER TABLE `t1` DROP FOREIGN KEY `f2`", + textdiffs: []string{ + "- CONSTRAINT `f2` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", + }, }, { name: "two identical foreign keys, dropping one, ignore vitess names", @@ -992,6 +1080,9 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 drop foreign key f2", cdiff: "ALTER TABLE `t1` DROP FOREIGN KEY `f2`", constraint: ConstraintNamesIgnoreVitess, + textdiffs: []string{ + "- CONSTRAINT `f2` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", + }, }, { name: "two identical foreign keys, dropping one, ignore all names", @@ -1000,6 +1091,9 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 drop foreign key f2", cdiff: "ALTER TABLE `t1` DROP FOREIGN KEY `f2`", constraint: ConstraintNamesIgnoreAll, + textdiffs: []string{ + "- CONSTRAINT `f2` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", + }, }, { name: "add two identical foreign key constraints, ignore all names", @@ -1008,6 +1102,10 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 add constraint f1 foreign key (i) references parent (id), add constraint f2 foreign key (i) references parent (id)", cdiff: "ALTER TABLE `t1` ADD CONSTRAINT `f1` FOREIGN KEY (`i`) REFERENCES `parent` (`id`), ADD CONSTRAINT `f2` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", constraint: ConstraintNamesIgnoreAll, + textdiffs: []string{ + "+ CONSTRAINT `f1` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", + "+ CONSTRAINT `f2` FOREIGN KEY (`i`) REFERENCES `parent` (`id`)", + }, }, { name: "implicit foreign key indexes", @@ -1030,6 +1128,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, i int, key ix(i), constraint f foreign key (i) references parent(id) on delete set null)", diff: "alter table t1 drop foreign key f, add constraint f foreign key (i) references parent (id) on delete set null", cdiff: "ALTER TABLE `t1` DROP FOREIGN KEY `f`, ADD CONSTRAINT `f` FOREIGN KEY (`i`) REFERENCES `parent` (`id`) ON DELETE SET NULL", + textdiffs: []string{ + "- CONSTRAINT `f` FOREIGN KEY (`i`) REFERENCES `parent` (`id`) ON DELETE CASCADE", + "+ CONSTRAINT `f` FOREIGN KEY (`i`) REFERENCES `parent` (`id`) ON DELETE SET NULL", + }, }, { name: "drop and add foreign key", @@ -1037,6 +1139,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, i int, key ix(i), constraint f2 foreign key (i) references parent(id) on delete set null)", diff: "alter table t1 drop foreign key f, add constraint f2 foreign key (i) references parent (id) on delete set null", cdiff: "ALTER TABLE `t1` DROP FOREIGN KEY `f`, ADD CONSTRAINT `f2` FOREIGN KEY (`i`) REFERENCES `parent` (`id`) ON DELETE SET NULL", + textdiffs: []string{ + "- CONSTRAINT `f` FOREIGN KEY (`i`) REFERENCES `parent` (`id`) ON DELETE CASCADE", + "+ CONSTRAINT `f2` FOREIGN KEY (`i`) REFERENCES `parent` (`id`) ON DELETE SET NULL", + }, }, { name: "ignore different foreign key order", @@ -1050,6 +1156,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t2 (id int primary key, i int, key f(i))", diff: "alter table t1 drop foreign key f", cdiff: "ALTER TABLE `t1` DROP FOREIGN KEY `f`", + textdiffs: []string{ + "- CONSTRAINT `f` FOREIGN KEY (`i`) REFERENCES `parent` (`id`) ON DELETE CASCADE", + }, }, // partitions { @@ -1641,6 +1750,7 @@ func TestCreateTableDiff(t *testing.T) { t.Logf("c: %v", sqlparser.CanonicalString(c.CreateTable)) t.Logf("other: %v", sqlparser.CanonicalString(other.CreateTable)) } + assert.Empty(t, ts.textdiffs) return } @@ -1726,6 +1836,11 @@ func TestCreateTableDiff(t *testing.T) { } } if len(ts.textdiffs) > 0 { + uniqueDiffs := make(map[string]bool) + for _, textdiff := range ts.textdiffs { + uniqueDiffs[textdiff] = true + } + assert.Equal(t, len(uniqueDiffs), len(ts.textdiffs)) // For this test, we should validate the given diffs for _, textdiff := range ts.textdiffs { assert.Containsf(t, annotatedUnifiedString, textdiff, annotatedUnifiedString) From a9da1aa84d03c29d086207e5e81aa6fb29c4f3a4 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 11:37:29 +0200 Subject: [PATCH 07/22] adding tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index f0a8fd9cfa0..1bbb1326d42 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -1167,6 +1167,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key, a int) partition by hash (id) partitions 4", diff: "alter table t1 add column a int", cdiff: "ALTER TABLE `t1` ADD COLUMN `a` int", + textdiffs: []string{ + "+ `a` int", + }, }, { name: "partitioning, column case", From d072894a3d1bebe751313a6e954cb8302bcce5c4 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 11:45:14 +0200 Subject: [PATCH 08/22] adding tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table_test.go | 48 +++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index 1bbb1326d42..e0085306be3 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -1177,6 +1177,11 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key, a int) partition by hash (ID) partitions 4", diff: "alter table t1 add column a int \npartition by hash (ID) partitions 4", cdiff: "ALTER TABLE `t1` ADD COLUMN `a` int \nPARTITION BY HASH (`ID`) PARTITIONS 4", + textdiffs: []string{ + "+ `a` int", + "-PARTITION BY HASH (`id`) PARTITIONS 4", + "+PARTITION BY HASH (`ID`) PARTITIONS 4", + }, }, { name: "remove partitioning", @@ -1184,6 +1189,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key, a int)", diff: "alter table t1 add column a int remove partitioning", cdiff: "ALTER TABLE `t1` ADD COLUMN `a` int REMOVE PARTITIONING", + textdiffs: []string{ + "+ `a` int", + "-PARTITION BY HASH (`id`) PARTITIONS 4", + }, }, { name: "remove partitioning 2", @@ -1191,6 +1200,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key)", diff: "alter table t1 remove partitioning", cdiff: "ALTER TABLE `t1` REMOVE PARTITIONING", + textdiffs: []string{ + "-PARTITION BY HASH (`id`) PARTITIONS 4", + }, }, { name: "change partitioning hash", @@ -1198,6 +1210,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) partition by hash (id) partitions 5", diff: "alter table t1 \npartition by hash (id) partitions 5", cdiff: "ALTER TABLE `t1` \nPARTITION BY HASH (`id`) PARTITIONS 5", + textdiffs: []string{ + "-PARTITION BY HASH (`id`) PARTITIONS 4", + "+PARTITION BY HASH (`id`) PARTITIONS 5", + }, }, { name: "change partitioning key", @@ -1205,6 +1221,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) partition by hash (id) partitions 5", diff: "alter table t1 \npartition by hash (id) partitions 5", cdiff: "ALTER TABLE `t1` \nPARTITION BY HASH (`id`) PARTITIONS 5", + textdiffs: []string{ + "-PARTITION BY KEY (`id`) PARTITIONS 2", + "+PARTITION BY HASH (`id`) PARTITIONS 5", + }, }, { name: "change partitioning list", @@ -1212,6 +1232,12 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) partition by list (id) (partition p1 values in(11,21), partition p2 values in (12,22))", diff: "alter table t1 \npartition by list (id)\n(partition p1 values in (11, 21),\n partition p2 values in (12, 22))", cdiff: "ALTER TABLE `t1` \nPARTITION BY LIST (`id`)\n(PARTITION `p1` VALUES IN (11, 21),\n PARTITION `p2` VALUES IN (12, 22))", + textdiffs: []string{ + "-PARTITION BY KEY (`id`) PARTITIONS 2", + "+PARTITION BY LIST (`id`)", + "+(PARTITION `p1` VALUES IN (11, 21),", + "+ PARTITION `p2` VALUES IN (12, 22))", + }, }, { name: "change partitioning range: rotate", @@ -1219,6 +1245,16 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) partition by range (id) (partition p2 values less than (20), partition p3 values less than (30), partition p4 values less than (40))", diff: "alter table t1 \npartition by range (id)\n(partition p2 values less than (20),\n partition p3 values less than (30),\n partition p4 values less than (40))", cdiff: "ALTER TABLE `t1` \nPARTITION BY RANGE (`id`)\n(PARTITION `p2` VALUES LESS THAN (20),\n PARTITION `p3` VALUES LESS THAN (30),\n PARTITION `p4` VALUES LESS THAN (40))", + textdiffs: []string{ + "-PARTITION BY RANGE (`id`)", + "-(PARTITION `p1` VALUES LESS THAN (10),", + "- PARTITION `p2` VALUES LESS THAN (20),", + "- PARTITION `p3` VALUES LESS THAN (30))", + "+PARTITION BY RANGE (`id`)", + "+(PARTITION `p2` VALUES LESS THAN (20),", + "+ PARTITION `p3` VALUES LESS THAN (30),", + "+ PARTITION `p4` VALUES LESS THAN (40))", + }, }, { name: "change partitioning range: ignore rotate", @@ -1233,6 +1269,9 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationDistinctStatements, diff: "alter table t1 drop partition p1", cdiff: "ALTER TABLE `t1` DROP PARTITION `p1`", + textdiffs: []string{ + "-(PARTITION `p1` VALUES LESS THAN (10),", + }, }, { name: "change partitioning range: statements, add", @@ -1241,6 +1280,9 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationDistinctStatements, diff: "alter table t1 add partition (partition p3 values less than (30))", cdiff: "ALTER TABLE `t1` ADD PARTITION (PARTITION `p3` VALUES LESS THAN (30))", + textdiffs: []string{ + "+ PARTITION `p3` VALUES LESS THAN (30)", + }, }, { name: "change partitioning range: statements, multiple drops", @@ -1249,6 +1291,10 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationDistinctStatements, diffs: []string{"alter table t1 drop partition p1", "alter table t1 drop partition p2"}, cdiffs: []string{"ALTER TABLE `t1` DROP PARTITION `p1`", "ALTER TABLE `t1` DROP PARTITION `p2`"}, + textdiffs: []string{ + "-(PARTITION `p1` VALUES LESS THAN (10),", + "- PARTITION `p2` VALUES LESS THAN (20),", + }, }, { name: "change partitioning range: statements, multiple adds", @@ -1848,7 +1894,7 @@ func TestCreateTableDiff(t *testing.T) { for _, textdiff := range ts.textdiffs { assert.Containsf(t, annotatedUnifiedString, textdiff, annotatedUnifiedString) } - assert.Equal(t, len(annotatedUnified.Removed())+len(annotatedUnified.Added()), len(ts.textdiffs)) + assert.Equalf(t, len(annotatedUnified.Removed())+len(annotatedUnified.Added()), len(ts.textdiffs), annotatedUnifiedString) } fmt.Println("============== ") fmt.Println(">>>>>>", alter.CanonicalStatementString()) From 6c842ed6c8fc70fcfdf2fa0e8fca23aff49813c9 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 11:53:35 +0200 Subject: [PATCH 09/22] adding tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table_test.go | 86 ++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index e0085306be3..fe742ea2b1c 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -1303,6 +1303,10 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationDistinctStatements, diffs: []string{"alter table t1 add partition (partition p2 values less than (20))", "alter table t1 add partition (partition p3 values less than (30))"}, cdiffs: []string{"ALTER TABLE `t1` ADD PARTITION (PARTITION `p2` VALUES LESS THAN (20))", "ALTER TABLE `t1` ADD PARTITION (PARTITION `p3` VALUES LESS THAN (30))"}, + textdiffs: []string{ + "+ PARTITION `p2` VALUES LESS THAN (20),", + "+ PARTITION `p3` VALUES LESS THAN (30)", + }, }, { name: "change partitioning range: statements, multiple, assorted", @@ -1311,6 +1315,10 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationDistinctStatements, diffs: []string{"alter table t1 drop partition p1", "alter table t1 add partition (partition p4 values less than (40))"}, cdiffs: []string{"ALTER TABLE `t1` DROP PARTITION `p1`", "ALTER TABLE `t1` ADD PARTITION (PARTITION `p4` VALUES LESS THAN (40))"}, + textdiffs: []string{ + "-(PARTITION `p1` VALUES LESS THAN (10),", + "+ PARTITION `p4` VALUES LESS THAN (40)", + }, }, { name: "change partitioning range: mixed with nonpartition changes", @@ -1319,6 +1327,11 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationDistinctStatements, diffs: []string{"alter table t1 add column i int", "alter table t1 drop partition p1", "alter table t1 drop partition p2"}, cdiffs: []string{"ALTER TABLE `t1` ADD COLUMN `i` int", "ALTER TABLE `t1` DROP PARTITION `p1`", "ALTER TABLE `t1` DROP PARTITION `p2`"}, + textdiffs: []string{ + "+ `i` int", + "-(PARTITION `p1` VALUES LESS THAN (10),", + "- PARTITION `p2` VALUES LESS THAN (20),", + }, }, { name: "change partitioning range: single partition change, mixed with nonpartition changes", @@ -1327,6 +1340,10 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationDistinctStatements, diffs: []string{"alter table t1 add column i int", "alter table t1 drop partition p1"}, cdiffs: []string{"ALTER TABLE `t1` ADD COLUMN `i` int", "ALTER TABLE `t1` DROP PARTITION `p1`"}, + textdiffs: []string{ + "+ `i` int", + "-(PARTITION `p1` VALUES LESS THAN (10),", + }, }, { name: "change partitioning range: mixed with nonpartition changes, full spec", @@ -1335,6 +1352,15 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationFullSpec, diff: "alter table t1 add column i int \npartition by range (id)\n(partition p3 values less than (30))", cdiff: "ALTER TABLE `t1` ADD COLUMN `i` int \nPARTITION BY RANGE (`id`)\n(PARTITION `p3` VALUES LESS THAN (30))", + textdiffs: []string{ + "+ `i` int", + "-PARTITION BY RANGE (`id`)", + "-(PARTITION `p1` VALUES LESS THAN (10),", + "- PARTITION `p2` VALUES LESS THAN (20),", + "- PARTITION `p3` VALUES LESS THAN (30))", + "+PARTITION BY RANGE (`id`)", + "+(PARTITION `p3` VALUES LESS THAN (30))", + }, }, { name: "change partitioning range: ignore rotate, not a rotation", @@ -1343,6 +1369,16 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationIgnore, diff: "alter table t1 \npartition by range (id)\n(partition p2 values less than (25),\n partition p3 values less than (30),\n partition p4 values less than (40))", cdiff: "ALTER TABLE `t1` \nPARTITION BY RANGE (`id`)\n(PARTITION `p2` VALUES LESS THAN (25),\n PARTITION `p3` VALUES LESS THAN (30),\n PARTITION `p4` VALUES LESS THAN (40))", + textdiffs: []string{ + "-PARTITION BY RANGE (`id`)", + "-(PARTITION `p1` VALUES LESS THAN (10),", + "- PARTITION `p2` VALUES LESS THAN (20),", + "- PARTITION `p3` VALUES LESS THAN (30))", + "+PARTITION BY RANGE (`id`)", + "+(PARTITION `p2` VALUES LESS THAN (25)", + "+ PARTITION `p3` VALUES LESS THAN (30),", + "+ PARTITION `p4` VALUES LESS THAN (40))", + }, }, { name: "change partitioning range: ignore rotate, not a rotation 2", @@ -1351,6 +1387,16 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationIgnore, diff: "alter table t1 \npartition by range (id)\n(partition p2 values less than (20),\n partition p3 values less than (35),\n partition p4 values less than (40))", cdiff: "ALTER TABLE `t1` \nPARTITION BY RANGE (`id`)\n(PARTITION `p2` VALUES LESS THAN (20),\n PARTITION `p3` VALUES LESS THAN (35),\n PARTITION `p4` VALUES LESS THAN (40))", + textdiffs: []string{ + "-PARTITION BY RANGE (`id`)", + "-(PARTITION `p1` VALUES LESS THAN (10),", + "- PARTITION `p2` VALUES LESS THAN (20),", + "- PARTITION `p3` VALUES LESS THAN (30))", + "+PARTITION BY RANGE (`id`)", + "+(PARTITION `p2` VALUES LESS THAN (20)", + "+ PARTITION `p3` VALUES LESS THAN (35),", + "+ PARTITION `p4` VALUES LESS THAN (40))", + }, }, { name: "change partitioning range: ignore rotate, not a rotation 3", @@ -1359,6 +1405,16 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationIgnore, diff: "alter table t1 \npartition by range (id)\n(partition p2 values less than (20),\n partition pX values less than (30),\n partition p4 values less than (40))", cdiff: "ALTER TABLE `t1` \nPARTITION BY RANGE (`id`)\n(PARTITION `p2` VALUES LESS THAN (20),\n PARTITION `pX` VALUES LESS THAN (30),\n PARTITION `p4` VALUES LESS THAN (40))", + textdiffs: []string{ + "-PARTITION BY RANGE (`id`)", + "-(PARTITION `p1` VALUES LESS THAN (10),", + "- PARTITION `p2` VALUES LESS THAN (20),", + "- PARTITION `p3` VALUES LESS THAN (30))", + "+PARTITION BY RANGE (`id`)", + "+(PARTITION `p2` VALUES LESS THAN (20)", + "+ PARTITION `pX` VALUES LESS THAN (30),", + "+ PARTITION `p4` VALUES LESS THAN (40))", + }, }, { name: "change partitioning range: ignore rotate, not a rotation 4", @@ -1367,6 +1423,16 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationIgnore, diff: "alter table t1 \npartition by range (id)\n(partition pX values less than (20),\n partition p3 values less than (30),\n partition p4 values less than (40))", cdiff: "ALTER TABLE `t1` \nPARTITION BY RANGE (`id`)\n(PARTITION `pX` VALUES LESS THAN (20),\n PARTITION `p3` VALUES LESS THAN (30),\n PARTITION `p4` VALUES LESS THAN (40))", + textdiffs: []string{ + "-PARTITION BY RANGE (`id`)", + "-(PARTITION `p1` VALUES LESS THAN (10),", + "- PARTITION `p2` VALUES LESS THAN (20),", + "- PARTITION `p3` VALUES LESS THAN (30))", + "+PARTITION BY RANGE (`id`)", + "+(PARTITION `pX` VALUES LESS THAN (20)", + "+ PARTITION `p3` VALUES LESS THAN (30),", + "+ PARTITION `p4` VALUES LESS THAN (40))", + }, }, { name: "change partitioning range: ignore rotate, nothing shared", @@ -1375,6 +1441,16 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationIgnore, diff: "alter table t1 \npartition by range (id)\n(partition p4 values less than (40),\n partition p5 values less than (50),\n partition p6 values less than (60))", cdiff: "ALTER TABLE `t1` \nPARTITION BY RANGE (`id`)\n(PARTITION `p4` VALUES LESS THAN (40),\n PARTITION `p5` VALUES LESS THAN (50),\n PARTITION `p6` VALUES LESS THAN (60))", + textdiffs: []string{ + "-PARTITION BY RANGE (`id`)", + "-(PARTITION `p1` VALUES LESS THAN (10),", + "- PARTITION `p2` VALUES LESS THAN (20),", + "- PARTITION `p3` VALUES LESS THAN (30))", + "+PARTITION BY RANGE (`id`)", + "+(PARTITION `p4` VALUES LESS THAN (40)", + "+ PARTITION `p5` VALUES LESS THAN (50),", + "+ PARTITION `p6` VALUES LESS THAN (60))", + }, }, { name: "change partitioning range: ignore rotate, no names shared, definitions shared", @@ -1383,6 +1459,16 @@ func TestCreateTableDiff(t *testing.T) { rotation: RangeRotationIgnore, diff: "alter table t1 \npartition by range (id)\n(partition pA values less than (20),\n partition pB values less than (30),\n partition pC values less than (40))", cdiff: "ALTER TABLE `t1` \nPARTITION BY RANGE (`id`)\n(PARTITION `pA` VALUES LESS THAN (20),\n PARTITION `pB` VALUES LESS THAN (30),\n PARTITION `pC` VALUES LESS THAN (40))", + textdiffs: []string{ + "-PARTITION BY RANGE (`id`)", + "-(PARTITION `p1` VALUES LESS THAN (10),", + "- PARTITION `p2` VALUES LESS THAN (20),", + "- PARTITION `p3` VALUES LESS THAN (30))", + "+PARTITION BY RANGE (`id`)", + "+(PARTITION `pA` VALUES LESS THAN (20)", + "+ PARTITION `pB` VALUES LESS THAN (30),", + "+ PARTITION `pC` VALUES LESS THAN (40))", + }, }, // From 59adcb12c8c0321f98c77dd2c423ceb8feb3cc06 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 12:12:07 +0200 Subject: [PATCH 10/22] adding tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table_test.go | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index fe742ea2b1c..18384ccdc27 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -1499,6 +1499,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) row_format=compressed", diff: "alter table t1 row_format COMPRESSED", cdiff: "ALTER TABLE `t1` ROW_FORMAT COMPRESSED", + textdiffs: []string{ + "+) ROW_FORMAT COMPRESSED", + }, }, { name: "add table option 2", @@ -1506,6 +1509,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) character set=utf8, row_format=compressed", diff: "alter table t1 row_format COMPRESSED", cdiff: "ALTER TABLE `t1` ROW_FORMAT COMPRESSED", + textdiffs: []string{ + "+ ROW_FORMAT COMPRESSED", + }, }, { name: "add table option 3", @@ -1513,6 +1519,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) row_format=compressed, character set=utf8", diff: "alter table t1 row_format COMPRESSED", cdiff: "ALTER TABLE `t1` ROW_FORMAT COMPRESSED", + textdiffs: []string{ + "+) ROW_FORMAT COMPRESSED", + }, }, { name: "add table option 3", @@ -1520,6 +1529,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) row_format=compressed, character set=utf8, checksum=1", diff: "alter table t1 row_format COMPRESSED checksum 1", cdiff: "ALTER TABLE `t1` ROW_FORMAT COMPRESSED CHECKSUM 1", + textdiffs: []string{ + "+) ROW_FORMAT COMPRESSED", + "+ CHECKSUM 1", + }, }, { name: "modify table option 1", @@ -1527,6 +1540,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) character set=utf8mb4", diff: "alter table t1 charset utf8mb4", cdiff: "ALTER TABLE `t1` CHARSET utf8mb4", + textdiffs: []string{ + "-) CHARSET utf8mb3", + "+) CHARSET utf8mb4", + }, }, { name: "modify table option 2", @@ -1534,6 +1551,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) character set=utf8mb4", diff: "alter table t1 charset utf8mb4", cdiff: "ALTER TABLE `t1` CHARSET utf8mb4", + textdiffs: []string{ + "-) CHARSET utf8mb3", + "+) CHARSET utf8mb4", + }, }, { name: "modify table option 3", @@ -1541,6 +1562,10 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) charset=utf8mb4", diff: "alter table t1 charset utf8mb4", cdiff: "ALTER TABLE `t1` CHARSET utf8mb4", + textdiffs: []string{ + "-) CHARSET utf8mb3", + "+) CHARSET utf8mb4", + }, }, { name: "modify table option 4", @@ -1548,6 +1573,12 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) row_format=compressed, character set=utf8mb4, checksum=1", diff: "alter table t1 charset utf8mb4 row_format COMPRESSED checksum 1", cdiff: "ALTER TABLE `t1` CHARSET utf8mb4 ROW_FORMAT COMPRESSED CHECKSUM 1", + textdiffs: []string{ + "-) CHARSET utf8mb3", + "+) ROW_FORMAT COMPRESSED,", + "+ CHARSET utf8mb4,", + "+ CHECKSUM 1", + }, }, { name: "remove table option 1", @@ -1726,6 +1757,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) engine=innodb, character set=utf8", diff: "alter table t1 engine InnoDB", cdiff: "ALTER TABLE `t1` ENGINE InnoDB", + textdiffs: []string{ + "+) ENGINE InnoDB", + }, }, { name: "normalized ENGINE MyISAM value", @@ -1768,6 +1802,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key)", diff: "alter table t1 comment ''", cdiff: "ALTER TABLE `t1` COMMENT ''", + textdiffs: []string{ + "-) COMMENT 'foo'", + }, }, // expressions { From 8ebeada9ed15040d7138238a8d4d0bcfe2f2ace4 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 13:11:14 +0200 Subject: [PATCH 11/22] adding tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index 18384ccdc27..8c2021d3f89 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -1767,6 +1767,9 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) engine=myisam, character set=utf8", diff: "alter table t1 engine MyISAM", cdiff: "ALTER TABLE `t1` ENGINE MyISAM", + textdiffs: []string{ + "+) ENGINE MyISAM", + }, }, { name: "normalized ENGINE MEMORY value", From 8e6293983e92e502a5d34ad39f94e34e0a38b1b8 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 13:11:38 +0200 Subject: [PATCH 12/22] test cleanup Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index 8c2021d3f89..15e68a0d4a7 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -17,7 +17,6 @@ limitations under the License. package schemadiff import ( - "fmt" "strings" "testing" @@ -2022,13 +2021,6 @@ func TestCreateTableDiff(t *testing.T) { } assert.Equalf(t, len(annotatedUnified.Removed())+len(annotatedUnified.Added()), len(ts.textdiffs), annotatedUnifiedString) } - fmt.Println("============== ") - fmt.Println(">>>>>>", alter.CanonicalStatementString()) - fmt.Println(annotatedFromString) - fmt.Println(annotatedToString) - fmt.Println("-------------- unified") - fmt.Println(annotatedUnifiedString) - fmt.Println("-------------- / ") } } { From 68404999de7421b5052f95c42fe39edd3f2c893f Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 13:36:06 +0200 Subject: [PATCH 13/22] comments Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index 15e68a0d4a7..510ab5f1d15 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -2009,13 +2009,13 @@ func TestCreateTableDiff(t *testing.T) { assert.NotEqual(t, eToStatementString, annotatedToString) } } - if len(ts.textdiffs) > 0 { + if len(ts.textdiffs) > 0 { // Still incomplete. + // For this test, we should validate the given diffs uniqueDiffs := make(map[string]bool) for _, textdiff := range ts.textdiffs { uniqueDiffs[textdiff] = true } - assert.Equal(t, len(uniqueDiffs), len(ts.textdiffs)) - // For this test, we should validate the given diffs + require.Equal(t, len(uniqueDiffs), len(ts.textdiffs)) // integrity of test for _, textdiff := range ts.textdiffs { assert.Containsf(t, annotatedUnifiedString, textdiff, annotatedUnifiedString) } From f9a0b2ca56473f6252369b41874be34339e17e8f Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 13:47:09 +0200 Subject: [PATCH 14/22] Remove TextualAnnotationFormat Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/annotations.go | 19 ++++--------------- go/vt/schemadiff/table_test.go | 6 +++--- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/go/vt/schemadiff/annotations.go b/go/vt/schemadiff/annotations.go index bf5c3136140..a070ccc6547 100644 --- a/go/vt/schemadiff/annotations.go +++ b/go/vt/schemadiff/annotations.go @@ -82,7 +82,7 @@ func (a *TextualAnnotations) Removed() (r []*AnnotatedText) { return a.ByType(RemovedTextualAnnotationType) } -func (a *TextualAnnotations) Export(format TextualAnnotationFormat) string { +func (a *TextualAnnotations) Export() string { textLines := make([]string, 0, len(a.texts)) for _, annotatedText := range a.texts { switch annotatedText.typ { @@ -92,13 +92,9 @@ func (a *TextualAnnotations) Export(format TextualAnnotationFormat) string { annotatedText.text = "-" + annotatedText.text default: // text unchanged - switch format { - case PlusMinusSpaceTextualAnnotationFormat: - if a.hasChanges { - // If there is absolutely no change, we don't add a space anywhere - annotatedText.text = " " + annotatedText.text - } - case PlusMinusTextualAnnotationFormat: + if a.hasChanges { + // If there is absolutely no change, we don't add a space anywhere + annotatedText.text = " " + annotatedText.text } } textLines = append(textLines, annotatedText.text) @@ -106,13 +102,6 @@ func (a *TextualAnnotations) Export(format TextualAnnotationFormat) string { return strings.Join(textLines, "\n") } -type TextualAnnotationFormat int - -const ( - PlusMinusSpaceTextualAnnotationFormat TextualAnnotationFormat = iota - PlusMinusTextualAnnotationFormat -) - func annotatedStatement(stmt string, annotationType TextualAnnotationType, annotations *TextualAnnotations) *TextualAnnotations { stmtLines := strings.Split(stmt, "\n") result := NewTextualAnnotations() diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index 510ab5f1d15..4522f5fe5fb 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -1978,9 +1978,9 @@ func TestCreateTableDiff(t *testing.T) { alterEntityDiff, ok := alter.(*AlterTableEntityDiff) require.True(t, ok) annotatedFrom, annotatedTo, annotatedUnified := alterEntityDiff.Annotated() - annotatedFromString := annotatedFrom.Export(PlusMinusSpaceTextualAnnotationFormat) - annotatedToString := annotatedTo.Export(PlusMinusSpaceTextualAnnotationFormat) - annotatedUnifiedString := annotatedUnified.Export(PlusMinusSpaceTextualAnnotationFormat) + annotatedFromString := annotatedFrom.Export() + annotatedToString := annotatedTo.Export() + annotatedUnifiedString := annotatedUnified.Export() { eFromStatementString := eFrom.Create().CanonicalStatementString() for _, annotation := range alterEntityDiff.annotations.Removed() { From b3284bee57ca5785b567ab9f16b12a6ed17884ee Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 14:39:22 +0200 Subject: [PATCH 15/22] pre-compute mutations Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/annotations.go | 50 ++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/go/vt/schemadiff/annotations.go b/go/vt/schemadiff/annotations.go index a070ccc6547..6b99e2ea9cf 100644 --- a/go/vt/schemadiff/annotations.go +++ b/go/vt/schemadiff/annotations.go @@ -20,6 +20,7 @@ import ( "strings" ) +// TextualAnnotationType is an enum for the type of annotation that can be applied to a line of text. type TextualAnnotationType int const ( @@ -28,14 +29,17 @@ const ( RemovedTextualAnnotationType ) +// AnnotatedText is a some text and its annotation type. The text is usually single-line, but it +// can be multi-line, as in the case of partition specs. type AnnotatedText struct { text string typ TextualAnnotationType } +// TextualAnnotations is a sequence of annotated texts. It is the annotated representation of a statement. type TextualAnnotations struct { - texts []*AnnotatedText - hasChanges bool + texts []*AnnotatedText + hasAnyChanges bool } func NewTextualAnnotations() *TextualAnnotations { @@ -49,7 +53,7 @@ func (a *TextualAnnotations) Len() int { func (a *TextualAnnotations) mark(text string, typ TextualAnnotationType) { a.texts = append(a.texts, &AnnotatedText{text: text, typ: typ}) if typ != UnchangedTextualAnnotationType { - a.hasChanges = true + a.hasAnyChanges = true } } @@ -65,6 +69,7 @@ func (a *TextualAnnotations) MarkUnchanged(text string) { a.mark(text, UnchangedTextualAnnotationType) } +// ByType returns the subset of annotations by given type. func (a *TextualAnnotations) ByType(typ TextualAnnotationType) (r []*AnnotatedText) { for _, text := range a.texts { if text.typ == typ { @@ -82,6 +87,7 @@ func (a *TextualAnnotations) Removed() (r []*AnnotatedText) { return a.ByType(RemovedTextualAnnotationType) } +// Export beautifies the annotated text and returns it as a string. func (a *TextualAnnotations) Export() string { textLines := make([]string, 0, len(a.texts)) for _, annotatedText := range a.texts { @@ -92,7 +98,7 @@ func (a *TextualAnnotations) Export() string { annotatedText.text = "-" + annotatedText.text default: // text unchanged - if a.hasChanges { + if a.hasAnyChanges { // If there is absolutely no change, we don't add a space anywhere annotatedText.text = " " + annotatedText.text } @@ -102,10 +108,14 @@ func (a *TextualAnnotations) Export() string { return strings.Join(textLines, "\n") } +// annotatedStatement returns a new TextualAnnotations object that annotates the given statement with the given annotations. +// The given annotations were created by the diffing algorithm, and represent the CanonicalString of some node. +// However, the given statement is just some text, and we need to find the annotations (some of which may be multi-line) +// inside our text, and return a per-line annotation. func annotatedStatement(stmt string, annotationType TextualAnnotationType, annotations *TextualAnnotations) *TextualAnnotations { stmtLines := strings.Split(stmt, "\n") result := NewTextualAnnotations() - annotationLines := map[string]bool{} + annotationLines := map[string]bool{} // single-line breakdown of all annotations for _, annotation := range annotations.ByType(annotationType) { // An annotated text could be multiline. Partition specs are such. lines := strings.Split(annotation.text, "\n") @@ -116,6 +126,20 @@ func annotatedStatement(stmt string, annotationType TextualAnnotationType, annot } } } + annotationLinesMutations := map[string](map[string]bool){} + // Mutations are expected ways to find an annotation inside a `CREATE TABLE` statement. + for annotationLine := range annotationLines { + possibleMutations := map[string]bool{ + annotationLine: true, + ") " + annotationLine: true, // e.g. ") ENGINE=InnoDB" + ") " + annotationLine + ",": true, // e.g. ") ENGINE=InnoDB,[\n ROW_FORMAT=COMPRESSED]" + "(" + annotationLine + ")": true, // e.g. "(PARTITION p0 VALUES LESS THAN (10)) + "(" + annotationLine + ",": true, // e.g. "(PARTITION p0 VALUES LESS THAN (10), + annotationLine + ",": true, // e.g. "i int unsigned," + annotationLine + ")": true, // e.g. "PARTITION p9 VALUES LESS THAN (90))" + } + annotationLinesMutations[annotationLine] = possibleMutations + } for i := range stmtLines { lineAnnotated := false trimmedLine := strings.TrimSpace(stmtLines[i]) @@ -126,20 +150,14 @@ func annotatedStatement(stmt string, annotationType TextualAnnotationType, annot if lineAnnotated { break } - possibleMutations := map[string]bool{ - annotationLine: true, - ") " + annotationLine: true, - ") " + annotationLine + ",": true, - "(" + annotationLine: true, - "(" + annotationLine + ",": true, - annotationLine + ",": true, - annotationLine + ")": true, - } + possibleMutations := annotationLinesMutations[annotationLine] if possibleMutations[trimmedLine] { // Annotate this line! result.mark(stmtLines[i], annotationType) lineAnnotated = true - delete(annotationLines, annotationLine) // no need to iterate it in the future + // No need to match this annotation again + delete(annotationLines, annotationLine) + delete(possibleMutations, annotationLine) } } if !lineAnnotated { @@ -149,9 +167,9 @@ func annotatedStatement(stmt string, annotationType TextualAnnotationType, annot return result } +// unifiedAnnotated takes two annotations of from, to statements and returns a unified annotation. func unifiedAnnotated(from *TextualAnnotations, to *TextualAnnotations) *TextualAnnotations { unified := NewTextualAnnotations() - fromIndex := 0 toIndex := 0 for fromIndex < from.Len() || toIndex < to.Len() { From 680430f8ffa44ce2604b7d92c48b5e01575f93be Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 14:39:42 +0200 Subject: [PATCH 16/22] adding tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index 4522f5fe5fb..ba3aa81a917 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -1261,6 +1261,34 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t1 (id int primary key) partition by range (id) (partition p2 values less than (20), partition p3 values less than (30), partition p4 values less than (40))", rotation: RangeRotationIgnore, }, + { + name: "change partitioning range: don't rotate, single partition", + from: "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10))", + to: "create table t1 (id int primary key) partition by range (id) (partition p2 values less than (20))", + rotation: RangeRotationFullSpec, + diff: "alter table t1 \npartition by range (id)\n(partition p2 values less than (20))", + cdiff: "ALTER TABLE `t1` \nPARTITION BY RANGE (`id`)\n(PARTITION `p2` VALUES LESS THAN (20))", + textdiffs: []string{ + "-PARTITION BY RANGE (`id`)", + "-(PARTITION `p1` VALUES LESS THAN (10))", + "+PARTITION BY RANGE (`id`)", + "+(PARTITION `p2` VALUES LESS THAN (20))", + }, + }, + { + name: "change partitioning range: don't rotate, single partition", + from: "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10))", + to: "create table t1 (id int primary key) partition by range (id) (partition p2 values less than (20))", + rotation: RangeRotationDistinctStatements, + diff: "alter table t1 \npartition by range (id)\n(partition p2 values less than (20))", + cdiff: "ALTER TABLE `t1` \nPARTITION BY RANGE (`id`)\n(PARTITION `p2` VALUES LESS THAN (20))", + textdiffs: []string{ + "-PARTITION BY RANGE (`id`)", + "-(PARTITION `p1` VALUES LESS THAN (10))", + "+PARTITION BY RANGE (`id`)", + "+(PARTITION `p2` VALUES LESS THAN (20))", + }, + }, { name: "change partitioning range: statements, drop", from: "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10), partition p2 values less than (20), partition p3 values less than (30))", From d6f8bae901506490a899a4e045d01d921265bbe1 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Mon, 26 Feb 2024 14:39:52 +0200 Subject: [PATCH 17/22] adding tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/annotations_test.go | 69 ++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 go/vt/schemadiff/annotations_test.go diff --git a/go/vt/schemadiff/annotations_test.go b/go/vt/schemadiff/annotations_test.go new file mode 100644 index 00000000000..a8eee4015f0 --- /dev/null +++ b/go/vt/schemadiff/annotations_test.go @@ -0,0 +1,69 @@ +/* +Copyright 2024 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package schemadiff + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "vitess.io/vitess/go/vt/sqlparser" +) + +func TestUnifiedAnnotated(t *testing.T) { + tcases := []struct { + name string + from string + to string + fromAnnotations *TextualAnnotations + toAnnotations *TextualAnnotations + expected string + }{ + { + "no change", + "CREATE TABLE t1 (a int)", + "CREATE TABLE t1 (a int)", + &TextualAnnotations{}, + &TextualAnnotations{}, + "CREATE TABLE `t1` (\n\t`a` int\n)", + }, + { + "simple", + "CREATE TABLE t1 (a int)", + "CREATE TABLE t1 (a int, b int)", + &TextualAnnotations{}, + &TextualAnnotations{texts: []*AnnotatedText{{text: "`b` int", typ: AddedTextualAnnotationType}}}, + " CREATE TABLE `t1` (\n \t`a` int\n+\t`b` int\n )", + }, + } + parser := sqlparser.NewTestParser() + + for _, tcase := range tcases { + t.Run(tcase.name, func(t *testing.T) { + fromStmt, err := parser.ParseStrictDDL(tcase.from) + require.NoError(t, err) + annotatedFrom := annotatedStatement(sqlparser.CanonicalString(fromStmt), RemovedTextualAnnotationType, tcase.fromAnnotations) + toStmt, err := parser.ParseStrictDDL(tcase.to) + require.NoError(t, err) + annotatedTo := annotatedStatement(sqlparser.CanonicalString(toStmt), AddedTextualAnnotationType, tcase.toAnnotations) + unified := unifiedAnnotated(annotatedFrom, annotatedTo) + export := unified.Export() + assert.Equalf(t, tcase.expected, export, "from: %v, to: %v", annotatedFrom.Export(), annotatedTo.Export()) + }) + } +} From 3dba8681802102f56f50bf495f81d0cd947dda11 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 29 Feb 2024 12:55:51 +0200 Subject: [PATCH 18/22] annotateAll Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/annotations.go | 10 +++++++++ go/vt/schemadiff/annotations_test.go | 33 ++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/go/vt/schemadiff/annotations.go b/go/vt/schemadiff/annotations.go index 6b99e2ea9cf..4f868f96fb3 100644 --- a/go/vt/schemadiff/annotations.go +++ b/go/vt/schemadiff/annotations.go @@ -167,6 +167,16 @@ func annotatedStatement(stmt string, annotationType TextualAnnotationType, annot return result } +// annotateAll blindly annotates all lines of the given statement with the given annotation type. +func annotateAll(stmt string, annotationType TextualAnnotationType) *TextualAnnotations { + stmtLines := strings.Split(stmt, "\n") + result := NewTextualAnnotations() + for _, line := range stmtLines { + result.mark(line, annotationType) + } + return result +} + // unifiedAnnotated takes two annotations of from, to statements and returns a unified annotation. func unifiedAnnotated(from *TextualAnnotations, to *TextualAnnotations) *TextualAnnotations { unified := NewTextualAnnotations() diff --git a/go/vt/schemadiff/annotations_test.go b/go/vt/schemadiff/annotations_test.go index a8eee4015f0..8cba3c1ee90 100644 --- a/go/vt/schemadiff/annotations_test.go +++ b/go/vt/schemadiff/annotations_test.go @@ -25,6 +25,22 @@ import ( "vitess.io/vitess/go/vt/sqlparser" ) +func TestAnnotateAll(t *testing.T) { + stmt := `create table t( + id int, + name varchar(100), + primary key(id) +) engine=innodb` + annotations := annotateAll(stmt, RemovedTextualAnnotationType) + assert.Equal(t, 5, annotations.Len()) + expect := `-create table t( +- id int, +- name varchar(100), +- primary key(id) +-) engine=innodb` + assert.Equal(t, expect, annotations.Export()) +} + func TestUnifiedAnnotated(t *testing.T) { tcases := []struct { name string @@ -67,3 +83,20 @@ func TestUnifiedAnnotated(t *testing.T) { }) } } + +func TestUnifiedAnnotatedAll(t *testing.T) { + stmt := `create table t( + id int, + name varchar(100), + primary key(id) +) engine=innodb` + annotatedTo := annotateAll(stmt, AddedTextualAnnotationType) + annotatedFrom := NewTextualAnnotations() + unified := unifiedAnnotated(annotatedFrom, annotatedTo) + expect := `+create table t( ++ id int, ++ name varchar(100), ++ primary key(id) ++) engine=innodb` + assert.Equal(t, expect, unified.Export()) +} From b23bb2dca65ea9de007290cd320ccbc16ac1e306 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 29 Feb 2024 15:19:56 +0200 Subject: [PATCH 19/22] EntityDiff has Annotated() function Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/annotations.go | 27 ++++++++++++ go/vt/schemadiff/diff_test.go | 78 ++++++++++++++++++++++++++++----- go/vt/schemadiff/table.go | 18 +++++--- go/vt/schemadiff/types.go | 2 +- go/vt/schemadiff/view.go | 12 +++++ 5 files changed, 121 insertions(+), 16 deletions(-) diff --git a/go/vt/schemadiff/annotations.go b/go/vt/schemadiff/annotations.go index 4f868f96fb3..1e0f1562348 100644 --- a/go/vt/schemadiff/annotations.go +++ b/go/vt/schemadiff/annotations.go @@ -210,3 +210,30 @@ func unifiedAnnotated(from *TextualAnnotations, to *TextualAnnotations) *Textual } return unified } + +// annotatedDiff returns the annotated representations of the from and to entities, and their unified representation. +func annotatedDiff(diff EntityDiff, entityAnnotations *TextualAnnotations) (from *TextualAnnotations, to *TextualAnnotations, unified *TextualAnnotations) { + fromEntity, toEntity := diff.Entities() + switch { + case fromEntity == nil && toEntity == nil: + // Should never get here. + return nil, nil, nil + case fromEntity == nil: + // A new entity was created. + from = NewTextualAnnotations() + to = annotateAll(toEntity.Create().CanonicalStatementString(), AddedTextualAnnotationType) + case toEntity == nil: + // An entity was dropped. + from = annotateAll(fromEntity.Create().CanonicalStatementString(), RemovedTextualAnnotationType) + to = NewTextualAnnotations() + case entityAnnotations == nil: + // Entity was modified, and we have no prior info about entity annotations. Treat this is as a complete rewrite. + from = annotateAll(fromEntity.Create().CanonicalStatementString(), RemovedTextualAnnotationType) + to = annotateAll(toEntity.Create().CanonicalStatementString(), AddedTextualAnnotationType) + default: + // Entity was modified, and we have prior info about entity annotations. + from = annotatedStatement(fromEntity.Create().CanonicalStatementString(), RemovedTextualAnnotationType, entityAnnotations) + to = annotatedStatement(toEntity.Create().CanonicalStatementString(), AddedTextualAnnotationType, entityAnnotations) + } + return from, to, unifiedAnnotated(from, to) +} diff --git a/go/vt/schemadiff/diff_test.go b/go/vt/schemadiff/diff_test.go index fbe7238e3fd..a1bf46fe4cd 100644 --- a/go/vt/schemadiff/diff_test.go +++ b/go/vt/schemadiff/diff_test.go @@ -18,6 +18,7 @@ package schemadiff import ( "context" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -43,6 +44,7 @@ func TestDiffTables(t *testing.T) { expectError string hints *DiffHints env *Environment + annotated []string }{ { name: "identical", @@ -58,6 +60,9 @@ func TestDiffTables(t *testing.T) { action: "alter", fromName: "t", toName: "t", + annotated: []string{ + " CREATE TABLE `t` (", " \t`id` int,", "+\t`i` int,", " \tPRIMARY KEY (`id`)", " )", + }, }, { name: "change of columns, boolean type", @@ -106,6 +111,9 @@ func TestDiffTables(t *testing.T) { cdiff: "CREATE TABLE `t` (\n\t`id` int,\n\tPRIMARY KEY (`id`)\n)", action: "create", toName: "t", + annotated: []string{ + "+CREATE TABLE `t` (", "+\t`id` int,", "+\tPRIMARY KEY (`id`)", "+)", + }, }, { name: "drop", @@ -114,6 +122,9 @@ func TestDiffTables(t *testing.T) { cdiff: "DROP TABLE `t`", action: "drop", fromName: "t", + annotated: []string{ + "-CREATE TABLE `t` (", "-\t`id` int,", "-\tPRIMARY KEY (`id`)", "-)", + }, }, { name: "none", @@ -390,6 +401,12 @@ func TestDiffTables(t *testing.T) { _, err = env.Parser().ParseStrictDDL(canonicalDiff) assert.NoError(t, err) } + if ts.annotated != nil { + // Optional test for assorted scenarios. + _, _, unified := d.Annotated() + unifiedExport := unified.Export() + assert.Equal(t, ts.annotated, strings.Split(unifiedExport, "\n")) + } // let's also check dq, and also validate that dq's statement is identical to d's assert.NoError(t, dqerr) require.NotNil(t, dq) @@ -403,15 +420,16 @@ func TestDiffTables(t *testing.T) { func TestDiffViews(t *testing.T) { tt := []struct { - name string - from string - to string - diff string - cdiff string - fromName string - toName string - action string - isError bool + name string + from string + to string + diff string + cdiff string + fromName string + toName string + action string + isError bool + annotated []string }{ { name: "identical", @@ -427,6 +445,10 @@ func TestDiffViews(t *testing.T) { action: "alter", fromName: "v1", toName: "v1", + annotated: []string{ + "-CREATE VIEW `v1`(`col1`, `col2`, `col3`) AS SELECT `a`, `b`, `c` FROM `t`", + "+CREATE VIEW `v1`(`col1`, `col2`, `colother`) AS SELECT `a`, `b`, `c` FROM `t`", + }, }, { name: "create", @@ -435,6 +457,9 @@ func TestDiffViews(t *testing.T) { cdiff: "CREATE VIEW `v1` AS SELECT `a`, `b`, `c` FROM `t`", action: "create", toName: "v1", + annotated: []string{ + "+CREATE VIEW `v1` AS SELECT `a`, `b`, `c` FROM `t`", + }, }, { name: "drop", @@ -443,6 +468,9 @@ func TestDiffViews(t *testing.T) { cdiff: "DROP VIEW `v1`", action: "drop", fromName: "v1", + annotated: []string{ + "-CREATE VIEW `v1` AS SELECT `a`, `b`, `c` FROM `t`", + }, }, { name: "none", @@ -523,7 +551,12 @@ func TestDiffViews(t *testing.T) { _, err = env.Parser().ParseStrictDDL(canonicalDiff) assert.NoError(t, err) } - + if ts.annotated != nil { + // Optional test for assorted scenarios. + _, _, unified := d.Annotated() + unifiedExport := unified.Export() + assert.Equal(t, ts.annotated, strings.Split(unifiedExport, "\n")) + } // let's also check dq, and also validate that dq's statement is identical to d's assert.NoError(t, dqerr) require.NotNil(t, dq) @@ -545,6 +578,7 @@ func TestDiffSchemas(t *testing.T) { cdiffs []string expectError string tableRename int + annotated []string }{ { name: "identical tables", @@ -681,6 +715,9 @@ func TestDiffSchemas(t *testing.T) { cdiffs: []string{ "CREATE TABLE `t` (\n\t`id` int,\n\tPRIMARY KEY (`id`)\n)", }, + annotated: []string{ + "+CREATE TABLE `t` (\n+\t`id` int,\n+\tPRIMARY KEY (`id`)\n+)", + }, }, { name: "drop table", @@ -691,6 +728,9 @@ func TestDiffSchemas(t *testing.T) { cdiffs: []string{ "DROP TABLE `t`", }, + annotated: []string{ + "-CREATE TABLE `t` (\n-\t`id` int,\n-\tPRIMARY KEY (`id`)\n-)", + }, }, { name: "create, alter, drop tables", @@ -706,6 +746,11 @@ func TestDiffSchemas(t *testing.T) { "ALTER TABLE `t2` MODIFY COLUMN `id` bigint", "CREATE TABLE `t4` (\n\t`id` int,\n\tPRIMARY KEY (`id`)\n)", }, + annotated: []string{ + "-CREATE TABLE `t1` (\n-\t`id` int,\n-\tPRIMARY KEY (`id`)\n-)", + " CREATE TABLE `t2` (\n-\t`id` int,\n+\t`id` bigint,\n \tPRIMARY KEY (`id`)\n )", + "+CREATE TABLE `t4` (\n+\t`id` int,\n+\tPRIMARY KEY (`id`)\n+)", + }, }, { name: "identical tables: drop and create", @@ -731,6 +776,9 @@ func TestDiffSchemas(t *testing.T) { "RENAME TABLE `t2a` TO `t2b`", }, tableRename: TableRenameHeuristicStatement, + annotated: []string{ + "-CREATE TABLE `t2a` (\n-\t`id` int unsigned,\n-\tPRIMARY KEY (`id`)\n-)\n+CREATE TABLE `t2b` (\n+\t`id` int unsigned,\n+\tPRIMARY KEY (`id`)\n+)", + }, }, { name: "drop and create all", @@ -968,6 +1016,16 @@ func TestDiffSchemas(t *testing.T) { assert.NoError(t, err) } + if ts.annotated != nil { + // Optional test for assorted scenarios. + if assert.Equalf(t, len(diffs), len(ts.annotated), "%+v", cstatements) { + for i, d := range diffs { + _, _, unified := d.Annotated() + assert.Equal(t, ts.annotated[i], unified.Export()) + } + } + } + { // Validate "apply()" on "from" converges with "to" schema1, err := NewSchemaFromSQL(env, ts.from) diff --git a/go/vt/schemadiff/table.go b/go/vt/schemadiff/table.go index 0f99ab14ca9..4c971f12c21 100644 --- a/go/vt/schemadiff/table.go +++ b/go/vt/schemadiff/table.go @@ -61,11 +61,7 @@ func (d *AlterTableEntityDiff) Entities() (from Entity, to Entity) { } func (d *AlterTableEntityDiff) Annotated() (from *TextualAnnotations, to *TextualAnnotations, unified *TextualAnnotations) { - fromStatementString := d.from.Create().CanonicalStatementString() - from = annotatedStatement(fromStatementString, RemovedTextualAnnotationType, d.annotations) - toStatementString := d.to.Create().CanonicalStatementString() - to = annotatedStatement(toStatementString, AddedTextualAnnotationType, d.annotations) - return from, to, unifiedAnnotated(from, to) + return annotatedDiff(d, d.annotations) } // Statement implements EntityDiff @@ -164,6 +160,10 @@ func (d *CreateTableEntityDiff) Entities() (from Entity, to Entity) { return nil, &CreateTableEntity{CreateTable: d.createTable} } +func (d *CreateTableEntityDiff) Annotated() (from *TextualAnnotations, to *TextualAnnotations, unified *TextualAnnotations) { + return annotatedDiff(d, nil) +} + // Statement implements EntityDiff func (d *CreateTableEntityDiff) Statement() sqlparser.Statement { if d == nil { @@ -237,6 +237,10 @@ func (d *DropTableEntityDiff) Entities() (from Entity, to Entity) { return d.from, nil } +func (d *DropTableEntityDiff) Annotated() (from *TextualAnnotations, to *TextualAnnotations, unified *TextualAnnotations) { + return annotatedDiff(d, nil) +} + // Statement implements EntityDiff func (d *DropTableEntityDiff) Statement() sqlparser.Statement { if d == nil { @@ -311,6 +315,10 @@ func (d *RenameTableEntityDiff) Entities() (from Entity, to Entity) { return d.from, d.to } +func (d *RenameTableEntityDiff) Annotated() (from *TextualAnnotations, to *TextualAnnotations, unified *TextualAnnotations) { + return annotatedDiff(d, nil) +} + // Statement implements EntityDiff func (d *RenameTableEntityDiff) Statement() sqlparser.Statement { if d == nil { diff --git a/go/vt/schemadiff/types.go b/go/vt/schemadiff/types.go index 4f044badeae..becf463a09b 100644 --- a/go/vt/schemadiff/types.go +++ b/go/vt/schemadiff/types.go @@ -67,7 +67,7 @@ type EntityDiff interface { // InstantDDLCapability returns the ability of this diff to run with ALGORITHM=INSTANT InstantDDLCapability() InstantDDLCapability - // TextualAnnotation() (from string, to string) + Annotated() (from *TextualAnnotations, to *TextualAnnotations, unified *TextualAnnotations) } const ( diff --git a/go/vt/schemadiff/view.go b/go/vt/schemadiff/view.go index c28be710116..823efc241ea 100644 --- a/go/vt/schemadiff/view.go +++ b/go/vt/schemadiff/view.go @@ -45,6 +45,10 @@ func (d *AlterViewEntityDiff) Entities() (from Entity, to Entity) { return d.from, d.to } +func (d *AlterViewEntityDiff) Annotated() (from *TextualAnnotations, to *TextualAnnotations, unified *TextualAnnotations) { + return annotatedDiff(d, nil) +} + // Statement implements EntityDiff func (d *AlterViewEntityDiff) Statement() sqlparser.Statement { if d == nil { @@ -118,6 +122,10 @@ func (d *CreateViewEntityDiff) Entities() (from Entity, to Entity) { return nil, &CreateViewEntity{CreateView: d.createView} } +func (d *CreateViewEntityDiff) Annotated() (from *TextualAnnotations, to *TextualAnnotations, unified *TextualAnnotations) { + return annotatedDiff(d, nil) +} + // Statement implements EntityDiff func (d *CreateViewEntityDiff) Statement() sqlparser.Statement { if d == nil { @@ -191,6 +199,10 @@ func (d *DropViewEntityDiff) Entities() (from Entity, to Entity) { return d.from, nil } +func (d *DropViewEntityDiff) Annotated() (from *TextualAnnotations, to *TextualAnnotations, unified *TextualAnnotations) { + return annotatedDiff(d, nil) +} + // Statement implements EntityDiff func (d *DropViewEntityDiff) Statement() sqlparser.Statement { if d == nil { From fb0a2dfbe943f6653bbb8c299658685d2f37259f Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 12 Mar 2024 02:51:26 +0200 Subject: [PATCH 20/22] add collate test Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index ba3aa81a917..135a1d03d9c 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -1718,6 +1718,14 @@ func TestCreateTableDiff(t *testing.T) { to: "create table t (id int, primary key(id)) COLLATE utf8mb4_0900_ai_ci", charset: TableCharsetCollateIgnoreEmpty, }, + { + name: "non empty collate with ignore empty table collate", + from: "create table t (id int, primary key(id)) COLLATE utf8mb4_0900_bin", + to: "create table t (id int, primary key(id)) COLLATE utf8mb4_0900_ai_ci", + charset: TableCharsetCollateIgnoreEmpty, + diff: "alter table t collate utf8mb4_0900_ai_ci", + cdiff: "ALTER TABLE `t` COLLATE utf8mb4_0900_ai_ci", + }, { name: "ignore empty table charset and collate in target", from: "create table t (id int, primary key(id)) DEFAULT CHARSET = utf8mb4 COLLATE utf8mb4_0900_ai_ci", From def6771447a169de2171524028ff2023b9e97386 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 12 Mar 2024 02:56:05 +0200 Subject: [PATCH 21/22] add modify table option test Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index 135a1d03d9c..bfdb3308c72 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -1806,6 +1806,17 @@ func TestCreateTableDiff(t *testing.T) { "+) ENGINE MyISAM", }, }, + { + name: "modify ENGINE option", + from: "create table t1 (id int primary key) engine=myisam", + to: "create table t1 (id int primary key) engine=InnoDB", + diff: "alter table t1 engine InnoDB", + cdiff: "ALTER TABLE `t1` ENGINE InnoDB", + textdiffs: []string{ + "-) ENGINE MyISAM", + "+) ENGINE InnoDB", + }, + }, { name: "normalized ENGINE MEMORY value", from: "create table t1 (id int primary key) character set=utf8", From 90d6627af708e27a8dc707b4ce7141f9b051b93f Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 12 Mar 2024 03:03:38 +0200 Subject: [PATCH 22/22] test add partitioning Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/schemadiff/table_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index bfdb3308c72..0e6ef9c12b2 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -1182,6 +1182,17 @@ func TestCreateTableDiff(t *testing.T) { "+PARTITION BY HASH (`ID`) PARTITIONS 4", }, }, + { + name: "add partitioning", + from: "create table t1 (id int primary key, a int)", + to: "create table t1 (id int primary key, a int) partition by hash (id) partitions 4", + diff: "alter table t1 \npartition by hash (id) partitions 4", + cdiff: "ALTER TABLE `t1` \nPARTITION BY HASH (`id`) PARTITIONS 4", + textdiffs: []string{ + "+PARTITION BY HASH (`id`) PARTITIONS 4", + }, + }, + { name: "remove partitioning", from: "create table t1 (id int primary key) partition by hash (id) partitions 4",