From 0be9536877d440d7bdfd688f816771aaa54ca030 Mon Sep 17 00:00:00 2001 From: Joseph Watson Date: Sun, 2 Apr 2023 02:00:26 -0400 Subject: [PATCH 1/5] fix(spanner): Fix issue where a provided spanner client is closed by Migrate.Close() --- database/spanner/spanner.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/database/spanner/spanner.go b/database/spanner/spanner.go index b733302d5..7711399c2 100644 --- a/database/spanner/spanner.go +++ b/database/spanner/spanner.go @@ -68,14 +68,16 @@ type Spanner struct { } type DB struct { - admin *sdb.DatabaseAdminClient - data *spanner.Client + admin *sdb.DatabaseAdminClient + data *spanner.Client + shared bool } func NewDB(admin sdb.DatabaseAdminClient, data spanner.Client) *DB { return &DB{ - admin: &admin, - data: &data, + admin: &admin, + data: &data, + shared: true, } } @@ -146,6 +148,9 @@ func (s *Spanner) Open(url string) (database.Driver, error) { // Close implements database.Driver func (s *Spanner) Close() error { + if s.db.shared { + return nil + } s.db.data.Close() return s.db.admin.Close() } From ef1b809d3295494426a658b92efb3ac2e93c7e26 Mon Sep 17 00:00:00 2001 From: Joseph Watson Date: Fri, 29 Nov 2024 01:11:48 -0500 Subject: [PATCH 2/5] feature(spanner): Implement support for DML --- database/spanner/README.md | 10 +- .../1481574547_create_users_table.down.sql | 1 + .../1481574547_create_users_table.up.sql | 15 ++ .../1496539702_add_city_to_users.down.sql | 1 + .../1496539702_add_city_to_users.up.sql | 1 + database/spanner/spanner.go | 149 +++++++++++++--- database/spanner/spanner_test.go | 164 +++++++++++++++--- go.mod | 1 + go.sum | 4 + 9 files changed, 292 insertions(+), 54 deletions(-) create mode 100644 database/spanner/examples/migrations2/1481574547_create_users_table.down.sql create mode 100644 database/spanner/examples/migrations2/1481574547_create_users_table.up.sql create mode 100644 database/spanner/examples/migrations2/1496539702_add_city_to_users.down.sql create mode 100644 database/spanner/examples/migrations2/1496539702_add_city_to_users.up.sql diff --git a/database/spanner/README.md b/database/spanner/README.md index dda7fe9b0..a0ba3150c 100644 --- a/database/spanner/README.md +++ b/database/spanner/README.md @@ -30,17 +30,11 @@ as described in [README.md#database-urls](../../README.md#database-urls) > 1496601752/u add_index_on_user_emails (2m12.155787369s) > 1496602638/u create_books_table (2m30.77299181s) -## DDL with comments +## DDL & DML with comments At the moment the GCP Spanner backed does not seem to allow for comments (See https://issuetracker.google.com/issues/159730604) so in order to be able to use migration with DDL containing comments `x-clean-statements` is required ## Multiple statements -In order to be able to use more than 1 DDL statement in the same migration file, the file has to be parsed and therefore the `x-clean-statements` flag is required - -## Testing - -To unit test the `spanner` driver, `SPANNER_DATABASE` needs to be set. You'll -need to sign-up to Google Cloud Platform (GCP) and have a running Spanner -instance since it is not possible to run Google Spanner outside GCP. +In order to be able to use more than 1 DDL or DML statement in the same migration file, the file has to be parsed and therefore the `x-clean-statements` flag is required diff --git a/database/spanner/examples/migrations2/1481574547_create_users_table.down.sql b/database/spanner/examples/migrations2/1481574547_create_users_table.down.sql new file mode 100644 index 000000000..fe6a7f589 --- /dev/null +++ b/database/spanner/examples/migrations2/1481574547_create_users_table.down.sql @@ -0,0 +1 @@ +DROP TABLE Users; \ No newline at end of file diff --git a/database/spanner/examples/migrations2/1481574547_create_users_table.up.sql b/database/spanner/examples/migrations2/1481574547_create_users_table.up.sql new file mode 100644 index 000000000..82a3f31b1 --- /dev/null +++ b/database/spanner/examples/migrations2/1481574547_create_users_table.up.sql @@ -0,0 +1,15 @@ +-- Create a table +CREATE TABLE Users ( + UserId INT64, + Name STRING(40), + Email STRING(83) +) PRIMARY KEY(UserId /* even inline comments */); + +CREATE UNIQUE INDEX UsersEmailIndex ON Users (Email); + +-- Comments are okay + +INSERT INTO Users(UserId, Name, Email) + VALUES + (100, "Username", "email@domain.com"), + (200, "Username2", "email2@domain.com"); \ No newline at end of file diff --git a/database/spanner/examples/migrations2/1496539702_add_city_to_users.down.sql b/database/spanner/examples/migrations2/1496539702_add_city_to_users.down.sql new file mode 100644 index 000000000..3c120386b --- /dev/null +++ b/database/spanner/examples/migrations2/1496539702_add_city_to_users.down.sql @@ -0,0 +1 @@ +ALTER TABLE Users DROP COLUMN city; \ No newline at end of file diff --git a/database/spanner/examples/migrations2/1496539702_add_city_to_users.up.sql b/database/spanner/examples/migrations2/1496539702_add_city_to_users.up.sql new file mode 100644 index 000000000..d81275291 --- /dev/null +++ b/database/spanner/examples/migrations2/1496539702_add_city_to_users.up.sql @@ -0,0 +1 @@ +ALTER TABLE Users ADD COLUMN city STRING(100); \ No newline at end of file diff --git a/database/spanner/spanner.go b/database/spanner/spanner.go index 7711399c2..a1dd08b08 100644 --- a/database/spanner/spanner.go +++ b/database/spanner/spanner.go @@ -13,8 +13,9 @@ import ( "cloud.google.com/go/spanner" sdb "cloud.google.com/go/spanner/admin/database/apiv1" - "cloud.google.com/go/spanner/spansql" + "github.com/cloudspannerecosystem/memefish" + "github.com/cloudspannerecosystem/memefish/token" "github.com/golang-migrate/migrate/v4" "github.com/golang-migrate/migrate/v4/database" @@ -60,11 +61,9 @@ type Config struct { // Spanner implements database.Driver for Google Cloud Spanner type Spanner struct { - db *DB - + db *DB config *Config - - lock *uatomic.Uint32 + lock *uatomic.Uint32 } type DB struct { @@ -179,26 +178,65 @@ func (s *Spanner) Run(migration io.Reader) error { return err } - stmts := []string{string(migr)} - if s.config.CleanStatements { - stmts, err = cleanStatements(migr) - if err != nil { - return err + ctx := context.Background() + + if !s.config.CleanStatements { + return s.runDdl(ctx, []string{string(migr)}) + } + + stmtGroups, err := statementGroups(migr) + if err != nil { + return err + } + + for _, group := range stmtGroups { + switch group.typ { + case statementTypeDDL: + if err := s.runDdl(ctx, group.stmts); err != nil { + return err + } + case statementTypeDML: + if err := s.runDml(ctx, group.stmts); err != nil { + return err + } + default: + return fmt.Errorf("unknown statement type: %s", group.typ) } } - ctx := context.Background() + return nil +} + +func (s *Spanner) runDdl(ctx context.Context, stmts []string) error { op, err := s.db.admin.UpdateDatabaseDdl(ctx, &adminpb.UpdateDatabaseDdlRequest{ Database: s.config.DatabaseName, Statements: stmts, }) if err != nil { - return &database.Error{OrigErr: err, Err: "migration failed", Query: migr} + return &database.Error{OrigErr: err, Err: "migration failed", Query: []byte(strings.Join(stmts, ";\n"))} } if err := op.Wait(ctx); err != nil { - return &database.Error{OrigErr: err, Err: "migration failed", Query: migr} + return &database.Error{OrigErr: err, Err: "migration failed", Query: []byte(strings.Join(stmts, ";\n"))} + } + + return nil +} + +func (s *Spanner) runDml(ctx context.Context, stmts []string) error { + _, err := s.db.data.ReadWriteTransaction(ctx, + func(ctx context.Context, txn *spanner.ReadWriteTransaction) error { + for _, s := range stmts { + _, err := txn.Update(ctx, spanner.Statement{SQL: s}) + if err != nil { + return err + } + } + return nil + }) + if err != nil { + return &database.Error{OrigErr: err, Err: "migration failed", Query: []byte(strings.Join(stmts, ";\n"))} } return nil @@ -345,17 +383,80 @@ func (s *Spanner) ensureVersionTable() (err error) { return nil } -func cleanStatements(migration []byte) ([]string, error) { - // The Spanner GCP backend does not yet support comments for the UpdateDatabaseDdl RPC - // (see https://issuetracker.google.com/issues/159730604) we use - // spansql to parse the DDL and output valid stamements without comments - ddl, err := spansql.ParseDDL("", string(migration)) - if err != nil { - return nil, err +type statementType string + +const ( + statementTypeUnknown statementType = "" + statementTypeDDL statementType = "DDL" + statementTypeDML statementType = "DML" +) + +type statementGroup struct { + typ statementType + stmts []string +} + +func statementGroups(migr []byte) (groups []*statementGroup, err error) { + lex := &memefish.Lexer{ + File: &token.File{Buffer: string(migr)}, } - stmts := make([]string, 0, len(ddl.List)) - for _, stmt := range ddl.List { - stmts = append(stmts, stmt.SQL()) + + group := &statementGroup{} + var stmtTyp statementType + var stmt strings.Builder + for { + if err := lex.NextToken(); err != nil { + return nil, err + } + + if stmtTyp == statementTypeUnknown { + switch { + case lex.Token.IsKeywordLike("INSERT") || lex.Token.IsKeywordLike("DELETE") || lex.Token.IsKeywordLike("UPDATE"): + stmtTyp = statementTypeDML + default: + stmtTyp = statementTypeDDL + } + if group.typ != stmtTyp { + if len(group.stmts) > 0 { + groups = append(groups, group) + } + group = &statementGroup{typ: stmtTyp} + } + } + + if lex.Token.Kind == token.TokenEOF || lex.Token.Kind == ";" { + if stmt.Len() > 0 { + group.stmts = append(group.stmts, stmt.String()) + } + stmtTyp = statementTypeUnknown + stmt.Reset() + + if lex.Token.Kind == token.TokenEOF { + if len(group.stmts) > 0 { + groups = append(groups, group) + } + + break + } + + continue + } + + if len(lex.Token.Comments) > 0 { + // preserve newline where comments are removed + if _, err := stmt.WriteString("\n"); err != nil { + return nil, err + } + } + if stmt.Len() > 0 { + if _, err := stmt.WriteString(lex.Token.Space); err != nil { + return nil, err + } + } + if _, err := stmt.WriteString(lex.Token.Raw); err != nil { + return nil, err + } } - return stmts, nil + + return groups, nil } diff --git a/database/spanner/spanner_test.go b/database/spanner/spanner_test.go index d6ab4db32..cbacd2d96 100644 --- a/database/spanner/spanner_test.go +++ b/database/spanner/spanner_test.go @@ -43,6 +43,16 @@ func Test(t *testing.T) { } dt.Test(t, d, []byte("CREATE TABLE test (id BOOL) PRIMARY KEY (id)")) }) + + withSpannerEmulator(t, func(t *testing.T) { + uri := fmt.Sprintf("spanner://%s?x-clean-statements=true", db) + s := &Spanner{} + d, err := s.Open(uri) + if err != nil { + t.Fatal(err) + } + dt.Test(t, d, []byte("CREATE TABLE table_name (\n id STRING(MAX) NOT NULL,\n) PRIMARY KEY (id);\nINSERT INTO table_name (id) VALUES ('1');")) + }) } func TestMigrate(t *testing.T) { @@ -59,42 +69,84 @@ func TestMigrate(t *testing.T) { } dt.TestMigrate(t, m) }) + + withSpannerEmulator(t, func(t *testing.T) { + s := &Spanner{} + uri := fmt.Sprintf("spanner://%s?x-clean-statements=true", db) + d, err := s.Open(uri) + if err != nil { + t.Fatal(err) + } + m, err := migrate.NewWithDatabaseInstance("file://./examples/migrations2", uri, d) + if err != nil { + t.Fatal(err) + } + dt.TestMigrate(t, m) + }) } -func TestCleanStatements(t *testing.T) { +func Test_statementGroups(t *testing.T) { testCases := []struct { name string multiStatement string - expected []string + expected []*statementGroup }{ { name: "no statement", multiStatement: "", - expected: []string{}, + expected: nil, }, { name: "single statement, single line, no semicolon, no comment", multiStatement: "CREATE TABLE table_name (id STRING(255) NOT NULL) PRIMARY KEY (id)", - expected: []string{"CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n) PRIMARY KEY(id)"}, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE TABLE table_name (id STRING(255) NOT NULL) PRIMARY KEY (id)", + }, + }, + }, }, { name: "single statement, multi line, no semicolon, no comment", multiStatement: `CREATE TABLE table_name ( id STRING(255) NOT NULL, ) PRIMARY KEY (id)`, - expected: []string{"CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n) PRIMARY KEY(id)"}, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n ) PRIMARY KEY (id)", + }, + }, + }, }, { name: "single statement, single line, with semicolon, no comment", multiStatement: "CREATE TABLE table_name (id STRING(255) NOT NULL) PRIMARY KEY (id);", - expected: []string{"CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n) PRIMARY KEY(id)"}, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE TABLE table_name (id STRING(255) NOT NULL) PRIMARY KEY (id)", + }, + }, + }, }, { name: "single statement, multi line, with semicolon, no comment", multiStatement: `CREATE TABLE table_name ( id STRING(255) NOT NULL, ) PRIMARY KEY (id);`, - expected: []string{"CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n) PRIMARY KEY(id)"}, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n ) PRIMARY KEY (id)", + }, + }, + }, }, { name: "multi statement, with trailing semicolon. no comment", @@ -104,9 +156,15 @@ func TestCleanStatements(t *testing.T) { ) PRIMARY KEY(id); CREATE INDEX table_name_id_idx ON table_name (id);`, - expected: []string{`CREATE TABLE table_name ( - id STRING(255) NOT NULL, -) PRIMARY KEY(id)`, "CREATE INDEX table_name_id_idx ON table_name(id)"}, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n ) PRIMARY KEY(id)", + "CREATE INDEX table_name_id_idx ON table_name (id)", + }, + }, + }, }, { name: "multi statement, no trailing semicolon, no comment", @@ -116,9 +174,15 @@ func TestCleanStatements(t *testing.T) { ) PRIMARY KEY(id); CREATE INDEX table_name_id_idx ON table_name (id)`, - expected: []string{`CREATE TABLE table_name ( - id STRING(255) NOT NULL, -) PRIMARY KEY(id)`, "CREATE INDEX table_name_id_idx ON table_name(id)"}, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n ) PRIMARY KEY(id)", + "CREATE INDEX table_name_id_idx ON table_name (id)", + }, + }, + }, }, { name: "multi statement, no trailing semicolon, standalone comment", @@ -129,9 +193,15 @@ func TestCleanStatements(t *testing.T) { ) PRIMARY KEY(id); CREATE INDEX table_name_id_idx ON table_name (id)`, - expected: []string{`CREATE TABLE table_name ( - id STRING(255) NOT NULL, -) PRIMARY KEY(id)`, "CREATE INDEX table_name_id_idx ON table_name(id)"}, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n ) PRIMARY KEY(id)", + "CREATE INDEX table_name_id_idx ON table_name (id)", + }, + }, + }, }, { name: "multi statement, no trailing semicolon, inline comment", @@ -141,15 +211,28 @@ func TestCleanStatements(t *testing.T) { ) PRIMARY KEY(id); CREATE INDEX table_name_id_idx ON table_name (id)`, - expected: []string{`CREATE TABLE table_name ( - id STRING(255) NOT NULL, -) PRIMARY KEY(id)`, "CREATE INDEX table_name_id_idx ON table_name(id)"}, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n ) PRIMARY KEY(id)", + "CREATE INDEX table_name_id_idx ON table_name (id)", + }, + }, + }, }, { name: "alter table with SET OPTIONS", multiStatement: `ALTER TABLE users ALTER COLUMN created SET OPTIONS (allow_commit_timestamp=true);`, - expected: []string{"ALTER TABLE users ALTER COLUMN created SET OPTIONS (allow_commit_timestamp = true)"}, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "ALTER TABLE users ALTER COLUMN created\n SET OPTIONS (allow_commit_timestamp=true)", + }, + }, + }, }, { name: "column with NUMERIC type", @@ -157,13 +240,50 @@ func TestCleanStatements(t *testing.T) { id STRING(255) NOT NULL, sum NUMERIC, ) PRIMARY KEY (id)`, - expected: []string{"CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n sum NUMERIC,\n) PRIMARY KEY(id)"}, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n sum NUMERIC,\n ) PRIMARY KEY (id)", + }, + }, + }, + }, + { + name: "DML statement", + multiStatement: "INSERT INTO table_name (id) VALUES ('1');", + expected: []*statementGroup{ + { + typ: statementTypeDML, + stmts: []string{ + "INSERT INTO table_name (id) VALUES ('1')", + }, + }, + }, + }, + { + name: "DDL & DML statement", + multiStatement: "CREATE TABLE table_name (\n id STRING(MAX) NOT NULL,\n) PRIMARY KEY (id);\nINSERT INTO table_name (id) VALUES ('1');", + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE TABLE table_name (\n id STRING(MAX) NOT NULL,\n) PRIMARY KEY (id)", + }, + }, + { + typ: statementTypeDML, + stmts: []string{ + "INSERT INTO table_name (id) VALUES ('1')", + }, + }, + }, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - stmts, err := cleanStatements([]byte(tc.multiStatement)) + stmts, err := statementGroups([]byte(tc.multiStatement)) require.NoError(t, err, "Error cleaning statements") assert.Equal(t, tc.expected, stmts) }) diff --git a/go.mod b/go.mod index 3c20151f2..e2a75a468 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/ClickHouse/clickhouse-go v1.4.3 github.com/aws/aws-sdk-go v1.49.6 github.com/cenkalti/backoff/v4 v4.1.2 + github.com/cloudspannerecosystem/memefish v0.0.0-20241126143851-bb8d10117087 github.com/cockroachdb/cockroach-go/v2 v2.1.1 github.com/dhui/dktest v0.4.5 github.com/docker/docker v27.2.0+incompatible diff --git a/go.sum b/go.sum index e30a51a2a..a52293678 100644 --- a/go.sum +++ b/go.sum @@ -65,6 +65,8 @@ github.com/ClickHouse/clickhouse-go v1.4.3 h1:iAFMa2UrQdR5bHJ2/yaSLffZkxpcOYQMCU github.com/ClickHouse/clickhouse-go v1.4.3/go.mod h1:EaI/sW7Azgz9UATzd5ZdZHRUhHgv5+JMS9NSr2smCJI= github.com/JohnCGriffin/overflow v0.0.0-20211019200055-46fa312c352c h1:RGWPOewvKIROun94nF7v2cua9qP+thov/7M50KEoeSU= github.com/JohnCGriffin/overflow v0.0.0-20211019200055-46fa312c352c/go.mod h1:X0CRv0ky0k6m906ixxpzmDRLvX58TFUKS2eePweuyxk= +github.com/MakeNowJust/heredoc/v2 v2.0.1 h1:rlCHh70XXXv7toz95ajQWOWQnN4WNLt0TdpZYIR/J6A= +github.com/MakeNowJust/heredoc/v2 v2.0.1/go.mod h1:6/2Abh5s+hc3g9nbWLe9ObDIOhaRrqsyY9MWy+4JdRM= github.com/Masterminds/semver/v3 v3.1.1 h1:hLg3sBzpNErnxhQtUy/mmLR2I9foDujNK030IGemrRc= github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= @@ -131,6 +133,8 @@ github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XL github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/cloudflare/golz4 v0.0.0-20150217214814-ef862a3cdc58 h1:F1EaeKL/ta07PY/k9Os/UFtwERei2/XzGemhpGnBKNg= github.com/cloudflare/golz4 v0.0.0-20150217214814-ef862a3cdc58/go.mod h1:EOBUe0h4xcZ5GoxqC5SDxFQ8gwyZPKQoEzownBlhI80= +github.com/cloudspannerecosystem/memefish v0.0.0-20241126143851-bb8d10117087 h1:9YNZvjLt/U9/TXOwyADzCGPt6nJh+zH+4WzLpmc/BCg= +github.com/cloudspannerecosystem/memefish v0.0.0-20241126143851-bb8d10117087/go.mod h1:iYAaNZfVIn4QYfUmXt+3EeHAok/kqpN/fp/8kgDHjx8= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cncf/xds/go v0.0.0-20240318125728-8a4994d93e50 h1:DBmgJDC9dTfkVyGgipamEh2BpGYxScCH1TOF1LL1cXc= github.com/cncf/xds/go v0.0.0-20240318125728-8a4994d93e50/go.mod h1:5e1+Vvlzido69INQaVO6d87Qn543Xr6nooe9Kz7oBFM= From 21d7da059ad3e4a14dc71e2d46aa77d8656ed2ed Mon Sep 17 00:00:00 2001 From: Joseph Watson Date: Fri, 29 Nov 2024 12:29:43 -0500 Subject: [PATCH 3/5] Additional tests for inline-comment --- database/spanner/spanner.go | 4 +-- database/spanner/spanner_test.go | 54 ++++++++++++++++++++++++++------ 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/database/spanner/spanner.go b/database/spanner/spanner.go index a1dd08b08..4a68d9ebd 100644 --- a/database/spanner/spanner.go +++ b/database/spanner/spanner.go @@ -442,8 +442,8 @@ func statementGroups(migr []byte) (groups []*statementGroup, err error) { continue } - if len(lex.Token.Comments) > 0 { - // preserve newline where comments are removed + if len(lex.Token.Comments) > 0 && strings.HasPrefix(lex.Token.Comments[0].Raw, "--") { + // standard comment Token consumes a \n, so we need to add it back if _, err := stmt.WriteString("\n"); err != nil { return nil, err } diff --git a/database/spanner/spanner_test.go b/database/spanner/spanner_test.go index cbacd2d96..e17b967d4 100644 --- a/database/spanner/spanner_test.go +++ b/database/spanner/spanner_test.go @@ -117,7 +117,7 @@ func Test_statementGroups(t *testing.T) { { typ: statementTypeDDL, stmts: []string{ - "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n ) PRIMARY KEY (id)", + "CREATE TABLE table_name (\n\t\t\tid STRING(255) NOT NULL,\n\t\t) PRIMARY KEY (id)", }, }, }, @@ -143,7 +143,7 @@ func Test_statementGroups(t *testing.T) { { typ: statementTypeDDL, stmts: []string{ - "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n ) PRIMARY KEY (id)", + "CREATE TABLE table_name (\n\t\t\tid STRING(255) NOT NULL,\n\t\t) PRIMARY KEY (id)", }, }, }, @@ -160,7 +160,7 @@ func Test_statementGroups(t *testing.T) { { typ: statementTypeDDL, stmts: []string{ - "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n ) PRIMARY KEY(id)", + "CREATE TABLE table_name (\n\t\t\tid STRING(255) NOT NULL,\n\t\t) PRIMARY KEY(id)", "CREATE INDEX table_name_id_idx ON table_name (id)", }, }, @@ -178,7 +178,7 @@ func Test_statementGroups(t *testing.T) { { typ: statementTypeDDL, stmts: []string{ - "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n ) PRIMARY KEY(id)", + "CREATE TABLE table_name (\n\t\t\tid STRING(255) NOT NULL,\n\t\t) PRIMARY KEY(id)", "CREATE INDEX table_name_id_idx ON table_name (id)", }, }, @@ -197,17 +197,17 @@ func Test_statementGroups(t *testing.T) { { typ: statementTypeDDL, stmts: []string{ - "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n ) PRIMARY KEY(id)", + "CREATE TABLE table_name (\n\t\t\tid STRING(255) NOT NULL,\n\t\t) PRIMARY KEY(id)", "CREATE INDEX table_name_id_idx ON table_name (id)", }, }, }, }, { - name: "multi statement, no trailing semicolon, inline comment", + name: "multi statement, no trailing semicolon, end-of-line comment", // From https://github.com/mattes/migrate/pull/281 multiStatement: `CREATE TABLE table_name ( - id STRING(255) NOT NULL, -- inline comment + id STRING(255) NOT NULL, -- end-of-line comment ) PRIMARY KEY(id); CREATE INDEX table_name_id_idx ON table_name (id)`, @@ -215,7 +215,41 @@ func Test_statementGroups(t *testing.T) { { typ: statementTypeDDL, stmts: []string{ - "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n ) PRIMARY KEY(id)", + "CREATE TABLE table_name (\n\t\t\tid STRING(255) NOT NULL,\n\t\t) PRIMARY KEY(id)", + "CREATE INDEX table_name_id_idx ON table_name (id)", + }, + }, + }, + }, + { + name: "multi statement, inline comment", + multiStatement: `CREATE TABLE table_name ( + id STRING(255) NOT NULL, /* inline comment */ + ) PRIMARY KEY(id); + + CREATE INDEX table_name_id_idx ON table_name (id);`, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE TABLE table_name (\n\t\t\tid STRING(255) NOT NULL,\n\t\t) PRIMARY KEY(id)", + "CREATE INDEX table_name_id_idx ON table_name (id)", + }, + }, + }, + }, + { + name: "multi statement, inline comment inside DML", + multiStatement: `CREATE TABLE table_name ( + id STRING(255 /* inline comment */) NOT NULL, + ) PRIMARY KEY(id); + + CREATE INDEX table_name_id_idx ON table_name (id);`, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE TABLE table_name (\n\t\t\tid STRING(255) NOT NULL,\n\t\t) PRIMARY KEY(id)", "CREATE INDEX table_name_id_idx ON table_name (id)", }, }, @@ -229,7 +263,7 @@ func Test_statementGroups(t *testing.T) { { typ: statementTypeDDL, stmts: []string{ - "ALTER TABLE users ALTER COLUMN created\n SET OPTIONS (allow_commit_timestamp=true)", + "ALTER TABLE users ALTER COLUMN created\n\t\t\tSET OPTIONS (allow_commit_timestamp=true)", }, }, }, @@ -244,7 +278,7 @@ func Test_statementGroups(t *testing.T) { { typ: statementTypeDDL, stmts: []string{ - "CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n sum NUMERIC,\n ) PRIMARY KEY (id)", + "CREATE TABLE table_name (\n\t\t\t\tid STRING(255) NOT NULL,\n\t\t\t\tsum NUMERIC,\n\t\t\t) PRIMARY KEY (id)", }, }, }, From 0db95f26de743d19bac5ca9345a88b8210ae89c9 Mon Sep 17 00:00:00 2001 From: Joseph Watson Date: Fri, 29 Nov 2024 13:19:04 -0500 Subject: [PATCH 4/5] Additional unit tests to address open issues in github --- database/spanner/spanner_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/database/spanner/spanner_test.go b/database/spanner/spanner_test.go index e17b967d4..d4207a9d9 100644 --- a/database/spanner/spanner_test.go +++ b/database/spanner/spanner_test.go @@ -313,6 +313,34 @@ func Test_statementGroups(t *testing.T) { }, }, }, + { + name: "ALTER with DEFAULT", + // From https://github.com/golang-migrate/migrate/issues/918 + multiStatement: "ALTER TABLE t1 ADD COLUMN c1 STRING(MAX) DEFAULT ('');ALTER TABLE t1 ADD COLUMN c1 STRING(MAX) NOT NULL DEFAULT ('');", + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "ALTER TABLE t1 ADD COLUMN c1 STRING(MAX) DEFAULT ('')", + "ALTER TABLE t1 ADD COLUMN c1 STRING(MAX) NOT NULL DEFAULT ('')", + }, + }, + }, + }, + { + name: "Change Streams", + multiStatement: `CREATE CHANGE STREAM NamesAndAlbums +FOR Singers(FirstName, LastName), Albums +OPTIONS ( retention_period = '36h' );`, + expected: []*statementGroup{ + { + typ: statementTypeDDL, + stmts: []string{ + "CREATE CHANGE STREAM NamesAndAlbums\nFOR Singers(FirstName, LastName), Albums\nOPTIONS ( retention_period = '36h' )", + }, + }, + }, + }, } for _, tc := range testCases { From dd4885a506f43b34a1975411f444e1c6e993c9f5 Mon Sep 17 00:00:00 2001 From: Joseph Watson Date: Sat, 11 Jan 2025 11:30:25 -0500 Subject: [PATCH 5/5] Update memefish --- go.mod | 4 ++-- go.sum | 9 ++++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index e2a75a468..25c5b858e 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/ClickHouse/clickhouse-go v1.4.3 github.com/aws/aws-sdk-go v1.49.6 github.com/cenkalti/backoff/v4 v4.1.2 - github.com/cloudspannerecosystem/memefish v0.0.0-20241126143851-bb8d10117087 + github.com/cloudspannerecosystem/memefish v0.2.0 github.com/cockroachdb/cockroach-go/v2 v2.1.1 github.com/dhui/dktest v0.4.5 github.com/docker/docker v27.2.0+incompatible @@ -148,7 +148,7 @@ require ( github.com/klauspost/asmfmt v1.3.2 // indirect github.com/klauspost/compress v1.15.11 // indirect github.com/klauspost/cpuid/v2 v2.0.9 // indirect - github.com/mattn/go-colorable v0.1.6 // indirect + github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.16 // indirect github.com/minio/asm2plan9s v0.0.0-20200509001527-cdd76441f9d8 // indirect github.com/minio/c2goasm v0.0.0-20190812172519-36a3d3bbc4f3 // indirect diff --git a/go.sum b/go.sum index a52293678..8dcd53559 100644 --- a/go.sum +++ b/go.sum @@ -133,8 +133,8 @@ github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XL github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/cloudflare/golz4 v0.0.0-20150217214814-ef862a3cdc58 h1:F1EaeKL/ta07PY/k9Os/UFtwERei2/XzGemhpGnBKNg= github.com/cloudflare/golz4 v0.0.0-20150217214814-ef862a3cdc58/go.mod h1:EOBUe0h4xcZ5GoxqC5SDxFQ8gwyZPKQoEzownBlhI80= -github.com/cloudspannerecosystem/memefish v0.0.0-20241126143851-bb8d10117087 h1:9YNZvjLt/U9/TXOwyADzCGPt6nJh+zH+4WzLpmc/BCg= -github.com/cloudspannerecosystem/memefish v0.0.0-20241126143851-bb8d10117087/go.mod h1:iYAaNZfVIn4QYfUmXt+3EeHAok/kqpN/fp/8kgDHjx8= +github.com/cloudspannerecosystem/memefish v0.2.0 h1:t4YVA7uGHaJyfxPFCHHShiN6yjcKsjSMwc1hZ+0BGrE= +github.com/cloudspannerecosystem/memefish v0.2.0/go.mod h1:rrHSb7mQfV8Xl47cvnE1dYJ8+zpvog49Yma0j4YCDnc= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cncf/xds/go v0.0.0-20240318125728-8a4994d93e50 h1:DBmgJDC9dTfkVyGgipamEh2BpGYxScCH1TOF1LL1cXc= github.com/cncf/xds/go v0.0.0-20240318125728-8a4994d93e50/go.mod h1:5e1+Vvlzido69INQaVO6d87Qn543Xr6nooe9Kz7oBFM= @@ -416,6 +416,8 @@ github.com/k0kubun/colorstring v0.0.0-20150214042306-9440f1994b88 h1:uC1QfSlInpQ github.com/k0kubun/colorstring v0.0.0-20150214042306-9440f1994b88/go.mod h1:3w7q1U84EfirKl04SVQ/s7nPm1ZPhiXd34z40TNz36k= github.com/k0kubun/pp v2.3.0+incompatible h1:EKhKbi34VQDWJtq+zpsKSEhkHHs9w2P8Izbq8IhLVSo= github.com/k0kubun/pp v2.3.0+incompatible/go.mod h1:GWse8YhT0p8pT4ir3ZgBbfZild3tgzSScAn6HmfYukg= +github.com/k0kubun/pp/v3 v3.4.1 h1:1WdFZDRRqe8UsR61N/2RoOZ3ziTEqgTPVqKrHeb779Y= +github.com/k0kubun/pp/v3 v3.4.1/go.mod h1:+SiNiqKnBfw1Nkj82Lh5bIeKQOAkPy6Xw9CAZUZ8npI= github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 h1:iQTw/8FWTuc7uiaSepXwyf3o52HaUYcV+Tu66S3F5GA= github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0/go.mod h1:1NbS8ALrpOvjt0rHPNLyCIeMtbizbir8U//inJ+zuB8= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= @@ -456,8 +458,9 @@ github.com/markbates/pkger v0.15.1/go.mod h1:0JoVlrol20BSywW79rN3kdFFsE5xYM+rSCQ github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.1/go.mod h1:FuOcm+DKB9mbwrcAfNl7/TZVBZ6rcnceauSikq3lYCQ= github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= -github.com/mattn/go-colorable v0.1.6 h1:6Su7aK7lXmJ/U79bYtBjLNaha4Fs1Rg9plHpcH+vvnE= github.com/mattn/go-colorable v0.1.6/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= +github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= +github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= github.com/mattn/go-isatty v0.0.5/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/mattn/go-isatty v0.0.7/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=