Repository: cloudspannerecosystem/harbourbridge
Pull request #74 (2020-07-10 11:01:12)

Fix: issue in parsing spatial datatypes and added reporting of auto_increment option


Patch:
diff --git a/internal/convert.go b/internal/convert.go
index 4dca660..67b2825 100644
--- a/internal/convert.go
+++ b/internal/convert.go
@@ -71,6 +71,7 @@ const (
 	Decimal
 	DecimalThatFits
 	Serial
+	AutoIncrement
 	Timestamp
 	Datetime
 	Widened
diff --git a/internal/report.go b/internal/report.go
index b58046c..b570c62 100644
--- a/internal/report.go
+++ b/internal/report.go
@@ -188,6 +188,8 @@ func buildTableReportBody(conv *Conv, srcTable string, issues map[string][]Schem
 					l = append(l, fmt.Sprintf("%s e.g. column '%s'", issueDB[i].brief, srcCol))
 				case ForeignKey:
 					l = append(l, fmt.Sprintf("Column '%s' uses foreign keys which Spanner does not support", srcCol))
+				case AutoIncrement:
+					l = append(l, fmt.Sprintf("Column '%s' is an autoincrement column. %s", srcCol, issueDB[i].brief))
 				case Timestamp:
 					// Avoid the confusing "timestamp is mapped to timestamp" message.
 					l = append(l, fmt.Sprintf("Some columns have source DB type 'timestamp without timezone' which is mapped to Spanner type timestamp e.g. column '%s'. %s", srcCol, issueDB[i].brief))
@@ -251,6 +253,7 @@ var issueDB = map[SchemaIssue]struct {
 	Decimal:               {brief: "Spanner does not support decimal. This type mapping could lose precision and is not recommended for production use", severity: warning},
 	DecimalThatFits:       {brief: "Spanner does not support decimal, but this type mapping preserves the decimal's specified precision", severity: note},
 	Serial:                {brief: "Spanner does not support autoincrementing types", severity: warning},
+	AutoIncrement:         {brief: "Spanner does not support auto_increment attribute", severity: warning},
 	Timestamp:             {brief: "Spanner timestamp is closer to PostgreSQL timestamptz", severity: note, batch: true},
 	Datetime:              {brief: "Spanner timestamp is closer to MySQL timestamp", severity: note, batch: true},
 	Time:                  {brief: "Spanner does not support time/year types", severity: note, batch: true},
diff --git a/mysql/infoschema.go b/mysql/infoschema.go
index 531a685..48751f6 100644
--- a/mysql/infoschema.go
+++ b/mysql/infoschema.go
@@ -218,7 +218,7 @@ func processTable(conv *internal.Conv, db *sql.DB, table schemaAndName) error {
 }
 
 func getColumns(table schemaAndName, db *sql.DB) (*sql.Rows, error) {
-	q := `SELECT c.column_name, c.data_type, c.column_type, c.is_nullable, c.column_default, c.character_maximum_length, c.numeric_precision, c.numeric_scale
+	q := `SELECT c.column_name, c.data_type, c.column_type, c.is_nullable, c.column_default, c.character_maximum_length, c.numeric_precision, c.numeric_scale, c.extra
               FROM information_schema.COLUMNS c
               where table_schema = ? and table_name = ? ORDER BY c.ordinal_position;`
 	return db.Query(q, table.schema, table.name)
@@ -228,10 +228,10 @@ func processColumns(conv *internal.Conv, cols *sql.Rows, constraints map[string]
 	colDefs := make(map[string]schema.Column)
 	var colNames []string
 	var colName, dataType, isNullable, columnType string
-	var colDefault sql.NullString
+	var colDefault, colExtra sql.NullString
 	var charMaxLen, numericPrecision, numericScale sql.NullInt64
 	for cols.Next() {
-		err := cols.Scan(&colName, &dataType, &columnType, &isNullable, &colDefault, &charMaxLen, &numericPrecision, &numericScale)
+		err := cols.Scan(&colName, &dataType, &columnType, &isNullable, &colDefault, &charMaxLen, &numericPrecision, &numericScale, &colExtra)
 		if err != nil {
 			conv.Unexpected(fmt.Sprintf("Can't scan: %v", err))
 			continue
@@ -251,6 +251,9 @@ func processColumns(conv *internal.Conv, cols *sql.Rows, constraints map[string]
 			}
 		}
 		ignored.Default = colDefault.Valid
+		if colExtra.String == "auto_increment" {
+			ignored.AutoIncrement = true
+		}
 		c := schema.Column{
 			Name:    colName,
 			Type:    toType(dataType, columnType, charMaxLen, numericPrecision, numericScale),
diff --git a/mysql/mysqldump.go b/mysql/mysqldump.go
index d9fd400..abacff1 100644
--- a/mysql/mysqldump.go
+++ b/mysql/mysqldump.go
@@ -36,6 +36,13 @@ var insertRegexp = regexp.MustCompile("INSERT\\sINTO\\s(.*?)\\sVALUES\\s")
 
 // MysqlSpatialDataTypes is an array of all MySQL spatial data types.
 var MysqlSpatialDataTypes = []string{"geometrycollection", "multipoint", "multilinestring", "multipolygon", "point", "linestring", "polygon", "geometry"}
+var spatialRegexps = func() []*regexp.Regexp {
+	l := make([]*regexp.Regexp, len(MysqlSpatialDataTypes))
+	for i, spatial := range MysqlSpatialDataTypes {
+		l[i] = regexp.MustCompile("(?i)" + " " + spatial)
+	}
+	return l
+}()
 
 // ProcessMySQLDump reads mysqldump data from r and does schema or data conversion,
 // depending on whether conv is configured for schema mode or data mode.
@@ -344,6 +351,8 @@ func updateColsByOption(conv *internal.Conv, tableName string, col *ast.ColumnDe
 			isPk = true
 		case ast.ColumnOptionNotNull:
 			column.NotNull = true
+		case ast.ColumnOptionAutoIncrement:
+			column.Ignored.AutoIncrement = true
 		case ast.ColumnOptionDefaultValue:
 			// If a data type specification includes no explicit DEFAULT
 			// value, MySQL determines if the column can take NULL as a value
@@ -423,7 +432,7 @@ func handleParseError(conv *internal.Conv, chunk string, err error, l [][]byte)
 				conv.Unexpected(fmt.Sprintf("Unsupported datatype '%s' encountered while parsing following statement at line number %d : \n%s", spatial, len(l), chunk))
 				internal.VerbosePrintf("Converting datatype '%s' to 'Text' and retrying to parse the statement\n", spatial)
 			}
-			return handleSpatialDatatype(conv, strings.ToLower(chunk), l)
+			return handleSpatialDatatype(conv, chunk, l)
 		}
 	}
 
@@ -465,8 +474,8 @@ func handleSpatialDatatype(conv *internal.Conv, chunk string, l [][]byte) ([]ast
 	if !conv.SchemaMode() {
 		return nil, true
 	}
-	for _, spatial := range MysqlSpatialDataTypes {
-		chunk = strings.ReplaceAll(chunk, " "+spatial, " text")
+	for _, spatialRegexp := range spatialRegexps {
+		chunk = spatialRegexp.ReplaceAllString(chunk, " text")
 	}
 	newTree, _, err := parser.New().Parse(chunk, "", "")
 	if err != nil {
diff --git a/mysql/toddl.go b/mysql/toddl.go
index 9608592..631e10e 100644
--- a/mysql/toddl.go
+++ b/mysql/toddl.go
@@ -62,6 +62,9 @@ func schemaToDDL(conv *internal.Conv) error {
 			if srcCol.Ignored.Default {
 				issues = append(issues, internal.DefaultValue)
 			}
+			if srcCol.Ignored.AutoIncrement {
+				issues = append(issues, internal.AutoIncrement)
+			}
 			if len(issues) > 0 {
 				conv.Issues[srcTable.Name][srcCol.Name] = issues
 			}
diff --git a/schema/schema.go b/schema/schema.go
index 0997813..efadac4 100644
--- a/schema/schema.go
+++ b/schema/schema.go
@@ -73,11 +73,12 @@ type Type struct {
 // represented. We drop the details, but retain presence/absence for
 // reporting purposes.
 type Ignored struct {
-	Check      bool
-	Identity   bool
-	Default    bool
-	Exclusion  bool
-	ForeignKey bool
+	Check         bool
+	Identity      bool
+	Default       bool
+	Exclusion     bool
+	ForeignKey    bool
+	AutoIncrement bool
 }
 
 // Print converts ty to a string suitable for printing.


Test patch:
diff --git a/mysql/infoschema_test.go b/mysql/infoschema_test.go
index fb4ee62..f397e3d 100644
--- a/mysql/infoschema_test.go
+++ b/mysql/infoschema_test.go
@@ -45,11 +45,11 @@ func TestProcessInfoSchemaMYSQL(t *testing.T) {
 		}, {
 			query: "SELECT (.+) FROM information_schema.COLUMNS (.+)",
 			args:  []driver.Value{"test", "cart"},
-			cols:  []string{"column_name", "data_type", "column_type", "is_nullable", "column_default", "character_maximum_length", "numeric_precision", "numeric_scale"},
+			cols:  []string{"column_name", "data_type", "column_type", "is_nullable", "column_default", "character_maximum_length", "numeric_precision", "numeric_scale", "extra"},
 			rows: [][]driver.Value{
-				{"productid", "text", "text", "NO", nil, nil, nil, nil},
-				{"userid", "text", "text", "NO", nil, nil, nil, nil},
-				{"quantity", "bigint", "bigint", "YES", nil, nil, 64, 0}},
+				{"productid", "text", "text", "NO", nil, nil, nil, nil, nil},
+				{"userid", "text", "text", "NO", nil, nil, nil, nil, nil},
+				{"quantity", "bigint", "bigint", "YES", nil, nil, 64, 0, nil}},
 		}, {
 			query: "SELECT (.+) FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS (.+)",
 			args:  []driver.Value{"test", "cart"},
@@ -60,33 +60,34 @@ func TestProcessInfoSchemaMYSQL(t *testing.T) {
 		}, {
 			query: "SELECT (.+) FROM information_schema.COLUMNS (.+)",
 			args:  []driver.Value{"test", "test"},
-			cols:  []string{"column_name", "data_type", "column_type", "is_nullable", "column_default", "character_maximum_length", "numeric_precision", "numeric_scale"},
+			cols:  []string{"column_name", "data_type", "column_type", "is_nullable", "column_default", "character_maximum_length", "numeric_precision", "numeric_scale", "extra"},
 			rows: [][]driver.Value{
-				{"id", "bigint", "bigint", "NO", nil, nil, 64, 0},
-				{"s", "set", "set", "YES", nil, nil, nil, nil},
-				{"txt", "text", "text", "NO", nil, nil, nil, nil},
-				{"b", "boolean", "boolean", "YES", nil, nil, nil, nil},
-				{"bs", "bigint", "bigint", "NO", "nextval('test11_bs_seq'::regclass)", nil, 64, 0},
-				{"bl", "blob", "blob", "YES", nil, nil, nil, nil},
-				{"c", "char", "char(1)", "YES", nil, 1, nil, nil},
-				{"c8", "char", "char(8)", "YES", nil, 8, nil, nil},
-				{"d", "date", "date", "YES", nil, nil, nil, nil},
-				{"f8", "double", "double", "YES", nil, nil, 53, nil},
-				{"f4", "float", "float", "YES", nil, nil, 24, nil},
-				{"i8", "bigint", "bigint", "YES", nil, nil, 64, 0},
-				{"i4", "integer", "integer", "YES", nil, nil, 32, 0},
-				{"i2", "smallint", "smallint", "YES", nil, nil, 16, 0},
-				{"si", "integer", "integer", "NO", "nextval('test11_s_seq'::regclass)", nil, 32, 0},
-				{"ts", "datetime", "datetime", "YES", nil, nil, nil, nil},
-				{"tz", "timestamp", "timestamp", "YES", nil, nil, nil, nil},
-				{"vc", "varchar", "varchar", "YES", nil, nil, nil, nil},
-				{"vc6", "varchar", "varchar(6)", "YES", nil, 6, nil, nil}},
+				{"id", "bigint", "bigint", "NO", nil, nil, 64, 0, nil},
+				{"s", "set", "set", "YES", nil, nil, nil, nil, nil},
+				{"txt", "text", "text", "NO", nil, nil, nil, nil, nil},
+				{"b", "boolean", "boolean", "YES", nil, nil, nil, nil, nil},
+				{"bs", "bigint", "bigint", "NO", "nextval('test11_bs_seq'::regclass)", nil, 64, 0, nil},
+				{"bl", "blob", "blob", "YES", nil, nil, nil, nil, nil},
+				{"c", "char", "char(1)", "YES", nil, 1, nil, nil, nil},
+				{"c8", "char", "char(8)", "YES", nil, 8, nil, nil, nil},
+				{"d", "date", "date", "YES", nil, nil, nil, nil, nil},
+				{"dec", "decimal", "decimal(20,5)", "YES", nil, nil, 20, 5, nil},
+				{"f8", "double", "double", "YES", nil, nil, 53, nil, nil},
+				{"f4", "float", "float", "YES", nil, nil, 24, nil, nil},
+				{"i8", "bigint", "bigint", "YES", nil, nil, 64, 0, nil},
+				{"i4", "integer", "integer", "YES", nil, nil, 32, 0, "auto_increment"},
+				{"i2", "smallint", "smallint", "YES", nil, nil, 16, 0, nil},
+				{"si", "integer", "integer", "NO", "nextval('test11_s_seq'::regclass)", nil, 32, 0, nil},
+				{"ts", "datetime", "datetime", "YES", nil, nil, nil, nil, nil},
+				{"tz", "timestamp", "timestamp", "YES", nil, nil, nil, nil, nil},
+				{"vc", "varchar", "varchar", "YES", nil, nil, nil, nil, nil},
+				{"vc6", "varchar", "varchar(6)", "YES", nil, 6, nil, nil, nil}},
 		},
 		{
 			query: "SELECT (.+) FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS (.+)",
 			args:  []driver.Value{"test", "test"},
 			cols:  []string{"column_name", "constraint_type"},
-			rows:  [][]driver.Value{{"id", "PRIMARY KEY"}},
+			rows:  [][]driver.Value{{"id", "PRIMARY KEY"}, {"id", "FOREIGN KEY"}},
 		},
 	}
 	db := mkMockDB(t, ms)
@@ -105,7 +106,7 @@ func TestProcessInfoSchemaMYSQL(t *testing.T) {
 			Pks: []ddl.IndexKey{ddl.IndexKey{Col: "productid"}, ddl.IndexKey{Col: "userid"}}},
 		"test": ddl.CreateTable{
 			Name:     "test",
-			ColNames: []string{"id", "s", "txt", "b", "bs", "bl", "c", "c8", "d", "f8", "f4", "i8", "i4", "i2", "si", "ts", "tz", "vc", "vc6"},
+			ColNames: []string{"id", "s", "txt", "b", "bs", "bl", "c", "c8", "d", "dec", "f8", "f4", "i8", "i4", "i2", "si", "ts", "tz", "vc", "vc6"},
 			ColDefs: map[string]ddl.ColumnDef{
 				"id":  ddl.ColumnDef{Name: "id", T: ddl.Int64{}, NotNull: true},
 				"s":   ddl.ColumnDef{Name: "s", T: ddl.String{Len: ddl.MaxLength{}}, IsArray: true},
@@ -116,6 +117,7 @@ func TestProcessInfoSchemaMYSQL(t *testing.T) {
 				"c":   ddl.ColumnDef{Name: "c", T: ddl.String{Len: ddl.Int64Length{Value: 1}}},
 				"c8":  ddl.ColumnDef{Name: "c8", T: ddl.String{Len: ddl.Int64Length{Value: 8}}},
 				"d":   ddl.ColumnDef{Name: "d", T: ddl.Date{}},
+				"dec": ddl.ColumnDef{Name: "dec", T: ddl.Float64{}},
 				"f8":  ddl.ColumnDef{Name: "f8", T: ddl.Float64{}},
 				"f4":  ddl.ColumnDef{Name: "f4", T: ddl.Float64{}},
 				"i8":  ddl.ColumnDef{Name: "i8", T: ddl.Int64{}},
@@ -132,12 +134,14 @@ func TestProcessInfoSchemaMYSQL(t *testing.T) {
 	assert.Equal(t, expectedSchema, stripSchemaComments(conv.SpSchema))
 	assert.Equal(t, len(conv.Issues["cart"]), 0)
 	expectedIssues := map[string][]internal.SchemaIssue{
-		"bs": []internal.SchemaIssue{internal.DefaultValue},
-		"f4": []internal.SchemaIssue{internal.Widened},
-		"i4": []internal.SchemaIssue{internal.Widened},
-		"i2": []internal.SchemaIssue{internal.Widened},
-		"si": []internal.SchemaIssue{internal.Widened, internal.DefaultValue},
-		"ts": []internal.SchemaIssue{internal.Datetime},
+		"id":  []internal.SchemaIssue{internal.ForeignKey},
+		"bs":  []internal.SchemaIssue{internal.DefaultValue},
+		"dec": []internal.SchemaIssue{internal.Decimal},
+		"f4":  []internal.SchemaIssue{internal.Widened},
+		"i4":  []internal.SchemaIssue{internal.Widened, internal.AutoIncrement},
+		"i2":  []internal.SchemaIssue{internal.Widened},
+		"si":  []internal.SchemaIssue{internal.Widened, internal.DefaultValue},
+		"ts":  []internal.SchemaIssue{internal.Datetime},
 	}
 	assert.Equal(t, expectedIssues, conv.Issues["test"])
 	assert.Equal(t, int64(0), conv.Unexpecteds())
@@ -211,11 +215,11 @@ func TestProcessSQLData_MultiCol(t *testing.T) {
 		}, {
 			query: "SELECT (.+) FROM information_schema.COLUMNS (.+)",
 			args:  []driver.Value{"test", "test"},
-			cols:  []string{"column_name", "data_type", "column_type", "is_nullable", "column_default", "character_maximum_length", "numeric_precision", "numeric_scale"},
+			cols:  []string{"column_name", "data_type", "column_type", "is_nullable", "column_default", "character_maximum_length", "numeric_precision", "numeric_scale", "extra"},
 			rows: [][]driver.Value{
-				{"a", "text", "text", "NO", nil, nil, nil, nil},
-				{"b", "double", "double", "YES", nil, nil, 53, nil},
-				{"c", "bigint", "bigint", "YES", nil, nil, 64, 0}},
+				{"a", "text", "text", "NO", nil, nil, nil, nil, nil},
+				{"b", "double", "double", "YES", nil, nil, 53, nil, nil},
+				{"c", "bigint", "bigint", "YES", nil, nil, 64, 0, nil}},
 		},
 		{
 			query: "SELECT (.+) FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS (.+)",