Cleanup regex usage in spanner DB driver

- Only compile a regex once for re-use
    - Don't use regex unnecessarily (removing trailing semicolon)
    - Add test for parsing multiple statements in a migration
This commit is contained in:
Dale Hui 2019-05-20 08:28:07 -07:00
parent bd81e32d1a
commit 923901c81f
3 changed files with 88 additions and 8 deletions

View File

@ -209,6 +209,8 @@ func (s *Spanner) Version() (version int, dirty bool, err error) {
return version, dirty, nil return version, dirty, nil
} }
var nameMatcher = regexp.MustCompile(`(CREATE TABLE\s(\S+)\s)|(CREATE.+INDEX\s(\S+)\s)`)
// Drop implements database.Driver. Retrieves the database schema first and // Drop implements database.Driver. Retrieves the database schema first and
// creates statements to drop the indexes and tables accordingly. // creates statements to drop the indexes and tables accordingly.
// Note: The drop statements are created in reverse order to how they're // Note: The drop statements are created in reverse order to how they're
@ -227,11 +229,10 @@ func (s *Spanner) Drop() error {
return nil return nil
} }
r := regexp.MustCompile(`(CREATE TABLE\s(\S+)\s)|(CREATE.+INDEX\s(\S+)\s)`)
stmts := make([]string, 0) stmts := make([]string, 0)
for i := len(res.Statements) - 1; i >= 0; i-- { for i := len(res.Statements) - 1; i >= 0; i-- {
s := res.Statements[i] s := res.Statements[i]
m := r.FindSubmatch([]byte(s)) m := nameMatcher.FindSubmatch([]byte(s))
if len(m) == 0 { if len(m) == 0 {
continue continue
@ -302,11 +303,15 @@ func (s *Spanner) ensureVersionTable() (err error) {
} }
func migrationStatements(migration []byte) []string { func migrationStatements(migration []byte) []string {
regex := regexp.MustCompile(";$")
migrationString := string(migration[:]) migrationString := string(migration[:])
migrationString = strings.TrimSpace(migrationString) migrationString = strings.TrimSpace(migrationString)
migrationString = regex.ReplaceAllString(migrationString, "")
statements := strings.Split(migrationString, ";") allStatements := strings.Split(migrationString, ";")
return statements nonEmptyStatements := allStatements[:0]
for _, s := range allStatements {
if s != "" {
nonEmptyStatements = append(nonEmptyStatements, s)
}
}
return nonEmptyStatements
} }

View File

@ -2,14 +2,20 @@ package spanner
import ( import (
"fmt" "fmt"
"github.com/golang-migrate/migrate/v4"
"os" "os"
"testing" "testing"
)
import (
"github.com/golang-migrate/migrate/v4"
dt "github.com/golang-migrate/migrate/v4/database/testing" dt "github.com/golang-migrate/migrate/v4/database/testing"
_ "github.com/golang-migrate/migrate/v4/source/file" _ "github.com/golang-migrate/migrate/v4/source/file"
) )
import (
"github.com/stretchr/testify/assert"
)
func Test(t *testing.T) { func Test(t *testing.T) {
if testing.Short() { if testing.Short() {
t.Skip("skipping test in short mode.") t.Skip("skipping test in short mode.")
@ -51,3 +57,72 @@ func TestMigrate(t *testing.T) {
} }
dt.TestMigrate(t, m, []byte("SELECT 1")) dt.TestMigrate(t, m, []byte("SELECT 1"))
} }
func TestMultistatementSplit(t *testing.T) {
testCases := []struct {
name string
multiStatement string
expected []string
}{
{
name: "single statement, single line, no semicolon",
multiStatement: "CREATE TABLE table_name (id STRING(255) NOT NULL) PRIMARY KEY (id)",
expected: []string{"CREATE TABLE table_name (id STRING(255) NOT NULL) PRIMARY KEY (id)"},
},
{
name: "single statement, multi line, no semicolon",
multiStatement: `CREATE TABLE table_name (
id STRING(255) NOT NULL,
) PRIMARY KEY (id)`,
expected: []string{`CREATE TABLE table_name (
id STRING(255) NOT NULL,
) PRIMARY KEY (id)`},
},
{
name: "single statement, single line, with semicolon",
multiStatement: "CREATE TABLE table_name (id STRING(255) NOT NULL) PRIMARY KEY (id);",
expected: []string{"CREATE TABLE table_name (id STRING(255) NOT NULL) PRIMARY KEY (id)"},
},
{
name: "single statement, multi line, with semicolon",
multiStatement: `CREATE TABLE table_name (
id STRING(255) NOT NULL,
) PRIMARY KEY (id);`,
expected: []string{`CREATE TABLE table_name (
id STRING(255) NOT NULL,
) PRIMARY KEY (id)`},
},
{
name: "multi statement, with trailing semicolon",
// From https://github.com/mattes/migrate/pull/281
multiStatement: `CREATE TABLE table_name (
id STRING(255) NOT NULL,
) 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)`, "\n\nCREATE INDEX table_name_id_idx ON table_name (id)"},
},
{
name: "multi statement, no trailing semicolon",
// From https://github.com/mattes/migrate/pull/281
multiStatement: `CREATE TABLE table_name (
id STRING(255) NOT NULL,
) 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)`, "\n\nCREATE INDEX table_name_id_idx ON table_name (id)"},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
if stmts := migrationStatements([]byte(tc.multiStatement)); !assert.Equal(t, stmts, tc.expected) {
t.Error()
}
})
}
}

2
go.mod
View File

@ -33,7 +33,7 @@ require (
github.com/satori/go.uuid v1.2.0 // indirect github.com/satori/go.uuid v1.2.0 // indirect
github.com/shopspring/decimal v0.0.0-20180709203117-cd690d0c9e24 // indirect github.com/shopspring/decimal v0.0.0-20180709203117-cd690d0c9e24 // indirect
github.com/sirupsen/logrus v1.4.1 // indirect github.com/sirupsen/logrus v1.4.1 // indirect
github.com/stretchr/testify v1.3.0 // indirect github.com/stretchr/testify v1.3.0
github.com/tidwall/pretty v0.0.0-20180105212114-65a9db5fad51 // indirect github.com/tidwall/pretty v0.0.0-20180105212114-65a9db5fad51 // indirect
github.com/xanzy/go-gitlab v0.15.0 github.com/xanzy/go-gitlab v0.15.0
github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c // indirect github.com/xdg/scram v0.0.0-20180814205039-7eeb5667e42c // indirect