From 1d932555750fb7dfbbcaacc2ac8fe4b39ad567b8 Mon Sep 17 00:00:00 2001 From: Hein Date: Mon, 12 Oct 2026 22:04:40 +0200 Subject: [PATCH] fix(assets): improve error reporting for asset failures fix(embed): update embedDirectives to return result directly fix(executor): clean up import order and formatting fix(syntax_test): change file permission syntax to octal fix(mysql): update file permission syntax to octal fix(pgsql): update file permission syntax to octal fix(sqlex): simplify Walk function parameter handling fix(transform): adjust identifier comparison logic fix(template): clean up function formatting and error handling --- cmd/relspec/assets.go | 3 ++- pkg/assetloader/embed.go | 6 +++--- pkg/assetloader/executor.go | 4 ++-- pkg/assetloader/syntax_test.go | 2 +- pkg/readers/mysql/defaults.go | 4 ++-- pkg/sqllex/scan.go | 2 +- pkg/transform/identifiers.go | 2 +- pkg/transform/identifiers_test.go | 4 +++- pkg/writers/mysql/conversion_test.go | 2 +- pkg/writers/mysql/live_diagnostics_test.go | 8 +++++--- pkg/writers/mysql/live_roundtrip_test.go | 6 ++++-- pkg/writers/mysql/writer.go | 15 ++++++++++++++- pkg/writers/pgsql/migration_writer.go | 12 ++++++------ pkg/writers/pgsql/quoting_test.go | 2 +- pkg/writers/pgsql/writer.go | 15 +++++++-------- pkg/writers/sqlexec/writer.go | 6 +++--- pkg/writers/template/writer.go | 8 +++++++- 17 files changed, 63 insertions(+), 38 deletions(-) diff --git a/cmd/relspec/assets.go b/cmd/relspec/assets.go index 062171b..0b69256 100644 --- a/cmd/relspec/assets.go +++ b/cmd/relspec/assets.go @@ -203,7 +203,8 @@ func runAssetsExecute(cmd *cobra.Command, args []string) error { if len(failures) > 0 { fmt.Fprintf(os.Stderr, "Failed: %d\n", len(failures)) fmt.Fprintf(os.Stderr, "\n⚠ Failed Entries Summary (%d failed):\n", len(failures)) - for i, f := range failures { + for i := range failures { + f := &failures[i] fmt.Fprintf(os.Stderr, " %d. %s (Priority=%d, Sequence=%d)\n Error: %v\n", i+1, filepath.Base(f.item.Entry.File), f.item.Priority, f.item.Sequence, f.err) } diff --git a/pkg/assetloader/embed.go b/pkg/assetloader/embed.go index 0bfe274..aea5a0e 100644 --- a/pkg/assetloader/embed.go +++ b/pkg/assetloader/embed.go @@ -42,10 +42,9 @@ func ProcessEmbedDirectives(sqlPath, sql string) (string, error) { return ProcessEmbedDirectivesWithOptions(sqlPath, sql, EmbedOptions{}) } -func embedDirectives(sql string) (string, []string) { - var directives []string +func embedDirectives(sql string) (result string, directives []string) { // Only standalone line comments are directives, and only in SQL code. - result := sqllex.Walk(sql, nil, func(comment string) string { + result = sqllex.Walk(sql, nil, func(comment string) string { if match := embedDirectivePattern.FindStringSubmatch(comment); match != nil { directives = append(directives, match[1]) return "" @@ -54,6 +53,7 @@ func embedDirectives(sql string) (string, []string) { }) return result, directives } + func HasEmbedDirectives(sql string) bool { _, directives := embedDirectives(sql) return len(directives) > 0 diff --git a/pkg/assetloader/executor.go b/pkg/assetloader/executor.go index 6406c41..4d1f30c 100644 --- a/pkg/assetloader/executor.go +++ b/pkg/assetloader/executor.go @@ -3,9 +3,10 @@ package assetloader import ( "context" "fmt" - "git.warky.dev/wdevs/relspecgo/pkg/sqllex" "path/filepath" + "git.warky.dev/wdevs/relspecgo/pkg/sqllex" + "github.com/jackc/pgx/v5" ) @@ -22,7 +23,6 @@ import ( // placeholder (not built-in and not in staticParams) returns an error. // PostgreSQL cast syntax (::type) is left untouched. func BuildQuery(call string, fileBytes []byte, filename string, staticParams map[string]string) (query string, args []any, err error) { - paramIndex := map[string]int{} // name → 1-based position var firstErr error diff --git a/pkg/assetloader/syntax_test.go b/pkg/assetloader/syntax_test.go index f9e4a09..aeea315 100644 --- a/pkg/assetloader/syntax_test.go +++ b/pkg/assetloader/syntax_test.go @@ -23,7 +23,7 @@ func TestEmbedLiteralOnePassAndSQLContexts(t *testing.T) { dir := t.TempDir() data := "price $1 ${2} $HOME $$ :b 'quote' \\path" for file, text := range map[string]string{"a.txt": data, "b.txt": "Second"} { - if err := os.WriteFile(filepath.Join(dir, file), []byte(text), 0600); err != nil { + if err := os.WriteFile(filepath.Join(dir, file), []byte(text), 0o600); err != nil { t.Fatal(err) } } diff --git a/pkg/readers/mysql/defaults.go b/pkg/readers/mysql/defaults.go index 2f0954d..cdebf0a 100644 --- a/pkg/readers/mysql/defaults.go +++ b/pkg/readers/mysql/defaults.go @@ -50,7 +50,6 @@ func columnDefaults(create string) map[string]string { depth-- if depth == 0 { definitions = append(definitions, create[start:i]) - break } } if ch == ')' && depth == 0 { @@ -177,8 +176,9 @@ func autoIncrementValue(create string) string { } return "1" } + func unsupportedNativeOptions(create string) []string { - words := strings.FieldsFunc(strings.ToUpper(unquotedSQL(create)), func(r rune) bool { return !(r >= 'A' && r <= 'Z' || r == '_') }) + words := strings.FieldsFunc(strings.ToUpper(unquotedSQL(create)), func(r rune) bool { return (r < 'A' || r > 'Z') && r != '_' }) var out []string for i, w := range words { switch w { diff --git a/pkg/sqllex/scan.go b/pkg/sqllex/scan.go index 24ff200..ee71959 100644 --- a/pkg/sqllex/scan.go +++ b/pkg/sqllex/scan.go @@ -9,7 +9,7 @@ func ident(c byte) bool { return start(c) || c >= '0' && c <= '9' || c >= 128 } // Walk visits named parameters and standalone line comments in executable SQL. Quoted // strings/identifiers, dollar bodies, casts and nested block comments are opaque. // Returning replacement text is literal: it is never scanned again. -func Walk(sql string, parameter func(string) string, lineComment func(string) string) string { +func Walk(sql string, parameter, lineComment func(string) string) string { var out strings.Builder for i := 0; i < len(sql); { begin := i diff --git a/pkg/transform/identifiers.go b/pkg/transform/identifiers.go index c0746f6..afcf319 100644 --- a/pkg/transform/identifiers.go +++ b/pkg/transform/identifiers.go @@ -53,7 +53,7 @@ func LowercaseIdentifiers(db *models.Database) (*models.Database, error) { out.Field(i).SetString(strings.ToLower(value.String())) continue } - if !(v.Type() == reflect.TypeOf(models.Index{}) && field == "Columns") && listFields[field] && value.Kind() == reflect.Slice && value.Type().Elem().Kind() == reflect.String { + if (v.Type() != reflect.TypeOf(models.Index{}) || field != "Columns") && listFields[field] && value.Kind() == reflect.Slice && value.Type().Elem().Kind() == reflect.String { list := reflect.MakeSlice(value.Type(), value.Len(), value.Len()) for j := 0; j < value.Len(); j++ { list.Index(j).SetString(strings.ToLower(value.Index(j).String())) diff --git a/pkg/transform/identifiers_test.go b/pkg/transform/identifiers_test.go index 1b347c7..3fdf4f9 100644 --- a/pkg/transform/identifiers_test.go +++ b/pkg/transform/identifiers_test.go @@ -1,8 +1,9 @@ package transform import ( - "git.warky.dev/wdevs/relspecgo/pkg/models" "testing" + + "git.warky.dev/wdevs/relspecgo/pkg/models" ) func TestLowercaseIdentifiersReferencesAndData(t *testing.T) { @@ -35,6 +36,7 @@ func TestLowercaseIdentifiersReferencesAndData(t *testing.T) { t.Fatal("input mutated") } } + func TestLowercaseIdentifierCollisions(t *testing.T) { db := models.InitDatabase("db") s := models.InitSchema("app") diff --git a/pkg/writers/mysql/conversion_test.go b/pkg/writers/mysql/conversion_test.go index af35c83..a7703d4 100644 --- a/pkg/writers/mysql/conversion_test.go +++ b/pkg/writers/mysql/conversion_test.go @@ -24,7 +24,7 @@ func TestStrictConversionPreflightsAllObjects(t *testing.T) { unsupported.Indexes[index.Name] = index db.Schemas[0].Tables = append(db.Schemas[0].Tables, unsupported) out := filepath.Join(t.TempDir(), "existing.sql") - if err := os.WriteFile(out, []byte("keep existing contents"), 0600); err != nil { + if err := os.WriteFile(out, []byte("keep existing contents"), 0o600); err != nil { t.Fatal(err) } opts := &writers.WriterOptions{StrictConversion: true, OutputPath: out} diff --git a/pkg/writers/mysql/live_diagnostics_test.go b/pkg/writers/mysql/live_diagnostics_test.go index 2ecacfe..12bd597 100644 --- a/pkg/writers/mysql/live_diagnostics_test.go +++ b/pkg/writers/mysql/live_diagnostics_test.go @@ -3,14 +3,16 @@ package mysql_test import ( "database/sql" "fmt" - "git.warky.dev/wdevs/relspecgo/pkg/models" - driver "github.com/go-sql-driver/mysql" "os" "path/filepath" "strings" "testing" "time" + driver "github.com/go-sql-driver/mysql" + + "git.warky.dev/wdevs/relspecgo/pkg/models" + "git.warky.dev/wdevs/relspecgo/pkg/readers" rmysql "git.warky.dev/wdevs/relspecgo/pkg/readers/mysql" "git.warky.dev/wdevs/relspecgo/pkg/writers" @@ -50,7 +52,7 @@ func TestLiveUnsupportedObjectsDiagnosed(t *testing.T) { t.Fatal(err) } path := filepath.Join(t.TempDir(), "schema.sql") - if err := os.WriteFile(path, []byte("original"), 0600); err != nil { + if err := os.WriteFile(path, []byte("original"), 0o600); err != nil { t.Fatal(err) } err = wmysql.NewWriter(&writers.WriterOptions{StrictConversion: true, OutputPath: path}).WriteDatabase(model) diff --git a/pkg/writers/mysql/live_roundtrip_test.go b/pkg/writers/mysql/live_roundtrip_test.go index e0033b6..f77ae00 100644 --- a/pkg/writers/mysql/live_roundtrip_test.go +++ b/pkg/writers/mysql/live_roundtrip_test.go @@ -4,7 +4,6 @@ import ( "database/sql" "encoding/json" "fmt" - "gopkg.in/yaml.v3" "os" "reflect" "sort" @@ -12,12 +11,15 @@ import ( "testing" "time" + "gopkg.in/yaml.v3" + + driver "github.com/go-sql-driver/mysql" + "git.warky.dev/wdevs/relspecgo/pkg/models" "git.warky.dev/wdevs/relspecgo/pkg/readers" rmysql "git.warky.dev/wdevs/relspecgo/pkg/readers/mysql" "git.warky.dev/wdevs/relspecgo/pkg/writers" wmysql "git.warky.dev/wdevs/relspecgo/pkg/writers/mysql" - driver "github.com/go-sql-driver/mysql" ) func TestLiveSchemaRoundTrip(t *testing.T) { diff --git a/pkg/writers/mysql/writer.go b/pkg/writers/mysql/writer.go index ab5f961..87605bb 100644 --- a/pkg/writers/mysql/writer.go +++ b/pkg/writers/mysql/writer.go @@ -10,11 +10,12 @@ import ( "strconv" "strings" + _ "github.com/go-sql-driver/mysql" + "git.warky.dev/wdevs/relspecgo/pkg/mariadb" "git.warky.dev/wdevs/relspecgo/pkg/models" "git.warky.dev/wdevs/relspecgo/pkg/transform" "git.warky.dev/wdevs/relspecgo/pkg/writers" - _ "github.com/go-sql-driver/mysql" ) type Writer struct { @@ -53,6 +54,7 @@ func (w *Writer) Statements(db *models.Database) ([]string, error) { } return databaseStatements(db), nil } + func (w *Writer) WriteDatabase(db *models.Database) error { statements, err := w.Statements(db) if err != nil { @@ -78,6 +80,7 @@ func (w *Writer) WriteDatabase(db *models.Database) error { } return closeErr } + func writeStatements(out io.Writer, statements []string, name string) error { name = strings.NewReplacer("\n", " ", "\r", " ").Replace(name) if _, err := fmt.Fprintf(out, "-- Database: %s\n", name); err != nil { @@ -93,9 +96,11 @@ func writeStatements(out io.Writer, statements []string, name string) error { } return nil } + func (w *Writer) WriteSchema(s *models.Schema) error { return w.WriteDatabase(&models.Database{Schemas: []*models.Schema{s}}) } + func (w *Writer) WriteTable(t *models.Table) error { return w.WriteSchema(&models.Schema{Name: "", Tables: []*models.Table{t}}) } @@ -106,6 +111,7 @@ func qualified(schema, name string) string { } return quote(schema) + "." + quote(name) } + func keys[T any](m map[string]T) []string { out := make([]string, 0, len(m)) for k := range m { @@ -122,9 +128,11 @@ func metaBool(m map[string]any, key string, def bool) bool { } return def } + func signature(c *models.Column) string { return fmt.Sprintf("%s/%d/%d/%d", c.Type, c.Length, c.Precision, c.Scale) } + func columnType(c *models.Column) string { if native := metaString(c.Metadata, "mysql_type"); native != "" && metaString(c.Metadata, "mysql_type_signature") == signature(c) { return native @@ -152,6 +160,7 @@ func columnType(c *models.Column) string { func literal(s string) string { return "'" + strings.ReplaceAll(strings.ReplaceAll(s, "\\", "\\\\"), "'", "''") + "'" } + func defaultSQL(c *models.Column) string { value := fmt.Sprint(c.Default) if native := metaString(c.Metadata, "mysql_default_sql"); native != "" && metaString(c.Metadata, "mysql_default_value") == value { @@ -185,6 +194,7 @@ func defaultSQL(c *models.Column) string { } return literal(value) } + func indexParts(i *models.Index) string { if len(i.KeyParts) == 0 { return quoted(i.Columns) @@ -205,6 +215,7 @@ func indexParts(i *models.Index) string { } return strings.Join(parts, ", ") } + func tableStatement(s *models.Schema, t *models.Table) string { schema := "" if s != nil { @@ -346,6 +357,7 @@ func tableStatement(s *models.Schema, t *models.Table) string { } return "CREATE TABLE IF NOT EXISTS " + qualified(schema, t.Name) + " (\n" + strings.Join(defs, ",\n") + "\n)" + tail + ";\n\n" } + func databaseStatements(db *models.Database) []string { var statements []string for _, s := range db.Schemas { @@ -378,6 +390,7 @@ func databaseStatements(db *models.Database) []string { } return statements } + func (w *Writer) execute(statements []string, conn string) error { if w.options.AtomicExecution { return fmt.Errorf("MySQL DDL implicitly commits; atomic execution is unsupported") diff --git a/pkg/writers/pgsql/migration_writer.go b/pkg/writers/pgsql/migration_writer.go index 3cfdadd..3cf643b 100644 --- a/pkg/writers/pgsql/migration_writer.go +++ b/pkg/writers/pgsql/migration_writer.go @@ -848,7 +848,7 @@ func adoptableUniqueIndex(table *models.Table, constraint *models.Constraint) st return "" } for i, col := range index.Columns { - if !(col == constraint.Columns[i]) { + if col != constraint.Columns[i] { return "" } } @@ -1239,21 +1239,21 @@ func constraintsEqual(c1, c2 *models.Constraint) bool { return false } for i, col := range c1.Columns { - if !(col == c2.Columns[i]) { + if col != c2.Columns[i] { return false } } // For foreign keys, also compare referenced table and columns if c1.Type == models.ForeignKeyConstraint { - if !(c1.ReferencedTable == c2.ReferencedTable) { + if c1.ReferencedTable != c2.ReferencedTable { return false } if len(c1.ReferencedColumns) != len(c2.ReferencedColumns) { return false } for i, col := range c1.ReferencedColumns { - if !(col == c2.ReferencedColumns[i]) { + if col != c2.ReferencedColumns[i] { return false } } @@ -1283,7 +1283,7 @@ func indexesEqual(idx1, idx2 *models.Index) bool { return false } for i, col := range idx1.Columns { - if !(col == idx2.Columns[i]) { + if col != idx2.Columns[i] { return false } } @@ -1365,7 +1365,7 @@ func primaryKeyColumnsMatch(modelTable *models.Table, current *models.Constraint return false } for i, col := range modelCols { - if !(col == current.Columns[i]) { + if col != current.Columns[i] { return false } } diff --git a/pkg/writers/pgsql/quoting_test.go b/pkg/writers/pgsql/quoting_test.go index 1a3396f..e9958bc 100644 --- a/pkg/writers/pgsql/quoting_test.go +++ b/pkg/writers/pgsql/quoting_test.go @@ -41,7 +41,7 @@ func TestQuotedNamesAndLowercaseOption(t *testing.T) { t.Fatal("input changed") } path := filepath.Join(t.TempDir(), "out.sql") - if err := os.WriteFile(path, []byte("keep"), 0600); err != nil { + if err := os.WriteFile(path, []byte("keep"), 0o600); err != nil { t.Fatal(err) } col.Name = "bad\x00name" diff --git a/pkg/writers/pgsql/writer.go b/pkg/writers/pgsql/writer.go index 8bc584e..bd389ec 100644 --- a/pkg/writers/pgsql/writer.go +++ b/pkg/writers/pgsql/writer.go @@ -29,11 +29,13 @@ type Writer struct { } // Shared execution results retain the PostgreSQL package's public type names. -type ExecutionReport = writers.ExecutionReport -type ExecutionFailure = writers.ExecutionFailure -type SchemaReport = writers.SchemaReport -type TableReport = writers.TableReport -type ExecutionError = writers.ExecutionError +type ( + ExecutionReport = writers.ExecutionReport + ExecutionFailure = writers.ExecutionFailure + SchemaReport = writers.SchemaReport + TableReport = writers.TableReport + ExecutionError = writers.ExecutionError +) // NewWriter creates a new PostgreSQL SQL writer func NewWriter(options *writers.WriterOptions) *Writer { @@ -2114,7 +2116,6 @@ func extractSequenceName(defaultExpr string) string { name.WriteByte(defaultExpr[i]) } return "" - } // executeDatabaseSQL applies db to a PostgreSQL database. By default it reads the live schema @@ -2497,7 +2498,6 @@ func parseQualifiedIdent(s string) (schema, name string) { } } return "", stripQuotes(token) - } // firstBareIdent returns the first whitespace/delimiter-terminated token, stripping outer quotes. @@ -2520,7 +2520,6 @@ func firstBareIdent(s string) string { } } return s - } // firstIdentAfterKeyword returns the first identifier token after keyword (matched case-insensitively via upperStmt). diff --git a/pkg/writers/sqlexec/writer.go b/pkg/writers/sqlexec/writer.go index 62dd081..1fff6ac 100644 --- a/pkg/writers/sqlexec/writer.go +++ b/pkg/writers/sqlexec/writer.go @@ -46,9 +46,9 @@ func (w *Writer) WriteDatabase(db *models.Database) error { var scripts []*models.Script for _, schema := range db.Schemas { for _, script := range sortedScripts(schema.Scripts) { - copy := *script - copy.Name = schema.Name + "." + script.Name - scripts = append(scripts, ©) + scriptCopy := *script + scriptCopy.Name = schema.Name + "." + script.Name + scripts = append(scripts, &scriptCopy) } } diff --git a/pkg/writers/template/writer.go b/pkg/writers/template/writer.go index ef158dc..8579c71 100644 --- a/pkg/writers/template/writer.go +++ b/pkg/writers/template/writer.go @@ -137,6 +137,7 @@ func (w *Writer) executeSchemaMode(db *models.Database) error { } return w.executeFiles(data) } + func (w *Writer) executeDomainMode(db *models.Database) error { var data []*TemplateData for _, domain := range db.Domains { @@ -144,6 +145,7 @@ func (w *Writer) executeDomainMode(db *models.Database) error { } return w.executeFiles(data) } + func (w *Writer) executeScriptMode(db *models.Database) error { var data []*TemplateData for _, schema := range db.Schemas { @@ -153,6 +155,7 @@ func (w *Writer) executeScriptMode(db *models.Database) error { } return w.executeFiles(data) } + func (w *Writer) executeTableMode(db *models.Database) error { var data []*TemplateData for _, schema := range db.Schemas { @@ -246,7 +249,10 @@ func (w *Writer) writeArtifact(root *os.Root, file artifact) error { if err != nil { return err } - defer root.Remove(temporary) + defer func() { + // Best-effort cleanup; a successful rename already removes the staging path. + _ = root.Remove(temporary) + }() _, writeErr := staged.WriteString(file.content) if err := errors.Join(writeErr, staged.Close()); err != nil { return err