Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

strip basic comments #19

Merged
merged 1 commit into from
Oct 11, 2020
Merged
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
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
env:
SPANNER_INSTANCE_ID: ${{env.SPANNER_INSTANCE_ID}}
SPANNER_PROJECT_ID: ${{env.SPANNER_PROJECT_ID}}
image: roryq/spanner-emulator:0.8.2
image: roryq/spanner-emulator:1.1.0
env:
SPANNER_INSTANCE_ID: inst
SPANNER_PROJECT_ID: proj
Expand Down
13 changes: 9 additions & 4 deletions pkg/spanner/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,16 @@
package spanner

import (
"cloud.google.com/go/spanner"
"context"
"fmt"
"github.com/google/uuid"
"google.golang.org/api/iterator"
"io/ioutil"
"os"
"testing"
"time"

"cloud.google.com/go/spanner"
"github.com/google/uuid"
"google.golang.org/api/iterator"
)

const (
Expand Down Expand Up @@ -261,7 +263,7 @@ func ensureMigrationColumn(t *testing.T, ctx context.Context, client *Client, co
func ensureMigrationHistoryRecord(t *testing.T, ctx context.Context, client *Client, version int64, dirty bool) {
history, err := client.GetMigrationHistory(ctx, migrationTable)
for i := range history {
if history[i].Version == version && history[i].Dirty == dirty{
if history[i].Version == version && history[i].Dirty == dirty {
return
}
}
Expand Down Expand Up @@ -625,6 +627,9 @@ func testClientWithDatabase(t *testing.T, ctx context.Context) (*Client, func())
t.Fatalf("failed to create database: %v", err)
}

// emulator weirdness in CI
time.Sleep(100 * time.Millisecond)

return client, func() {
defer client.Close()

Expand Down
30 changes: 22 additions & 8 deletions pkg/spanner/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,13 @@ var (
// as it must be idempotent. This probably isn't solvable with more regexes.
notPartitionedDmlRegex = regexp.MustCompile("INSERT")

// matches a single comment on its own line
oneLineSingleComment = regexp.MustCompile(`(?m)^\s*--.*$`)
)

const (
statementKindDDL statementKind = "DDL"
statementKindDML statementKind = "DML"
statementKindDDL statementKind = "DDL"
statementKindDML statementKind = "DML"
statementKindPartitionedDML statementKind = "PartitionedDML"
)

Expand Down Expand Up @@ -132,12 +134,24 @@ func LoadMigrations(dir string) (Migrations, error) {
return migrations, nil
}

func stripComments(statement string) string {
return oneLineSingleComment.ReplaceAllString(statement, "")
}

func stripStatement(statement string) string {
return strings.TrimSpace(
stripComments(
strings.TrimSpace(
statement)))

}

func toStatements(file []byte) []string {
contents := bytes.Split(file, []byte(statementsSeparator))

statements := make([]string, 0, len(contents))
for _, c := range contents {
if statement := strings.TrimSpace(string(c)); statement != "" {
if statement := stripStatement(string(c)); statement != "" {
statements = append(statements, statement)
}
}
Expand All @@ -147,8 +161,8 @@ func toStatements(file []byte) []string {

func inspectStatementsKind(statements []string) (statementKind, error) {
kindMap := map[statementKind]uint64{
statementKindDDL: 0,
statementKindDML: 0,
statementKindDDL: 0,
statementKindDML: 0,
statementKindPartitionedDML: 0,
}

Expand All @@ -175,19 +189,19 @@ func distinctKind(kindMap map[statementKind]uint64, kind statementKind) bool {
target := kindMap[kind]

var total uint64
for k:= range kindMap {
for k := range kindMap {
total = total + kindMap[k]
}

return target == total
}

func getStatementKind(statement string) statementKind {
if isPartitionedDMLOnly(statement){
if isPartitionedDMLOnly(statement) {
return statementKindPartitionedDML
}

if isDMLAny(statement){
if isDMLAny(statement) {
return statementKindDML
}

Expand Down
69 changes: 69 additions & 0 deletions pkg/spanner/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,72 @@ func Test_inspectStatementsKind(t *testing.T) {
})
}
}

func Test_stripStatement(t *testing.T) {
tests := []struct {
name string
statement string
want string
}{
{
name: "Basic single line comment is removed",
statement: `-- THIS SHOULD BE REMOVED
CREATE TABLE Singers (
SingerID STRING(36) NOT NULL,
FirstName STRING(1024),
) PRIMARY KEY(SingerID)`,
want: `CREATE TABLE Singers (
SingerID STRING(36) NOT NULL,
FirstName STRING(1024),
) PRIMARY KEY(SingerID)`,
},
{
name: "Maligned single line comment is removed",
statement: ` -- THIS SHOULD BE REMOVED
CREATE TABLE Singers (
SingerID STRING(36) NOT NULL,
FirstName STRING(1024),
) PRIMARY KEY(SingerID)`,
want: `CREATE TABLE Singers (
SingerID STRING(36) NOT NULL,
FirstName STRING(1024),
) PRIMARY KEY(SingerID)`,
},
// {
// name: "Single line comment after SQL is removed",
// statement: `CREATE TABLE SchemaMigrations (
// Version INT64 NOT NULL,
// Dirty BOOL NOT NULL, -- THIS SHOULD BE REMOVED
//) PRIMARY KEY(Version)`,
// want: `CREATE TABLE SchemaMigrations (
// Version INT64 NOT NULL,
// Dirty BOOL NOT NULL,
//) PRIMARY KEY(Version)`,
// },
// {
// name: "Double quoted comment remains",
// statement: `INSERT INTO Singers(SingerID, FirstName) VALUES(1, "first name
//-- THIS STAYS IN DOUBLE QUOTES
//")`,
// want: `INSERT INTO Singers(SingerID, FirstName) VALUES(1, "first name
//-- THIS STAYS IN DOUBLE QUOTES
//")`,
// },
// {
// name: "Single quoted comment remains",
// statement: `INSERT INTO Singers(SingerID, FirstName) VALUES(1, 'first name
//-- THIS STAYS IN DOUBLE QUOTES
//')`,
// want: `INSERT INTO Singers(SingerID, FirstName) VALUES(1, 'first name
//-- THIS STAYS IN DOUBLE QUOTES
//')`,
// },
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := stripStatement(tt.statement); got != tt.want {
t.Errorf("stripStatement() = %v, want %v", got, tt.want)
}
})
}
}