Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cmd/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,15 @@ var (
if err != nil {
return err
}
ignoreSequences, err := cmd.Flags().GetBool("ignore-sequences")
if err != nil {
return err
}
ddlOption := &hammer.DDLOption{
IgnoreAlterDatabase: ignoreAlterDatabase,
IgnoreChangeStreams: ignoreChangeStreams,
IgnoreModels: ignoreModels,
IgnoreSequences: ignoreSequences,
}

if hammer.Scheme(databaseURI) != "spanner" {
Expand Down Expand Up @@ -89,6 +94,7 @@ func init() {
applyCmd.Flags().Bool("ignore-alter-database", false, "ignore alter database statements")
applyCmd.Flags().Bool("ignore-change-streams", false, "ignore change streams statements")
applyCmd.Flags().Bool("ignore-models", false, "ignore model statements")
applyCmd.Flags().Bool("ignore-sequences", false, "ignore sequence statements")

rootCmd.AddCommand(applyCmd)
}
6 changes: 6 additions & 0 deletions cmd/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,15 @@ var (
if err != nil {
return err
}
ignoreSequences, err := cmd.Flags().GetBool("ignore-sequences")
if err != nil {
return err
}
ddlOption := &hammer.DDLOption{
IgnoreAlterDatabase: ignoreAlterDatabase,
IgnoreChangeStreams: ignoreChangeStreams,
IgnoreModels: ignoreModels,
IgnoreSequences: ignoreSequences,
}

if hammer.Scheme(databaseURI) != "spanner" {
Expand Down Expand Up @@ -75,6 +80,7 @@ func init() {
createCmd.Flags().Bool("ignore-alter-database", false, "ignore alter database statements")
createCmd.Flags().Bool("ignore-change-streams", false, "ignore change streams statements")
createCmd.Flags().Bool("ignore-models", false, "ignore model statements")
createCmd.Flags().Bool("ignore-sequences", false, "ignore sequence statements")

rootCmd.AddCommand(createCmd)
}
6 changes: 6 additions & 0 deletions cmd/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,15 @@ var (
if err != nil {
return err
}
ignoreSequences, err := cmd.Flags().GetBool("ignore-sequences")
if err != nil {
return err
}
ddlOption := &hammer.DDLOption{
IgnoreAlterDatabase: ignoreAlterDatabase,
IgnoreChangeStreams: ignoreChangeStreams,
IgnoreModels: ignoreModels,
IgnoreSequences: ignoreSequences,
}

source1, err := hammer.NewSource(ctx, sourceURI1)
Expand Down Expand Up @@ -88,6 +93,7 @@ func init() {
diffCmd.Flags().Bool("ignore-alter-database", false, "ignore alter database statements")
diffCmd.Flags().Bool("ignore-change-streams", false, "ignore change streams statements")
diffCmd.Flags().Bool("ignore-models", false, "ignore model statements")
diffCmd.Flags().Bool("ignore-sequences", false, "ignore sequence statements")

rootCmd.AddCommand(diffCmd)
}
6 changes: 6 additions & 0 deletions cmd/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,15 @@ var (
if err != nil {
return err
}
ignoreSequences, err := cmd.Flags().GetBool("ignore-sequences")
if err != nil {
return err
}
ddlOption := &hammer.DDLOption{
IgnoreAlterDatabase: ignoreAlterDatabase,
IgnoreChangeStreams: ignoreChangeStreams,
IgnoreModels: ignoreModels,
IgnoreSequences: ignoreSequences,
}

source, err := hammer.NewSource(ctx, sourceURI)
Expand All @@ -68,6 +73,7 @@ func init() {
exportCmd.Flags().Bool("ignore-alter-database", false, "ignore alter database statements")
exportCmd.Flags().Bool("ignore-change-streams", false, "ignore change streams statements")
exportCmd.Flags().Bool("ignore-models", false, "ignore model statements")
exportCmd.Flags().Bool("ignore-sequences", false, "ignore sequence statements")

rootCmd.AddCommand(exportCmd)
}
3 changes: 3 additions & 0 deletions internal/hammer/ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ func ParseDDL(uri, schema string, option *DDLOption) (DDL, error) {
if _, ok := stmt.(*ast.CreateModel); ok && option.IgnoreModels {
continue
}
if _, ok := stmt.(*ast.CreateSequence); ok && option.IgnoreSequences {
continue
}
list = append(list, stmt)
}
return DDL{List: list}, nil
Expand Down
29 changes: 29 additions & 0 deletions internal/hammer/ddl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,35 @@ OPTIONS (
want: `CREATE TABLE Users (
UserID STRING(10) NOT NULL,
Name STRING(10) NOT NULL
) PRIMARY KEY (UserID);`,
},
{
name: "parse sequence",
schema: `CREATE TABLE Users (
UserID STRING(10) NOT NULL,
) PRIMARY KEY(UserID);

CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive");
`,
option: &hammer.DDLOption{},
want: `CREATE TABLE Users (
UserID STRING(10) NOT NULL
) PRIMARY KEY (UserID);
CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive");`,
},
{
name: "Ignore sequences",
schema: `CREATE TABLE Users (
UserID STRING(10) NOT NULL,
) PRIMARY KEY(UserID);

CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive");
`,
option: &hammer.DDLOption{
IgnoreSequences: true,
},
want: `CREATE TABLE Users (
UserID STRING(10) NOT NULL
) PRIMARY KEY (UserID);`,
},
}
Expand Down
94 changes: 93 additions & 1 deletion internal/hammer/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func Diff(ddl1, ddl2 DDL) (DDL, error) {
func NewDatabase(ddl DDL) (*Database, error) {
var (
tables []*Table
sequences []*Sequence
changeStreams []*ChangeStream
views []*View
roles []*Role
Expand Down Expand Up @@ -74,6 +75,8 @@ func NewDatabase(ddl DDL) (*Database, error) {
case *ast.AlterDatabase:
alterDatabaseOptions = stmt
options = stmt.Options
case *ast.CreateSequence:
sequences = append(sequences, &Sequence{CreateSequence: stmt})
case *ast.CreateChangeStream:
switch forType := stmt.For.(type) {
case *ast.ChangeStreamForTables:
Expand Down Expand Up @@ -106,11 +109,12 @@ func NewDatabase(ddl DDL) (*Database, error) {
}
}

return &Database{tables: tables, changeStreams: changeStreams, views: views, roles: roles, grants: grants, alterDatabaseOptions: alterDatabaseOptions, options: options}, nil
return &Database{tables: tables, sequences: sequences, changeStreams: changeStreams, views: views, roles: roles, grants: grants, alterDatabaseOptions: alterDatabaseOptions, options: options}, nil
}

type Database struct {
tables []*Table
sequences []*Sequence
changeStreams []*ChangeStream
views []*View
roles []*Role
Expand Down Expand Up @@ -224,6 +228,10 @@ type Grant struct {
*ast.Grant
}

type Sequence struct {
*ast.CreateSequence
}

type ChangeStream struct {
*ast.CreateChangeStream
}
Expand Down Expand Up @@ -255,6 +263,7 @@ type Generator struct {
dropedTable []string
dropedIndex []string
dropedChangeStream []string
droppedSequence []string
droppedConstraints []*ast.TableConstraint
droppedGrant []*Grant
willCreateOrAlterChangeStreamIDs map[string]*ChangeStream
Expand All @@ -267,6 +276,17 @@ func (g *Generator) GenerateDDL() DDL {
// for alter database
ddl.AppendDDL(g.generateDDLForAlterDatabaseOptions())

// for sequences (CREATE and ALTER before tables, since tables may reference sequences)
for _, toSeq := range g.to.sequences {
name := identsToComparable(toSeq.Name.Idents...)
fromSeq, exists := g.findSequenceByName(g.from.sequences, name)
if !exists {
ddl.Append(toSeq.CreateSequence)
continue
}
ddl.AppendDDL(g.generateDDLForAlterSequence(fromSeq, toSeq))
}

// for alter table
for _, toTable := range g.to.tables {
fromTable, exists := g.findTableByName(g.from.tables, identsToComparable(toTable.Name.Idents...))
Expand Down Expand Up @@ -351,6 +371,18 @@ func (g *Generator) GenerateDDL() DDL {
}
}
}
// drop sequences (after table drops, since tables may reference sequences)
for _, fromSeq := range g.from.sequences {
name := identsToComparable(fromSeq.Name.Idents...)
if g.isDroppedSequence(name) {
continue
}
if _, exists := g.findSequenceByName(g.to.sequences, name); !exists {
ddl.Append(&ast.DropSequence{Name: fromSeq.Name})
g.droppedSequence = append(g.droppedSequence, name)
}
}

// for views
for _, toView := range g.to.views {
_, exists := g.findViewByName(g.from.views, identsToComparable(toView.Name.Idents...))
Expand Down Expand Up @@ -982,6 +1014,15 @@ func (g *Generator) isDropedChangeStream(name string) bool {
return false
}

func (g *Generator) isDroppedSequence(name string) bool {
for _, s := range g.droppedSequence {
if s == name {
return true
}
}
return false
}

func (g *Generator) isDroppedGrant(grant *Grant) bool {
for _, dg := range g.droppedGrant {
if equalGrant(dg, grant) {
Expand Down Expand Up @@ -1312,6 +1353,57 @@ func (g *Generator) findChangeStreamByName(database *Database, name string) (cha
return
}

func (g *Generator) findSequenceByName(sequences []*Sequence, name string) (seq *Sequence, exists bool) {
for _, s := range sequences {
if strings.EqualFold(identsToComparable(s.Name.Idents...), name) {
seq = s
exists = true
break
}
}
return
}

func (g *Generator) sequenceParamsEqual(x, y []ast.SequenceParam) bool {
return cmp.Equal(x, y, cmpopts.IgnoreTypes(token.Pos(0)))
}

func (g *Generator) generateDDLForAlterSequence(from, to *Sequence) DDL {
ddl := DDL{}

// If params changed, we need DROP + CREATE
if !g.sequenceParamsEqual(from.Params, to.Params) {
ddl.Append(&ast.DropSequence{Name: from.Name})
ddl.Append(to.CreateSequence)
g.droppedSequence = append(g.droppedSequence, identsToComparable(from.Name.Idents...))
return ddl
}
Comment on lines +1374 to +1380
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ast.SequenceParam can be translated into

DROP and CREATE sequence is not safe operation because sequences maintain their states. If it is needed, flag like --allow-recreate may be considerable choice. It is same as spanner-schema-diff-tool

By default, changes to indexes will also cause a failure. The
--allowRecreateIndexes command line option enables index changes by
generating statements to drop and recreate the index.

By default, changes to foreign key constraints will also cause a failure. The
--allowRecreateForeignKeys command line option enables foreign key changes by
generating statements to drop and recreate the constraint.

    --allowDropStatements         Enables output of DROP commands to delete
                                  columns, tables or indexes not used in the
                                  new DDL file.
    --allowRecreateForeignKeys    Allows dropping and recreating Foreign Keys
                                  (and their backing Indexes) to apply changes.
    --allowRecreateIndexes        Allows dropping and recreating secondary
                                  Indexes to apply changes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the comment. I agree with your point. I'll also check the specification of spanner-schema-diff-tool that you mentioned.


// If only options changed, use ALTER SEQUENCE SET OPTIONS
if !g.optionsEqual(from.Options, to.Options) {
options := to.Options
if options == nil {
options = &ast.Options{}
}
if from.Options != nil {
for _, r := range from.Options.Records {
if optionsValueFromName(to.Options, r.Name.Name) == nil {
options.Records = append(options.Records, &ast.OptionsDef{
Name: &ast.Ident{Name: r.Name.Name},
Value: &ast.NullLiteral{},
})
}
}
}
ddl.Append(&ast.AlterSequence{
Name: to.Name,
Options: options,
})
}

return ddl
}

func (g *Generator) generateDDLForAlterChangeStream(from, to *ChangeStream) DDL {
ddl := DDL{}
fromType, toType := from.Type(), to.Type()
Expand Down
89 changes: 89 additions & 0 deletions internal/hammer/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2634,6 +2634,95 @@ CREATE CHANGE STREAM CS2 FOR t2;
"DROP TABLE t1",
},
},
// sequence tests
{
name: "create sequence",
from: ``,
to: `
CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive");
`,
ignoreAlterDatabase: true,
expected: []string{
`CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive")`,
},
},
{
name: "drop sequence",
from: `
CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive");
`,
to: ``,
ignoreAlterDatabase: true,
expected: []string{
"DROP SEQUENCE MySeq",
},
},
{
name: "no change sequence",
from: `
CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive");
`,
to: `
CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive");
`,
ignoreAlterDatabase: true,
expected: []string{},
},
{
name: "alter sequence options",
from: `
CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive", start_with_counter = 1000);
`,
to: `
CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive", start_with_counter = 5000);
`,
ignoreAlterDatabase: true,
expected: []string{
`ALTER SEQUENCE MySeq SET OPTIONS (sequence_kind = "bit_reversed_positive", start_with_counter = 5000)`,
},
},
{
name: "alter sequence remove options",
from: `
CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive", start_with_counter = 1000);
`,
to: `
CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive");
`,
ignoreAlterDatabase: true,
expected: []string{
`ALTER SEQUENCE MySeq SET OPTIONS (sequence_kind = "bit_reversed_positive", start_with_counter = null)`,
},
},
{
name: "alter sequence params change drop and create",
from: `
CREATE SEQUENCE MySeq BIT_REVERSED_POSITIVE OPTIONS (sequence_kind = "bit_reversed_positive");
`,
to: `
CREATE SEQUENCE MySeq BIT_REVERSED_POSITIVE SKIP RANGE 1, 1000 OPTIONS (sequence_kind = "bit_reversed_positive");
`,
ignoreAlterDatabase: true,
expected: []string{
"DROP SEQUENCE MySeq",
`CREATE SEQUENCE MySeq BIT_REVERSED_POSITIVESKIP RANGE 1, 1000 OPTIONS (sequence_kind = "bit_reversed_positive")`,
},
},
{
name: "sequence created before table",
from: ``,
to: `
CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive");
CREATE TABLE t1 (
id INT64 NOT NULL DEFAULT (GET_NEXT_SEQUENCE_VALUE(SEQUENCE MySeq)),
) PRIMARY KEY(id);
`,
ignoreAlterDatabase: true,
expected: []string{
`CREATE SEQUENCE MySeq OPTIONS (sequence_kind = "bit_reversed_positive")`,
"CREATE TABLE t1 (\n id INT64 NOT NULL DEFAULT (GET_NEXT_SEQUENCE_VALUE(SEQUENCE MySeq))\n) PRIMARY KEY (id)",
},
},
}
for _, v := range values {
t.Run(v.name, func(t *testing.T) {
Expand Down
Loading