fix: convert EVERY column in a MySQL baseline, not just the first per line
The quoting fix was necessary but not sufficient: "column 'op_name' can't have a default value" persisted. The translation worked line by line and rewrote only the FIRST column on each line. Our own migrations put one column per line, so nothing showed there — but sqlite_master stores a table's CREATE statement with every ALTER-added column appended to the SAME line, and the baseline is built from exactly that. On a logbook that had been through upgrades, most columns therefore kept their TEXT type and MySQL rejected the schema. Two consequences fixed together: every match is now rewritten, and each one is decided from ITS OWN declaration — the text between the surrounding commas — rather than from the whole line. Deciding per line would have made every column sharing a line with a default-bearing one into VARCHAR(255), which breaches the InnoDB row-size limit the TEXT rule exists to respect. Tests cover the many-columns-on-one-line shape and pin that a neighbour's DEFAULT does not leak.
This commit is contained in:
+45
-14
@@ -41,8 +41,8 @@ var (
|
||||
// baseline as TEXT. That produced both "BLOB/TEXT column 'state' used in key
|
||||
// specification without a key length" and "column 'op_name' can't have a
|
||||
// default value" on a MySQL logbook, neither of which names the real cause.
|
||||
reColText = regexp.MustCompile("(`?)(\\w+)(`?)\\s+TEXT\\b")
|
||||
reKeyColumn = regexp.MustCompile(`(?m)^(\s*)key\b`)
|
||||
reColText = regexp.MustCompile("(`?)(\\w+)(`?)\\s+TEXT\\b")
|
||||
reKeyColumn = regexp.MustCompile(`(?m)^(\s*)key\b`)
|
||||
// SQLite quotes identifiers with double quotes — e.g. a table renamed by
|
||||
// ALTER … RENAME TO ends up as CREATE TABLE "name" in sqlite_master. MySQL
|
||||
// uses backticks. Our schema has no double-quoted string literals, so it's
|
||||
@@ -109,28 +109,59 @@ func mysqlDDL(stmt string) string {
|
||||
// right MySQL type, deciding per line so it can see whether the line carries a
|
||||
// DEFAULT or an inline PRIMARY KEY. See varcharColumns / longTextColumns.
|
||||
func translateTextColumns(s string) string {
|
||||
lines := strings.Split(s, "\n")
|
||||
for i, line := range lines {
|
||||
m := reColText.FindStringSubmatchIndex(line)
|
||||
if m == nil {
|
||||
continue
|
||||
}
|
||||
// Groups: 1 = opening backtick (or empty), 2 = the name, 3 = closing one.
|
||||
openQ, col, closeQ := line[m[2]:m[3]], line[m[4]:m[5]], line[m[6]:m[7]]
|
||||
// EVERY column declaration is handled, wherever it sits.
|
||||
//
|
||||
// This used to work line by line and convert only the FIRST column on each
|
||||
// line. That is fine for our hand-written migrations, which put one column
|
||||
// per line, and wrong for the SQLite baseline: sqlite_master stores a table's
|
||||
// CREATE statement with every ALTER-added column appended to the SAME line,
|
||||
// so on a real logbook only one of them was converted and MySQL rejected the
|
||||
// rest — "column 'op_name' can't have a default value".
|
||||
//
|
||||
// Each match is also decided from ITS OWN declaration — the text between the
|
||||
// surrounding commas — rather than from the whole line, so a DEFAULT
|
||||
// belonging to a neighbouring column cannot decide this one's type.
|
||||
out := make([]byte, 0, len(s)+64)
|
||||
last := 0
|
||||
for _, m := range reColText.FindAllStringSubmatchIndex(s, -1) {
|
||||
openQ, col, closeQ := s[m[2]:m[3]], s[m[4]:m[5]], s[m[6]:m[7]]
|
||||
decl := declarationAround(s, m[0], m[1])
|
||||
var typ string
|
||||
switch {
|
||||
case longTextColumns[col] != "":
|
||||
typ = longTextColumns[col]
|
||||
case varcharColumns[col],
|
||||
strings.Contains(line, "DEFAULT"),
|
||||
strings.Contains(line, "PRIMARY KEY"):
|
||||
strings.Contains(decl, "DEFAULT"),
|
||||
strings.Contains(decl, "PRIMARY KEY"):
|
||||
typ = "VARCHAR(255)"
|
||||
default:
|
||||
typ = "TEXT"
|
||||
}
|
||||
lines[i] = line[:m[0]] + openQ + col + closeQ + " " + typ + line[m[1]:]
|
||||
out = append(out, s[last:m[0]]...)
|
||||
out = append(out, (openQ + col + closeQ + " " + typ)...)
|
||||
last = m[1]
|
||||
}
|
||||
return strings.Join(lines, "\n")
|
||||
return string(append(out, s[last:]...))
|
||||
}
|
||||
|
||||
// declarationAround returns the single column declaration containing [start,end)
|
||||
// — the text between the commas, brackets or newlines either side.
|
||||
//
|
||||
// Types are decided from this rather than from the whole line because several
|
||||
// declarations share a line in the SQLite baseline, and a neighbour's DEFAULT
|
||||
// must not decide our column's type.
|
||||
func declarationAround(s string, start, end int) string {
|
||||
from := strings.LastIndexAny(s[:start], ",(\n")
|
||||
if from < 0 {
|
||||
from = 0
|
||||
}
|
||||
to := strings.IndexAny(s[end:], ",)\n")
|
||||
if to < 0 {
|
||||
to = len(s)
|
||||
} else {
|
||||
to += end
|
||||
}
|
||||
return s[from:to]
|
||||
}
|
||||
|
||||
// OpenMySQL opens the shared MySQL logbook, creating the database if needed,
|
||||
|
||||
@@ -205,3 +205,48 @@ func TestMySQLDDL_QuotedTextColumns(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A CREATE TABLE with several columns on ONE line — the shape sqlite_master
|
||||
// stores once columns have been added by ALTER TABLE.
|
||||
//
|
||||
// The old translation converted only the first column per line, so on a real
|
||||
// logbook the baseline kept most of its columns as TEXT and MySQL refused the
|
||||
// schema: "column 'op_name' can't have a default value". Our own migration files
|
||||
// hide this — they put one column per line — which is why it only ever appeared
|
||||
// against a database that had been through upgrades.
|
||||
func TestMySQLDDL_ManyColumnsOnOneLine(t *testing.T) {
|
||||
in := "CREATE TABLE station_profiles (id INTEGER PRIMARY KEY, `name` TEXT, " +
|
||||
"`op_name` TEXT NOT NULL DEFAULT '', `comment` TEXT, `state` TEXT);"
|
||||
got := mysqlDDL(in)
|
||||
|
||||
// Default-bearing and indexed columns must be VARCHAR…
|
||||
for _, want := range []string{"`op_name` VARCHAR(255)", "`state` VARCHAR(255)"} {
|
||||
if !strings.Contains(got, want) {
|
||||
t.Errorf("missing %q in:\n %s", want, got)
|
||||
}
|
||||
}
|
||||
// …while the plain ones stay TEXT, or the row-size limit is breached.
|
||||
for _, want := range []string{"`name` TEXT", "`comment` TEXT"} {
|
||||
if !strings.Contains(got, want) {
|
||||
t.Errorf("missing %q in:\n %s", want, got)
|
||||
}
|
||||
}
|
||||
// No column may be left as a bare TEXT that carries a default.
|
||||
if strings.Contains(got, "TEXT NOT NULL DEFAULT") {
|
||||
t.Errorf("a TEXT column still carries a DEFAULT — MySQL rejects that:\n %s", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A neighbour's DEFAULT must not decide this column's type: deciding per LINE
|
||||
// made every column on a shared line VARCHAR as soon as one of them had a
|
||||
// default, which is how the row-size limit gets breached quietly.
|
||||
func TestMySQLDDL_DefaultDoesNotLeakAcrossColumns(t *testing.T) {
|
||||
in := "CREATE TABLE t (`a` TEXT NOT NULL DEFAULT '', `b` TEXT);"
|
||||
got := mysqlDDL(in)
|
||||
if !strings.Contains(got, "`a` VARCHAR(255)") {
|
||||
t.Errorf("the column WITH the default should be VARCHAR:\n %s", got)
|
||||
}
|
||||
if !strings.Contains(got, "`b` TEXT") {
|
||||
t.Errorf("the column without one should stay TEXT:\n %s", got)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user