Skip to content

Commit e6faa29

Browse files
alethenorioalethenorio
committed
Add support for comments in Spanner migrations
At the moment GCP Spanner backend does not seem to support comments (issue being tracked at https://issuetracker.google.com/issues/159730604). By adding support for parsing the migration DDL with spansql we are able to support comments and remove them before applying on the database
1 parent 8b6e3f1 commit e6faa29

File tree

5 files changed

+173
-73
lines changed

5 files changed

+173
-73
lines changed

‎database/spanner/README.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ See [Google Spanner Documentation](https://cloud.google.com/spanner/docs) for de
1212
| Param | WithInstance Config | Description |
1313
| ----- | ------------------- | ----------- |
1414
| `x-migrations-table` | `MigrationsTable` | Name of the migrations table |
15+
| `x-clean-statements` | `CleanStatements` | Whether to parse and clean DDL statements before running migration towards Spanner (Required for comments and multiple statements) |
1516
| `url` | `DatabaseName` | The full path to the Spanner database resource. If provided as part of `Config` it must not contain a scheme or query string to match the format `projects/{projectId}/instances/{instanceId}/databases/{databaseName}`|
1617
| `projectId` || The Google Cloud Platform project id
1718
| `instanceId` || The id of the instance running Spanner
@@ -28,6 +29,15 @@ See [Google Spanner Documentation](https://cloud.google.com/spanner/docs) for de
2829
> 1496601752/u add_index_on_user_emails (2m12.155787369s)
2930
> 1496602638/u create_books_table (2m30.77299181s)
3031
32+
## DDL with comments
33+
34+
At the moment the GCP Spanner backed does not seem to allow for comments (See https://issuetracker.google.com/issues/159730604)
35+
so in order to be able to use migration with DDL containing comments `x-clean-stamements` is required
36+
37+
## Multiple statements
38+
39+
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
40+
3141
## Testing
3242
3343
To unit test the `spanner` driver, `SPANNER_DATABASE` needs to be set. You'll

‎database/spanner/spanner.go‎

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ import (
77
"log"
88
nurl "net/url"
99
"regexp"
10+
"strconv"
1011
"strings"
1112

1213
"golang.org/x/net/context"
1314

1415
"cloud.google.com/go/spanner"
1516
sdb "cloud.google.com/go/spanner/admin/database/apiv1"
17+
"cloud.google.com/go/spanner/spansql"
1618

1719
"github.com/golang-migrate/migrate/v4"
1820
"github.com/golang-migrate/migrate/v4/database"
@@ -42,6 +44,11 @@ var (
4244
type Config struct {
4345
MigrationsTable string
4446
DatabaseName string
47+
// Whether to parse the migration DDL with spansql before
48+
// running them towards Spanner.
49+
// Parsing outputs clean DDL statements such as reformatted
50+
// and void of comments.
51+
CleanStatements bool
4552
}
4653

4754
// Spanner implements database.Driver for Google Cloud Spanner
@@ -110,10 +117,20 @@ func (s *Spanner) Open(url string) (database.Driver, error) {
110117

111118
migrationsTable := purl.Query().Get("x-migrations-table")
112119

120+
cleanQuery := purl.Query().Get("x-clean-statements")
121+
clean := false
122+
if cleanQuery != "" {
123+
clean, err = strconv.ParseBool(cleanQuery)
124+
if err != nil {
125+
return nil, err
126+
}
127+
}
128+
113129
db := &DB{admin: adminClient, data: dataClient}
114130
return WithInstance(db, &Config{
115131
DatabaseName: dbname,
116132
MigrationsTable: migrationsTable,
133+
CleanStatements: clean,
117134
})
118135
}
119136

@@ -141,10 +158,15 @@ func (s *Spanner) Run(migration io.Reader) error {
141158
return err
142159
}
143160

144-
// run migration
145-
stmts := migrationStatements(migr)
146-
ctx := context.Background()
161+
stmts := []string{string(migr)}
162+
if s.config.CleanStatements {
163+
stmts, err = cleanStatements(migr)
164+
if err != nil {
165+
return err
166+
}
167+
}
147168

169+
ctx := context.Background()
148170
op, err := s.db.admin.UpdateDatabaseDdl(ctx, &adminpb.UpdateDatabaseDdlRequest{
149171
Database: s.config.DatabaseName,
150172
Statements: stmts,
@@ -302,17 +324,17 @@ func (s *Spanner) ensureVersionTable() (err error) {
302324
return nil
303325
}
304326

305-
func migrationStatements(migration []byte) []string {
306-
migrationString := string(migration[:])
307-
migrationString = strings.TrimSpace(migrationString)
308-
309-
allStatements := strings.Split(migrationString, ";")
310-
nonEmptyStatements := allStatements[:0]
311-
for _, s := range allStatements {
312-
s = strings.TrimSpace(s)
313-
if s != "" {
314-
nonEmptyStatements = append(nonEmptyStatements, s)
315-
}
327+
func cleanStatements(migration []byte) ([]string, error) {
328+
// The Spanner GCP backend does not yet support comments for the UpdateDatabaseDdl RPC
329+
// (see https://issuetracker.google.com/issues/159730604) we use
330+
// spansql to parse the DDL and output valid stamements without comments
331+
ddl, err := spansql.ParseDDL("", string(migration))
332+
if err != nil {
333+
return nil, err
334+
}
335+
stmts := make([]string, 0, len(ddl.List))
336+
for _, stmt := range ddl.List {
337+
stmts = append(stmts, stmt.SQL())
316338
}
317-
return nonEmptyStatements
339+
return stmts, nil
318340
}

‎database/spanner/spanner_test.go‎

Lines changed: 60 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,14 @@ import (
44
"fmt"
55
"os"
66
"testing"
7-
)
87

9-
import (
108
"github.com/golang-migrate/migrate/v4"
9+
1110
dt "github.com/golang-migrate/migrate/v4/database/testing"
12-
_ "github.com/golang-migrate/migrate/v4/source/file"
13-
)
1411

15-
import (
12+
_ "github.com/golang-migrate/migrate/v4/source/file"
1613
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
1715
)
1816

1917
func Test(t *testing.T) {
@@ -58,71 +56,97 @@ func TestMigrate(t *testing.T) {
5856
dt.TestMigrate(t, m)
5957
}
6058

61-
func TestMultistatementSplit(t *testing.T) {
59+
func TestCleanStatements(t *testing.T) {
6260
testCases := []struct {
6361
name string
6462
multiStatement string
6563
expected []string
6664
}{
6765
{
68-
name: "single statement, single line, no semicolon",
66+
name: "no statement",
67+
multiStatement: "",
68+
expected: []string{},
69+
},
70+
{
71+
name: "single statement, single line, no semicolon, no comment",
6972
multiStatement: "CREATE TABLE table_name (id STRING(255) NOT NULL) PRIMARY KEY (id)",
70-
expected: []string{"CREATE TABLE table_name (id STRING(255) NOT NULL) PRIMARY KEY (id)"},
73+
expected: []string{"CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n) PRIMARY KEY(id)"},
7174
},
7275
{
73-
name: "single statement, multi line, no semicolon",
76+
name: "single statement, multi line, no semicolon, no comment",
7477
multiStatement: `CREATE TABLE table_name (
75-
id STRING(255) NOT NULL,
76-
) PRIMARY KEY (id)`,
77-
expected: []string{`CREATE TABLE table_name (
78-
id STRING(255) NOT NULL,
79-
) PRIMARY KEY (id)`},
78+
id STRING(255) NOT NULL,
79+
) PRIMARY KEY (id)`,
80+
expected: []string{"CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n) PRIMARY KEY(id)"},
8081
},
8182
{
82-
name: "single statement, single line, with semicolon",
83+
name: "single statement, single line, with semicolon, no comment",
8384
multiStatement: "CREATE TABLE table_name (id STRING(255) NOT NULL) PRIMARY KEY (id);",
84-
expected: []string{"CREATE TABLE table_name (id STRING(255) NOT NULL) PRIMARY KEY (id)"},
85+
expected: []string{"CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n) PRIMARY KEY(id)"},
8586
},
8687
{
87-
name: "single statement, multi line, with semicolon",
88+
name: "single statement, multi line, with semicolon, no comment",
8889
multiStatement: `CREATE TABLE table_name (
89-
id STRING(255) NOT NULL,
90-
) PRIMARY KEY (id);`,
90+
id STRING(255) NOT NULL,
91+
) PRIMARY KEY (id);`,
92+
expected: []string{"CREATE TABLE table_name (\n id STRING(255) NOT NULL,\n) PRIMARY KEY(id)"},
93+
},
94+
{
95+
name: "multi statement, with trailing semicolon. no comment",
96+
// From https://github.com/mattes/migrate/pull/281
97+
multiStatement: `CREATE TABLE table_name (
98+
id STRING(255) NOT NULL,
99+
) PRIMARY KEY(id);
100+
101+
CREATE INDEX table_name_id_idx ON table_name (id);`,
102+
expected: []string{`CREATE TABLE table_name (
103+
id STRING(255) NOT NULL,
104+
) PRIMARY KEY(id)`, "CREATE INDEX table_name_id_idx ON table_name(id)"},
105+
},
106+
{
107+
name: "multi statement, no trailing semicolon, no comment",
108+
// From https://github.com/mattes/migrate/pull/281
109+
multiStatement: `CREATE TABLE table_name (
110+
id STRING(255) NOT NULL,
111+
) PRIMARY KEY(id);
112+
113+
CREATE INDEX table_name_id_idx ON table_name (id)`,
91114
expected: []string{`CREATE TABLE table_name (
92-
id STRING(255) NOT NULL,
93-
) PRIMARY KEY (id)`},
115+
id STRING(255) NOT NULL,
116+
) PRIMARY KEY(id)`, "CREATE INDEX table_name_id_idx ON table_name(id)"},
94117
},
95118
{
96-
name: "multi statement, with trailing semicolon",
119+
name: "multi statement, no trailing semicolon, standalone comment",
97120
// From https://github.com/mattes/migrate/pull/281
98121
multiStatement: `CREATE TABLE table_name (
99-
id STRING(255) NOT NULL,
100-
) PRIMARY KEY(id);
122+
-- standalone comment
123+
id STRING(255) NOT NULL,
124+
) PRIMARY KEY(id);
101125
102-
CREATE INDEX table_name_id_idx ON table_name (id);`,
126+
CREATE INDEX table_name_id_idx ON table_name (id)`,
103127
expected: []string{`CREATE TABLE table_name (
104-
id STRING(255) NOT NULL,
105-
) PRIMARY KEY(id)`, "CREATE INDEX table_name_id_idx ON table_name (id)"},
128+
id STRING(255) NOT NULL,
129+
) PRIMARY KEY(id)`, "CREATE INDEX table_name_id_idx ON table_name(id)"},
106130
},
107131
{
108-
name: "multi statement, no trailing semicolon",
132+
name: "multi statement, no trailing semicolon, inline comment",
109133
// From https://github.com/mattes/migrate/pull/281
110134
multiStatement: `CREATE TABLE table_name (
111-
id STRING(255) NOT NULL,
112-
) PRIMARY KEY(id);
135+
id STRING(255) NOT NULL, -- inline comment
136+
) PRIMARY KEY(id);
113137
114-
CREATE INDEX table_name_id_idx ON table_name (id)`,
138+
CREATE INDEX table_name_id_idx ON table_name (id)`,
115139
expected: []string{`CREATE TABLE table_name (
116-
id STRING(255) NOT NULL,
117-
) PRIMARY KEY(id)`, "CREATE INDEX table_name_id_idx ON table_name (id)"},
140+
id STRING(255) NOT NULL,
141+
) PRIMARY KEY(id)`, "CREATE INDEX table_name_id_idx ON table_name(id)"},
118142
},
119143
}
120144

121145
for _, tc := range testCases {
122146
t.Run(tc.name, func(t *testing.T) {
123-
if stmts := migrationStatements([]byte(tc.multiStatement)); !assert.Equal(t, stmts, tc.expected) {
124-
t.Error()
125-
}
147+
stmts, err := cleanStatements([]byte(tc.multiStatement))
148+
require.NoError(t, err, "Error cleaning statements")
149+
assert.Equal(t, tc.expected, stmts)
126150
})
127151
}
128152
}

‎go.mod‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
module github.com/golang-migrate/migrate/v4
22

33
require (
4-
cloud.google.com/go/spanner v1.5.0
5-
cloud.google.com/go/storage v1.6.0
4+
cloud.google.com/go/spanner v1.6.0
5+
cloud.google.com/go/storage v1.8.0
66
github.com/ClickHouse/clickhouse-go v1.3.12
77
github.com/Microsoft/go-winio v0.4.14 // indirect
88
github.com/aws/aws-sdk-go v1.17.7
@@ -40,10 +40,10 @@ require (
4040
github.com/xdg/stringprep v1.0.0 // indirect
4141
gitlab.com/nyarla/go-crypt v0.0.0-20160106005555-d9a5dc2b789b // indirect
4242
go.mongodb.org/mongo-driver v1.1.0
43-
golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e
44-
golang.org/x/tools v0.0.0-20200413015812-1f08ef6002a8
45-
google.golang.org/api v0.21.0
46-
google.golang.org/genproto v0.0.0-20200413115906-b5235f65be36
43+
golang.org/x/net v0.0.0-20200520182314-0ba52f642ac2
44+
golang.org/x/tools v0.0.0-20200522201501-cb1345f3a375
45+
google.golang.org/api v0.25.0
46+
google.golang.org/genproto v0.0.0-20200526151428-9bb895338b15
4747
modernc.org/b v1.0.0 // indirect
4848
modernc.org/db v1.0.0 // indirect
4949
modernc.org/file v1.0.0 // indirect

0 commit comments

Comments
 (0)