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] 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 {