From dd97413b8ebb17b92d88d4b9e04a45f4f2dcc3a2 Mon Sep 17 00:00:00 2001 From: Hein Date: Mon, 12 Oct 2026 20:59:46 +0200 Subject: [PATCH] fix: contain generated files and assets and harden SQL rendering --- README.md | 55 ++++ cmd/relspec/convert.go | 10 + cmd/relspec/job.go | 17 + cmd/relspec/lowercase_test.go | 82 +++++ cmd/relspec/merge.go | 9 + cmd/relspec/prisma_options.go | 19 +- cmd/relspec/root.go | 21 +- cmd/relspec/templ.go | 8 + docs/JOB_FILES.md | 1 + docs/SCRIPTS_COMMAND.md | 32 +- docs/TEMPLATE_MODE.md | 10 + docs/audit_1.md | 16 + pkg/assetloader/embed.go | 111 ++++++- pkg/assetloader/embed_test.go | 111 +++++++ pkg/assetloader/executor.go | 21 +- pkg/assetloader/files.go | 36 ++ pkg/assetloader/loader.go | 68 ++-- pkg/assetloader/syntax_test.go | 60 ++++ pkg/jobs/jobs.go | 7 +- pkg/readers/sqldir/reader.go | 2 +- pkg/safefile/root.go | 136 ++++++++ pkg/sqllex/scan.go | 101 ++++++ pkg/sqllex/scan_test.go | 28 ++ pkg/transform/identifiers.go | 157 +++++++++ pkg/transform/identifiers_test.go | 51 +++ pkg/writers/mysql/writer.go | 45 +-- pkg/writers/mysql/writer_test.go | 30 +- pkg/writers/pgsql/README.md | 22 ++ pkg/writers/pgsql/column_comment_test.go | 9 +- pkg/writers/pgsql/diff_statements_test.go | 22 +- pkg/writers/pgsql/directives_test.go | 4 +- pkg/writers/pgsql/generated_column_test.go | 4 +- pkg/writers/pgsql/identifiers.go | 116 +++++++ pkg/writers/pgsql/migration_writer.go | 80 +++-- pkg/writers/pgsql/migration_writer_test.go | 54 +-- pkg/writers/pgsql/quoting_test.go | 251 ++++++++++++++ pkg/writers/pgsql/serial_sequence_test.go | 2 +- pkg/writers/pgsql/statement_helpers_test.go | 2 +- pkg/writers/pgsql/template_functions.go | 46 +-- pkg/writers/pgsql/template_functions_test.go | 6 +- pkg/writers/pgsql/templates.go | 223 ++++--------- .../alter_column_default_with_check.tmpl | 10 +- .../alter_column_nullability_with_check.tmpl | 10 +- .../alter_column_type_with_check.tmpl | 4 +- .../pgsql/templates/audit_function.tmpl | 32 +- pkg/writers/pgsql/templates/audit_tables.tmpl | 48 +-- .../pgsql/templates/audit_trigger.tmpl | 14 +- .../pgsql/templates/base_constraint.tmpl | 28 +- pkg/writers/pgsql/templates/base_ddl.tmpl | 16 +- .../pgsql/templates/comment_column.tmpl | 2 +- .../pgsql/templates/comment_table.tmpl | 2 +- .../templates/create_check_constraint.tmpl | 10 +- .../create_foreign_key_with_check.tmpl | 10 +- .../pgsql/templates/create_primary_key.tmpl | 10 +- ...create_primary_key_with_autogen_check.tmpl | 14 +- .../templates/create_unique_constraint.tmpl | 10 +- pkg/writers/pgsql/templates/fragments.tmpl | 4 +- .../pgsql/templates/set_sequence_value.tmpl | 10 +- pkg/writers/pgsql/type_conversion_test.go | 4 +- pkg/writers/pgsql/writer.go | 309 +++++++++++------- pkg/writers/pgsql/writer_test.go | 78 ++--- pkg/writers/sqlexec/README.md | 8 + pkg/writers/sqlexec/writer.go | 17 +- pkg/writers/sqlexec/writer_test.go | 27 +- pkg/writers/template/README.md | 21 +- pkg/writers/template/writer.go | 216 ++++++------ pkg/writers/template/writer_modes_test.go | 124 ++++++- pkg/writers/writer.go | 3 + 68 files changed, 2328 insertions(+), 798 deletions(-) create mode 100644 cmd/relspec/lowercase_test.go create mode 100644 pkg/assetloader/files.go create mode 100644 pkg/assetloader/syntax_test.go create mode 100644 pkg/safefile/root.go create mode 100644 pkg/sqllex/scan.go create mode 100644 pkg/sqllex/scan_test.go create mode 100644 pkg/transform/identifiers.go create mode 100644 pkg/transform/identifiers_test.go create mode 100644 pkg/writers/pgsql/identifiers.go create mode 100644 pkg/writers/pgsql/quoting_test.go diff --git a/README.md b/README.md index 5dad0f3..7f03ac7 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,9 @@ relspec convert --from pgsql --from-conn "postgres://user:pass@localhost/mydb" \ # DBML → PostgreSQL DDL relspec convert --from dbml --from-path schema.dbml --to pgsql --to-path schema.sql +# Lowercase model names and structured references before output +relspec convert --from dbml --from-path schema.dbml --to pgsql --to-path schema.sql --lowercase + # PostgreSQL → SQLite (auto flattens schemas) relspec convert --from pgsql --from-conn "postgres://..." --to sqlite --to-path schema.sql @@ -57,6 +60,25 @@ connections or `--dry-run`). Source files are polled every `--watch-interval` is ignored so generating into the source tree does not loop. Conversion errors are printed and watching continues. +#### Identifier casing and PostgreSQL quoting + +PostgreSQL output always double-quotes model identifiers, including reserved +words, and preserves their case by default. For example, schema `Sales` and table +`Order` produce `"Sales"."Order"`; with `--lowercase`, they produce +`"sales"."order"`. Quoted names are case-sensitive in PostgreSQL. + +`--lowercase` is available on `convert`, `batch`, `merge`, `split`, `templ`, and +`job run`. It lowercases model names and structured references on a copy and +rejects collisions such as `Users` and `users` in the same schema. SQL expressions, +defaults, comments, enum values, and data retain their case. Adjust identifier +references inside your own SQL expressions when opting into lowercasing. + +Generated PostgreSQL SQL also escapes identifier quotes and literal values and +chooses dollar-quote delimiters that do not collide with function or procedural +block contents. Raw SQL expressions remain executable input. See the +[PostgreSQL writer documentation](pkg/writers/pgsql/README.md#identifier-and-value-handling) +for library options and details. + ### `batch` — Convert many inputs in one run Converts each input independently (one output per input, unlike `--from-list` @@ -165,6 +187,7 @@ jobs: overwrite: true options: flatten_schema: false + lowercase_identifiers: true # opt in; defaults to false logfile: .relspec/log/build-schema.log ``` @@ -177,6 +200,38 @@ any job runs. Path-like fields also support `${NAME}` environment-variable references, which are expanded and safety-checked during pre-flight. See [docs/JOB_FILES.md](docs/JOB_FILES.md). +Use `options.lowercase_identifiers: true` per job, or +`relspec job run build-schema --lowercase` to enable lowercasing for schema outputs +throughout the selected job plan. + +### `scripts` — SQL scripts and embedded assets + +```bash +relspec scripts list --dir ./migrations +relspec scripts execute --dir ./migrations --conn "$DATABASE_URL" +``` + +Execution stops on the first error by default. `--continue-on-error` (alias +`--coe`) attempts later scripts and still exits nonzero if any script fails. + +Scripts can embed nearby files using a standalone directive comment: + +```sql +-- @embed: path=assets/message.txt var=:message mode=text +INSERT INTO messages (body) VALUES (:message); +``` + +Embed replacement preserves dollar text such as `$1`, `${2}`, and `$HOME` exactly. +Placeholders are replaced only in executable SQL; quoted strings, quoted +identifiers, comments, casts, and dollar-quoted bodies retain their contents. +Inserted content is never scanned for further placeholders. Asset-call named +parameters follow the same SQL lexical boundaries. See +[docs/SCRIPTS_COMMAND.md](docs/SCRIPTS_COMMAND.md) for file containment, limits, +binary embedding, and execution details. + +MySQL DDL execution keeps each generated statement whole, preserving multiline +defaults containing semicolons or comment-like text. + ### `edit` — Interactive TUI editor ```bash diff --git a/cmd/relspec/convert.go b/cmd/relspec/convert.go index ccfdaed..9853ff9 100644 --- a/cmd/relspec/convert.go +++ b/cmd/relspec/convert.go @@ -28,6 +28,7 @@ import ( "git.warky.dev/wdevs/relspecgo/pkg/readers/sqlite" "git.warky.dev/wdevs/relspecgo/pkg/readers/typeorm" "git.warky.dev/wdevs/relspecgo/pkg/readers/yaml" + "git.warky.dev/wdevs/relspecgo/pkg/transform" "git.warky.dev/wdevs/relspecgo/pkg/writers" wbun "git.warky.dev/wdevs/relspecgo/pkg/writers/bun" wdbml "git.warky.dev/wdevs/relspecgo/pkg/writers/dbml" @@ -449,6 +450,15 @@ func readDatabaseForConvert(dbType, filePath, connString string) (*models.Databa } func writeDatabase(db *models.Database, dbType, outputPath, packageName, schemaFilter string, flattenSchema bool, nullableTypes, nullableArrays string, continueOnError bool, extraFields string) error { + if lowercaseIdentifiers { + var err error + schemaFilter = strings.ToLower(schemaFilter) + db, err = transform.LowercaseIdentifiers(db) + if err != nil { + return err + } + } + var writer writers.Writer writerOpts := newWriterOptions(outputPath, packageName, flattenSchema, nullableTypes, nullableArrays, continueOnError) diff --git a/cmd/relspec/job.go b/cmd/relspec/job.go index e201e0d..327aa45 100644 --- a/cmd/relspec/job.go +++ b/cmd/relspec/job.go @@ -19,6 +19,7 @@ import ( "git.warky.dev/wdevs/relspecgo/pkg/models" "git.warky.dev/wdevs/relspecgo/pkg/readers" "git.warky.dev/wdevs/relspecgo/pkg/readers/sqldir" + "git.warky.dev/wdevs/relspecgo/pkg/transform" "git.warky.dev/wdevs/relspecgo/pkg/writers" wpgsql "git.warky.dev/wdevs/relspecgo/pkg/writers/pgsql" "git.warky.dev/wdevs/relspecgo/pkg/writers/sqlexec" @@ -522,6 +523,12 @@ func runTemplJob(rj *resolvedJob, lg *jobLogger) error { return fmt.Errorf("schema not found: %s", schema) } } + if lowercaseIdentifiers || rj.job.Options.LowercaseIdentifiers { + db, err = transform.LowercaseIdentifiers(db) + if err != nil { + return err + } + } mode := rj.job.Mode if mode == "" { mode = "database" @@ -837,7 +844,17 @@ func inputLabel(ri resolvedInput) string { // writeJobOutput writes db to the job's output target (file or database). func writeJobOutput(rj *resolvedJob, db *models.Database, lg *jobLogger) error { + if lowercaseIdentifiers || rj.job.Options.LowercaseIdentifiers { + var err error + db, err = transform.LowercaseIdentifiers(db) + if err != nil { + return err + } + } o := rj.job.Options + if lowercaseIdentifiers || o.LowercaseIdentifiers { + o.Schema = strings.ToLower(o.Schema) + } format := strings.ToLower(rj.job.Output.Format) if rj.outputConn != "" { diff --git a/cmd/relspec/lowercase_test.go b/cmd/relspec/lowercase_test.go new file mode 100644 index 0000000..2484e78 --- /dev/null +++ b/cmd/relspec/lowercase_test.go @@ -0,0 +1,82 @@ +package main + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + "git.warky.dev/wdevs/relspecgo/pkg/models" +) + +func TestLowercaseFlagAndFileOutput(t *testing.T) { + prior := lowercaseIdentifiers + t.Cleanup(func() { lowercaseIdentifiers = prior }) + for _, cmd := range []struct { + name string + flag bool + }{{"convert", convertCmd.Flags().Lookup("lowercase") != nil}, {"batch", batchCmd.Flags().Lookup("lowercase") != nil}, {"merge", mergeCmd.Flags().Lookup("lowercase") != nil}, {"templ", templCmd.Flags().Lookup("lowercase") != nil}, {"job", jobCmd.PersistentFlags().Lookup("lowercase") != nil}} { + if !cmd.flag { + t.Fatalf("missing %s --lowercase", cmd.name) + } + } + if err := convertCmd.Flags().Set("lowercase", "true"); err != nil { + t.Fatal(err) + } + path := filepath.Join(t.TempDir(), "schema.sql") + db := models.InitDatabase("DB") + s := models.InitSchema("App") + table := models.InitTable("Users", "App") + col := models.InitColumn("Name", "Users", "App") + col.Type = "text" + col.Default = "Keep CASE" + table.Columns[col.Name] = col + s.Tables = append(s.Tables, table) + db.Schemas = append(db.Schemas, s) + if err := writeDatabase(db, "pgsql", path, "", "", false, "", "", false, ""); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), `"app"."users"`) || !strings.Contains(string(data), `"name" text DEFAULT 'Keep CASE'`) { + t.Fatalf("%s", data) + } + if table.Name != "Users" { + t.Fatal("input mutated") + } +} + +func TestJobLowercaseOption(t *testing.T) { + prior := lowercaseIdentifiers + lowercaseIdentifiers = false + t.Cleanup(func() { lowercaseIdentifiers = prior }) + dir := t.TempDir() + writeTestJSON(t, filepath.Join(dir, "input.json"), []string{"MixedCase"}) + writeFile(t, filepath.Join(dir, "relspec.yml"), `version: 1 +jobs: + lower: + command: convert + inputs: + - format: json + path: input.json + output: + format: json + path: output.json + options: + lowercase_identifiers: true +`) + set := mustLoadSet(t, filepath.Join(dir, "relspec.yml")) + if err := executeJobPlan(set, "lower", false, false, &bytes.Buffer{}); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(filepath.Join(dir, "output.json")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), "mixedcase") || strings.Contains(string(data), "MixedCase") { + t.Fatalf("%s", data) + } +} diff --git a/cmd/relspec/merge.go b/cmd/relspec/merge.go index 8eabb57..2931e4b 100644 --- a/cmd/relspec/merge.go +++ b/cmd/relspec/merge.go @@ -24,6 +24,7 @@ import ( "git.warky.dev/wdevs/relspecgo/pkg/readers/sqlite" "git.warky.dev/wdevs/relspecgo/pkg/readers/typeorm" "git.warky.dev/wdevs/relspecgo/pkg/readers/yaml" + "git.warky.dev/wdevs/relspecgo/pkg/transform" "git.warky.dev/wdevs/relspecgo/pkg/writers" wbun "git.warky.dev/wdevs/relspecgo/pkg/writers/bun" wdbml "git.warky.dev/wdevs/relspecgo/pkg/writers/dbml" @@ -407,6 +408,14 @@ func readDatabaseForMerge(dbType, filePath, connString, label string) (*models.D } func writeDatabaseForMerge(dbType, filePath, connString string, db *models.Database, label string, flattenSchema bool) error { + if lowercaseIdentifiers { + var err error + db, err = transform.LowercaseIdentifiers(db) + if err != nil { + return err + } + } + var writer writers.Writer switch strings.ToLower(dbType) { diff --git a/cmd/relspec/prisma_options.go b/cmd/relspec/prisma_options.go index 3ab43cd..12167bf 100644 --- a/cmd/relspec/prisma_options.go +++ b/cmd/relspec/prisma_options.go @@ -22,14 +22,15 @@ func newReaderOptions(filePath, connString string) *readers.ReaderOptions { func newWriterOptions(outputPath, packageName string, flattenSchema bool, nullableTypes, nullableArrays string, continueOnError bool) *writers.WriterOptions { return &writers.WriterOptions{ - OutputPath: outputPath, - PackageName: packageName, - FlattenSchema: flattenSchema, - NullableTypes: nullableTypes, - NullableArrays: nullableArrays, - TypeMappings: typeMappings, - Prisma7: prisma7, - ContinueOnError: continueOnError, - StrictDirectives: strictDirectives, + OutputPath: outputPath, + PackageName: packageName, + FlattenSchema: flattenSchema, + LowercaseIdentifiers: lowercaseIdentifiers, + NullableTypes: nullableTypes, + NullableArrays: nullableArrays, + TypeMappings: typeMappings, + Prisma7: prisma7, + ContinueOnError: continueOnError, + StrictDirectives: strictDirectives, } } diff --git a/cmd/relspec/root.go b/cmd/relspec/root.go index 5e5bf14..9b77fc5 100644 --- a/cmd/relspec/root.go +++ b/cmd/relspec/root.go @@ -12,14 +12,15 @@ import ( // version/buildDate mirror pkg/buildinfo so existing call sites keep working. // The actual values are set there via ldflags (see Makefile). var ( - version = buildinfo.Version - buildDate = buildinfo.BuildDate - prisma7 bool - noVersion bool - silent bool - strictDirectives bool - typeMapFlags []string - typeMappings map[string]string + version = buildinfo.Version + buildDate = buildinfo.BuildDate + prisma7 bool + noVersion bool + silent bool + strictDirectives bool + lowercaseIdentifiers bool + typeMapFlags []string + typeMappings map[string]string ) var rootCmd = &cobra.Command{ @@ -39,12 +40,16 @@ JSON, YAML, SQL, etc.).`, } func init() { + for _, cmd := range []*cobra.Command{convertCmd, batchCmd, mergeCmd, splitCmd, templCmd} { + cmd.Flags().BoolVar(&lowercaseIdentifiers, "lowercase", false, "Lowercase all model identifiers and structured references before output (SQL expressions and data keep their case)") + } rootCmd.AddCommand(convertCmd) rootCmd.AddCommand(batchCmd) rootCmd.AddCommand(diffCmd) rootCmd.AddCommand(inspectCmd) rootCmd.AddCommand(scriptsCmd) rootCmd.AddCommand(jobCmd) + jobCmd.PersistentFlags().BoolVar(&lowercaseIdentifiers, "lowercase", false, "Lowercase all model identifiers and structured references before schema output") rootCmd.AddCommand(assetsCmd) rootCmd.AddCommand(templCmd) rootCmd.AddCommand(editCmd) diff --git a/cmd/relspec/templ.go b/cmd/relspec/templ.go index 66922af..c68ee26 100644 --- a/cmd/relspec/templ.go +++ b/cmd/relspec/templ.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" "git.warky.dev/wdevs/relspecgo/pkg/models" + "git.warky.dev/wdevs/relspecgo/pkg/transform" "git.warky.dev/wdevs/relspecgo/pkg/writers" wtemplate "git.warky.dev/wdevs/relspecgo/pkg/writers/template" ) @@ -142,6 +143,13 @@ func runTempl(cmd *cobra.Command, args []string) error { } } + if lowercaseIdentifiers { + db, err = transform.LowercaseIdentifiers(db) + if err != nil { + return err + } + } + // Create template writer fmt.Fprintf(os.Stderr, "Loading template: %s\n", templTemplatePath) fmt.Fprintf(os.Stderr, "Execution mode: %s\n", templMode) diff --git a/docs/JOB_FILES.md b/docs/JOB_FILES.md index 8424bc3..be4fd27 100644 --- a/docs/JOB_FILES.md +++ b/docs/JOB_FILES.md @@ -191,6 +191,7 @@ jobs: # (file output = full DDL); set options.full_ddl to force full DDL overwrite: false # default false options: + lowercase_identifiers: false # lowercase names/references; SQL expressions and data keep their case flatten_schema: false schema: public package: models # for gorm/bun output diff --git a/docs/SCRIPTS_COMMAND.md b/docs/SCRIPTS_COMMAND.md index 4876228..43ddc31 100644 --- a/docs/SCRIPTS_COMMAND.md +++ b/docs/SCRIPTS_COMMAND.md @@ -96,11 +96,39 @@ INSERT INTO assets (message, payload) VALUES (:message, decode(:payload, 'base64')::bytea); ``` -- `path`: File path resolved relative to the SQL file containing the directive +- `path`: Relative file path beneath the SQL file’s directory; traversal, absolute paths, and symlinks are rejected - `var`: Named placeholder to replace, such as `:message` - `mode`: `text` embeds an escaped SQL string literal; `base64` embeds a base64 string literal -The directive comment is removed from the SQL, and every matching placeholder is replaced before the script is listed or executed. +A directive must be a standalone SQL line comment. Directive-like text inside +quoted strings, quoted identifiers, block comments, or dollar-quoted bodies is +ordinary SQL content. The directive is removed, and every matching placeholder in +executable SQL is replaced once before listing or execution. Replacements are +literal, preserving `$1`, `${2}`, `$HOME`, and other dollar text; inserted content +is never scanned for another placeholder. Duplicate embed variables or variables +used only inside strings/comments are errors. PostgreSQL casts (`::type`) remain +unchanged. Asset-call named parameters follow these same lexical boundaries; +repeated parameters share one positional argument. + +Embed and manifest asset reads share root-scoped containment and a 64 MiB per-file +limit. Only regular files are accepted. The SQL directory reader uses its selected +script tree as an explicit asset root; embed paths remain relative to each script. +Symlinks in asset paths are rejected, and containment is enforced again at open time. +Manifest entries are checked at discovery and execution, so replacing a file with an +escaping symlink after scanning cannot expose an external file. No per-directive +external-file override is supported; copy required assets into the allowed tree. + +Library callers can choose a trusted root and size limit with +`assetloader.ProcessEmbedDirectivesWithOptions(..., assetloader.EmbedOptions{ +AssetRoot: root, MaxFileBytes: limit})`; zero means the 64 MiB default, and negative +or unbounded limits are rejected. Manifest items retain `AssetRoot` and +`MaxFileBytes` for execution-time validation. + +For scripts loaded from a model rather than a SQL directory, the executor requires +trusted writer configuration `Metadata["asset_root"]` before expanding directives. +Model `source_path` metadata supplies resolution context only; it does not grant +local-file access. Missing or out-of-root source paths fail before reading assets. + ## Commands diff --git a/docs/TEMPLATE_MODE.md b/docs/TEMPLATE_MODE.md index 909f894..c1e44bf 100644 --- a/docs/TEMPLATE_MODE.md +++ b/docs/TEMPLATE_MODE.md @@ -570,3 +570,13 @@ COMMIT; - [RelSpec Documentation](../README.md) - [Model Structure Reference](../pkg/models/) - [Example Templates](../examples/templates/) + +## Output path safety + +In schema, domain, script, and table modes, `--output` always names a directory, +including a nonexistent directory without a trailing slash. An empty value uses +the current directory. Database mode uses a single output file as before. +Generated paths must stay relative to that directory. Traversal, absolute or +Windows-specific unsafe paths, symlinks, filename collisions (including case +variants), and file/directory collisions are rejected before publishing artifacts. +Nested patterns such as `{{.ParentSchema.Name}}/{{.Name}}.txt` remain supported. diff --git a/docs/audit_1.md b/docs/audit_1.md index 21df6d6..f1d9c96 100644 --- a/docs/audit_1.md +++ b/docs/audit_1.md @@ -14,6 +14,18 @@ Implementation batches (2026-10-12): - [x] **A01, shared execution results:** PostgreSQL, SQLite, SQL Server, and raw script execution now use `writers.ExecutionReport` / `ExecutionFailure`. Stop/continue modes always fail on SQL errors; counters/results survive early failure and aggregate across script schemas. Reports distinguish applied, failed, skipped, rolled-back, and uncertain operations. Generated DDL supports optional atomic execution; raw scripts report script-managed outcomes without promising statement-level commit/rollback. Requested report failures are errors in every executor. Live PostgreSQL, SQL Server, SQLite, and dependent-job regressions pass. - [x] **A02, conversion expressions and preflight:** column metadata `pgsql_using` configures an explicit SQL conversion expression; `pgsql_preflight` adds a row predicate. Full-DDL and diff plans evaluate every existing value before schema mutations, and live preflight failure stops even with continuation enabled. Tests cover later invalid rows, preservation of a constraint scheduled for removal, custom conversions, predicate rejection, invalid configuration, and block-delimiter collisions. Standalone schema generation also includes preflights. +- [x] **A03, contained template output:** multi-file output has an explicit directory root, portable relative filename validation, and preflight collision/symlink checks. Rendering/validation complete before writing; exclusive per-file staging and root-scoped rename prevent generated filenames from escaping the root. Multi-file atomic publication remains A16 work. +- [x] **A04, contained asset reads:** embed directives and manifest execution use shared root-scoped reads, symlink checks, regular-file validation, and a bounded 64 MiB default. Manifest paths are rechecked at execution. Imported source-path metadata cannot authorize file reads without a trusted configured asset root. Library callers can explicitly configure a root/size limit; directives cannot opt out of containment. +- [x] **A05, PostgreSQL quoting:** schema/full-DDL/diff paths and embedded templates always quote model identifiers, quote literal/catalog/regclass values, sanitize names in generated line comments, and choose collision-safe dollar delimiters for procedural and audit function bodies. NUL is rejected before output; migration comparisons preserve identifier case. Raw SQL expressions remain an explicit executable-input boundary. +- [x] **Requested lowercase option:** `--lowercase` on convert/batch/merge/split/templ/job run, job `options.lowercase_identifiers`, and PostgreSQL `WriterOptions.LowercaseIdentifiers` normalize names and structured references on a copy, rejecting collisions. SQL expressions, defaults, comments, enum values, and data keep their case; callers must adjust references inside raw SQL themselves. +- [x] **A08, SQL-aware substitution/execution:** embeds use one-pass literal substitution, preserving dollar characters and preventing replacement rescanning. Named parameters and embed directives respect SQL strings, quoted identifiers, nested comments, casts, and dollar bodies; duplicate/unused embed variables fail. MySQL executes structured generated statements without splitting multiline literals or stripping their contents. + +A05–A08 verification: unit regressions and live PostgreSQL 18 checks cover unusual names, comments/defaults, rerunnable file/statement output, diff migrations, sequences, primary-key replacement, audit functions/triggers, and both standard-string modes. MySQL statement preservation is unit-tested; a live MySQL instance was not used for A08. `go test -mod=vendor -p 1 ./...` passed with `RELSPEC_TEST_PG_CONN` set to the disposable instance; `go vet -mod=vendor ./...`, executable CLI help checks, and `git diff --check` passed. SQL Server tests were not configured for this batch. The disposable instance was removed after verification. + +Additional observation: a parallel live full-suite run hit a PostgreSQL reader catalog race (`could not open relation with OID`) while another package's test dropped schema objects. The writer safely fell back to full DDL and its preflight still stopped all mutations, but the existing test assumed no successful read-only preflights before failure. Running live packages serially passed. Reader resilience to concurrent catalog changes remains outside A05/A08; A10 specifically concerns cancellation/timeouts. + +A03–A04 verification: `go test -mod=vendor ./...`, targeted regression tests, `go vet -mod=vendor ./...`, and `git diff --check` passed. Database-dependent tests were not configured for this filesystem-only batch. Windows test binaries were cross-compiled for the template and asset packages; Windows runtime filesystem tests remain unperformed. + Verification: `go test -mod=vendor ./...` passed with `RELSPEC_TEST_PG_CONN` and `RELSPEC_TEST_MSSQL_CONN` pointing to disposable PostgreSQL 18 and SQL Server 2022 Developer instances; SQLite regressions use temporary database files. A minimal public table was seeded for the existing PostgreSQL `ReadTable` test, which requires a nonempty default schema. `go vet -mod=vendor ./...` and `git diff --check` passed. Executable CLI checks verified `scripts execute --ignore-errors`, `--continue-on-error`, and `--coe` all attempt a later script after failure and exit 1. Raw script tests intentionally make no statement-level rollback claim. A09 remains open for concurrency, history, and migration recovery; sequence/external side effects are outside transaction rollback guarantees. Original audit evidence is retained for historical context. ## Assessment @@ -92,6 +104,8 @@ The broad exception handler also treats unrelated failures as permission to atte ### A03 — Generated template filenames escape the output directory +**Repair status: addressed.** Multi-file roots, generated paths, collisions, symlinks, and root-scoped writes are covered by regression tests. A nonexistent output path now consistently denotes the directory itself rather than its parent. + **P1 / High · Security · Reproduced** Evidence: `pkg/writers/template/writer.go:249–308`. @@ -106,6 +120,8 @@ An imported model can therefore cause writes outside the intended destination, i ### A04 — SQL embed directives can read outside the script directory +**Repair status: addressed.** Shared bounded root-scoped reads protect directives and manifest assets, including revalidation after discovery. Model source-path metadata alone no longer grants local file access. + **P1 / High · Security · Reproduced** Evidence: `pkg/assetloader/embed.go:73–74`; compare the containment checks in `pkg/assetloader/loader.go` and `pkg/jobs/jobs.go`. diff --git a/pkg/assetloader/embed.go b/pkg/assetloader/embed.go index 430fbd0..0bfe274 100644 --- a/pkg/assetloader/embed.go +++ b/pkg/assetloader/embed.go @@ -9,10 +9,22 @@ import ( "strconv" "strings" "unicode/utf8" + + "git.warky.dev/wdevs/relspecgo/pkg/safefile" + "git.warky.dev/wdevs/relspecgo/pkg/sqllex" ) const ScriptSourcePathMetadataKey = "source_path" +const DefaultMaxAssetBytes int64 = 64 << 20 // 64 MiB per manifest/embedded asset + +// EmbedOptions are trusted caller configuration, not script/model metadata. +// An empty AssetRoot defaults to the SQL file's directory; external reads are rejected. +type EmbedOptions struct { + AssetRoot string + MaxFileBytes int64 +} + var ( embedDirectivePattern = regexp.MustCompile(`(?m)^\s*--\s*@embed:\s*(.+?)\s*$`) embedAttrPattern = regexp.MustCompile(`([a-zA-Z_][a-zA-Z0-9_]*)=("[^"]*"|'[^']*'|\S+)`) @@ -27,7 +39,28 @@ var ( // string literal. Base64 mode embeds a quoted base64 literal suitable for // decode(:var, 'base64'). func ProcessEmbedDirectives(sqlPath, sql string) (string, error) { - directives := embedDirectivePattern.FindAllStringSubmatch(sql, -1) + return ProcessEmbedDirectivesWithOptions(sqlPath, sql, EmbedOptions{}) +} + +func embedDirectives(sql string) (string, []string) { + var directives []string + // Only standalone line comments are directives, and only in SQL code. + result := sqllex.Walk(sql, nil, func(comment string) string { + if match := embedDirectivePattern.FindStringSubmatch(comment); match != nil { + directives = append(directives, match[1]) + return "" + } + return comment + }) + return result, directives +} +func HasEmbedDirectives(sql string) bool { + _, directives := embedDirectives(sql) + return len(directives) > 0 +} + +func ProcessEmbedDirectivesWithOptions(sqlPath, sql string, options EmbedOptions) (string, error) { + result, directives := embedDirectives(sql) if len(directives) == 0 { return sql, nil } @@ -36,22 +69,63 @@ func ProcessEmbedDirectives(sqlPath, sql string) (string, error) { return "", fmt.Errorf("sql path is required for embed directives") } - result := embedDirectivePattern.ReplaceAllString(sql, "") + assetRoot := options.AssetRoot + if assetRoot == "" { + assetRoot = filepath.Dir(sqlPath) + } + absRoot, err := filepath.Abs(assetRoot) + if err != nil { + return "", err + } + absSQL, err := filepath.Abs(sqlPath) + if err != nil { + return "", err + } + scriptRel, err := filepath.Rel(absRoot, absSQL) + if err != nil { + return "", err + } + if _, err := safefile.Relative(filepath.ToSlash(scriptRel)); err != nil { + return "", fmt.Errorf("SQL source path is outside asset root: %w", err) + } + root, err := openAssetRoot(absRoot) + if err != nil { + return "", err + } + defer root.Close() + limit := options.MaxFileBytes + if limit == 0 { + limit = DefaultMaxAssetBytes + } + literals := map[string]string{} + used := map[string]bool{} for i, directive := range directives { - literal, placeholder, err := embedDirectiveLiteral(sqlPath, directive[1], i+1) + literal, placeholder, err := embedDirectiveLiteral(root, filepath.Dir(scriptRel), limit, sqlPath, directive, i+1) if err != nil { return "", err } - if !embedPlaceholderPattern(placeholder).MatchString(result) { - return "", fmt.Errorf("%s embed directive %d: placeholder %s not found", sqlPath, i+1, placeholder) + if _, exists := literals[placeholder]; exists { + return "", fmt.Errorf("duplicate embed placeholder %s", placeholder) + } + literals[placeholder] = literal + } + result = sqllex.Walk(result, func(name string) string { + if literal, ok := literals[name]; ok { + used[name] = true + return literal + } + return name + }, nil) + for name := range literals { + if !used[name] { + return "", fmt.Errorf("%s: embed placeholder %s not found in executable SQL", sqlPath, name) } - result = replaceEmbedPlaceholder(result, placeholder, literal) } return result, nil } -func embedDirectiveLiteral(sqlPath, raw string, directiveNumber int) (literal, placeholder string, err error) { +func embedDirectiveLiteral(root *os.Root, scriptDir string, maxBytes int64, sqlPath, raw string, directiveNumber int) (literal, placeholder string, err error) { attrs, err := parseEmbedAttrs(raw) if err != nil { return "", "", fmt.Errorf("%s embed directive %d: %w", sqlPath, directiveNumber, err) @@ -70,8 +144,12 @@ func embedDirectiveLiteral(sqlPath, raw string, directiveNumber int) (literal, p return "", "", fmt.Errorf("%s embed directive %d: mode must be text or base64", sqlPath, directiveNumber) } - resolved := filepath.Join(filepath.Dir(sqlPath), filepath.Clean(pathValue)) - data, err := os.ReadFile(resolved) + relative, err := safefile.Relative(pathValue) + if err != nil { + return "", "", fmt.Errorf("%s embed directive %d: %w", sqlPath, directiveNumber, err) + } + resolved := filepath.Join(scriptDir, relative) + data, err := safefile.Read(root, resolved, maxBytes) if err != nil { return "", "", fmt.Errorf("%s embed directive %d: reading %s: %w", sqlPath, directiveNumber, resolved, err) } @@ -153,13 +231,10 @@ func sqlStringLiteral(value string) (string, error) { if strings.ContainsRune(value, '\x00') { return "", fmt.Errorf("embedded text contains NUL byte") } - return "'" + strings.ReplaceAll(value, "'", "''") + "'", nil -} - -func replaceEmbedPlaceholder(sql, placeholder, literal string) string { - return embedPlaceholderPattern(placeholder).ReplaceAllString(sql, "${1}"+literal+"${2}") -} - -func embedPlaceholderPattern(placeholder string) *regexp.Regexp { - return regexp.MustCompile(`(^|[^a-zA-Z0-9_:])` + regexp.QuoteMeta(placeholder) + `([^a-zA-Z0-9_]|$)`) + prefix := "" + if strings.Contains(value, "\\") { + prefix = "E" + value = strings.ReplaceAll(value, "\\", "\\\\") + } + return prefix + "'" + strings.ReplaceAll(value, "'", "''") + "'", nil } diff --git a/pkg/assetloader/embed_test.go b/pkg/assetloader/embed_test.go index 062c2c3..3ebb9ce 100644 --- a/pkg/assetloader/embed_test.go +++ b/pkg/assetloader/embed_test.go @@ -141,3 +141,114 @@ SELECT :body, :body_extra; t.Fatalf("placeholder boundary was not respected:\n%s", got) } } + +func TestEmbedRejectsTraversalAbsolutePathsAndSymlinks(t *testing.T) { + root := t.TempDir() + scriptDir := filepath.Join(root, "scripts") + if err := os.Mkdir(scriptDir, 0o755); err != nil { + t.Fatal(err) + } + secret := filepath.Join(root, "secret.txt") + if err := os.WriteFile(secret, []byte("never print this secret"), 0o600); err != nil { + t.Fatal(err) + } + for _, path := range []string{"../secret.txt", secret, `..\secret.txt`, `C:\secret.txt`, `\\server\secret.txt`} { + sql := "-- @embed: path='" + path + "' var=:body mode=text\nSELECT :body;" + got, err := ProcessEmbedDirectives(filepath.Join(scriptDir, "seed.sql"), sql) + if err == nil || got != "" { + t.Fatalf("unsafe embed path accepted %q: %q %v", path, got, err) + } + if strings.Contains(err.Error(), "never print this secret") { + t.Fatal("diagnostic leaked asset bytes") + } + } + if err := os.Symlink(root, filepath.Join(scriptDir, "link")); err != nil { + t.Skip(err) + } + if _, err := ProcessEmbedDirectives(filepath.Join(scriptDir, "seed.sql"), "-- @embed: path=link/secret.txt var=:body mode=text\nSELECT :body;"); err == nil { + t.Fatal("symlink parent must be rejected") + } + if err := os.Symlink(secret, filepath.Join(scriptDir, "leaf.txt")); err != nil { + t.Fatal(err) + } + if _, err := ProcessEmbedDirectives(filepath.Join(scriptDir, "seed.sql"), "-- @embed: path=leaf.txt var=:body mode=text\nSELECT :body;"); err == nil { + t.Fatal("symlink asset must be rejected") + } +} + +func TestEmbedExplicitRootAndSizeLimit(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, "scripts") + if err := os.Mkdir(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "body.txt"), []byte("12345"), 0o600); err != nil { + t.Fatal(err) + } + sql := "-- @embed: path=body.txt var=:body mode=text\nSELECT :body;" + path := filepath.Join(dir, "seed.sql") + if _, err := ProcessEmbedDirectivesWithOptions(path, sql, EmbedOptions{AssetRoot: root, MaxFileBytes: 4}); err == nil || !strings.Contains(err.Error(), "byte limit") { + t.Fatalf("expected size limit error: %v", err) + } + if got, err := ProcessEmbedDirectivesWithOptions(path, sql, EmbedOptions{AssetRoot: root, MaxFileBytes: 5}); err != nil || !strings.Contains(got, "SELECT '12345';") { + t.Fatalf("valid bounded embed: %q %v", got, err) + } + if _, err := ProcessEmbedDirectivesWithOptions(path, sql, EmbedOptions{AssetRoot: t.TempDir()}); err == nil { + t.Fatal("source path outside explicit root must fail") + } + if _, err := ProcessEmbedDirectivesWithOptions(path, sql, EmbedOptions{MaxFileBytes: -1}); err == nil { + t.Fatal("negative limits must fail") + } +} + +func TestManifestReadRechecksContainmentAndSize(t *testing.T) { + root, external := t.TempDir(), t.TempDir() + file := filepath.Join(root, "body.txt") + if err := os.WriteFile(file, []byte("valid"), 0o600); err != nil { + t.Fatal(err) + } + item := Item{AssetRoot: root, MaxFileBytes: 4, Entry: ManifestEntry{File: file}} + if _, err := readItemAsset(item); err == nil { + t.Fatal("manifest size bound must apply at execution") + } + item.MaxFileBytes = 5 + if got, err := readItemAsset(item); err != nil || string(got) != "valid" { + t.Fatalf("valid manifest asset: %q %v", got, err) + } + secret := filepath.Join(external, "secret.txt") + if err := os.WriteFile(secret, []byte("secret"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Remove(file); err != nil { + t.Fatal(err) + } + if err := os.Symlink(secret, file); err != nil { + t.Skip(err) + } + if _, err := readItemAsset(item); err == nil { + t.Fatal("replacement symlink after scanning must fail") + } + item.Entry.File = secret + if _, err := readItemAsset(item); err == nil { + t.Fatal("forged asset path must fail") + } +} + +func TestEmbedDefaultLimitRejectsOversizedFile(t *testing.T) { + dir := t.TempDir() + file, err := os.Create(filepath.Join(dir, "large.bin")) + if err != nil { + t.Fatal(err) + } + if err := file.Truncate(DefaultMaxAssetBytes + 1); err != nil { + file.Close() + t.Fatal(err) + } + if err := file.Close(); err != nil { + t.Fatal(err) + } + _, err = ProcessEmbedDirectives(filepath.Join(dir, "seed.sql"), "-- @embed: path=large.bin var=:payload mode=base64\nSELECT :payload;") + if err == nil || !strings.Contains(err.Error(), "byte limit") { + t.Fatalf("expected default size bound: %v", err) + } +} diff --git a/pkg/assetloader/executor.go b/pkg/assetloader/executor.go index f18579b..6406c41 100644 --- a/pkg/assetloader/executor.go +++ b/pkg/assetloader/executor.go @@ -3,20 +3,12 @@ package assetloader import ( "context" "fmt" - "os" + "git.warky.dev/wdevs/relspecgo/pkg/sqllex" "path/filepath" - "regexp" - "strings" "github.com/jackc/pgx/v5" ) -// namedPlaceholder matches :identifier patterns (but not ::cast syntax). -var namedPlaceholder = regexp.MustCompile(`:([a-zA-Z_][a-zA-Z0-9_]*)`) - -// pgCastMarker temporarily replaces :: to protect PostgreSQL cast syntax. -const pgCastMarker = "\x00PGCAST\x00" - // BuildQuery converts a SQL call that uses :name named placeholders into a // pgx-compatible positional-parameter query ($1, $2, …) and returns the // corresponding argument slice. @@ -30,13 +22,11 @@ const pgCastMarker = "\x00PGCAST\x00" // 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) { - // Protect :: casts before running the placeholder regex. - protected := strings.ReplaceAll(call, "::", pgCastMarker) paramIndex := map[string]int{} // name → 1-based position var firstErr error - query = namedPlaceholder.ReplaceAllStringFunc(protected, func(match string) string { + query = sqllex.Walk(call, func(match string) string { if firstErr != nil { return match } @@ -70,15 +60,12 @@ func BuildQuery(call string, fileBytes []byte, filename string, staticParams map paramIndex[name] = idx args = append(args, val) return fmt.Sprintf("$%d", idx) - }) + }, nil) if firstErr != nil { return "", nil, firstErr } - // Restore :: casts. - query = strings.ReplaceAll(query, pgCastMarker, "::") - return query, args, nil } @@ -86,7 +73,7 @@ func BuildQuery(call string, fileBytes []byte, filename string, staticParams map // absolute path set by ScanDir) and executes the configured SQL call via conn. // The file's raw bytes are bound as a []byte parameter — no encoding or escaping. func ExecuteItem(ctx context.Context, conn *pgx.Conn, item Item) error { - data, err := os.ReadFile(item.Entry.File) + data, err := readItemAsset(item) if err != nil { return fmt.Errorf("reading asset file %s: %w", item.Entry.File, err) } diff --git a/pkg/assetloader/files.go b/pkg/assetloader/files.go new file mode 100644 index 0000000..82afb5d --- /dev/null +++ b/pkg/assetloader/files.go @@ -0,0 +1,36 @@ +package assetloader + +import ( + "fmt" + "os" + "path/filepath" + + "git.warky.dev/wdevs/relspecgo/pkg/safefile" +) + +func openAssetRoot(path string) (*os.Root, error) { + if err := safefile.CheckRoot(path); err != nil { + return nil, err + } + return os.OpenRoot(path) +} + +func readItemAsset(item Item) ([]byte, error) { + if item.AssetRoot == "" { + return nil, fmt.Errorf("asset root is required for manifest execution") + } + root, err := openAssetRoot(item.AssetRoot) + if err != nil { + return nil, err + } + defer root.Close() + relative, err := filepath.Rel(item.AssetRoot, item.Entry.File) + if err != nil { + return nil, err + } + limit := item.MaxFileBytes + if limit == 0 { + limit = DefaultMaxAssetBytes + } + return safefile.Read(root, relative, limit) +} diff --git a/pkg/assetloader/loader.go b/pkg/assetloader/loader.go index 7fee0d0..87b907d 100644 --- a/pkg/assetloader/loader.go +++ b/pkg/assetloader/loader.go @@ -18,9 +18,10 @@ import ( "regexp" "sort" "strconv" - "strings" "gopkg.in/yaml.v3" + + "git.warky.dev/wdevs/relspecgo/pkg/safefile" ) // ManifestEntry describes a single file to load from an assets.yaml manifest. @@ -37,6 +38,9 @@ type ManifestEntry struct { // Item combines a manifest entry with its ordering metadata and the resolved // directory where the manifest and asset file reside. type Item struct { + // AssetRoot is the trusted scan root, retained for validation at execution time. + AssetRoot string + MaxFileBytes int64 // Priority and Sequence come from the parent directory's naming pattern. Priority int Sequence uint @@ -57,7 +61,12 @@ var dirPattern = regexp.MustCompile(`^(\d+)[_-](\d+)[_-](.+)$`) // absent or contains invalid YAML. func LoadManifest(dir string) ([]ManifestEntry, error) { manifestPath := filepath.Join(dir, "assets.yaml") - data, err := os.ReadFile(manifestPath) + root, err := openAssetRoot(dir) + if err != nil { + return nil, err + } + defer root.Close() + data, err := safefile.Read(root, "assets.yaml", DefaultMaxAssetBytes) if err != nil { return nil, fmt.Errorf("reading %s: %w", manifestPath, err) } @@ -92,7 +101,7 @@ func ScanDir(baseDir string) ([]Item, error) { if d.IsDir() { return nil } - if d.Name() != "assets.yaml" { + if d.Type()&os.ModeSymlink != 0 || d.Name() != "assets.yaml" { return nil } @@ -119,10 +128,12 @@ func ScanDir(baseDir string) ([]Item, error) { } items = append(items, Item{ - Priority: priority, - Sequence: sequence, - DirName: dirName, - Dir: manifestDir, + AssetRoot: absBase, + MaxFileBytes: DefaultMaxAssetBytes, + Priority: priority, + Sequence: sequence, + DirName: dirName, + Dir: manifestDir, Entry: ManifestEntry{ File: resolved, // absolute path, safe to read Call: entry.Call, @@ -170,31 +181,28 @@ func parseDirName(dir string) (priority int, sequence uint, name string) { // Returns the absolute path, a skip flag (true when the entry should be silently // dropped), and any hard error. func resolveAssetPath(absBase, manifestDir, file string) (absPath string, skip bool, err error) { - // Clean and join before any symlink resolution so we can detect traversal. - joined := filepath.Join(manifestDir, filepath.Clean(file)) - - // Ensure the cleaned path is still inside absBase. - rel, err := filepath.Rel(absBase, joined) - if err != nil || strings.HasPrefix(rel, "..") { - // Path escapes the base directory; skip silently. + if _, err := safefile.Relative(file); err != nil { return "", true, nil } - - // Walk each component to detect symlinks. - parts := strings.Split(rel, string(filepath.Separator)) - current := absBase - for _, part := range parts { - current = filepath.Join(current, part) - info, statErr := os.Lstat(current) - if statErr != nil { - // File doesn't exist; skip. - return "", true, nil - } - if info.Mode()&os.ModeSymlink != 0 { - // Symlink in path; skip silently. - return "", true, nil - } + joined := filepath.Join(manifestDir, file) + rel, err := filepath.Rel(absBase, joined) + if err != nil { + return "", true, nil + } + root, err := openAssetRoot(absBase) + if err != nil { + return "", false, err + } + defer root.Close() + if err := safefile.Check(root, rel); err != nil { + return "", true, nil + } + info, err := root.Stat(rel) + if err != nil || !info.Mode().IsRegular() { + return "", true, nil + } + if info.Size() > DefaultMaxAssetBytes { + return "", false, fmt.Errorf("asset %q exceeds %d byte limit", rel, DefaultMaxAssetBytes) } - return joined, false, nil } diff --git a/pkg/assetloader/syntax_test.go b/pkg/assetloader/syntax_test.go new file mode 100644 index 0000000..f9e4a09 --- /dev/null +++ b/pkg/assetloader/syntax_test.go @@ -0,0 +1,60 @@ +package assetloader + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestBuildQuerySQLContexts(t *testing.T) { + call := `SELECT ':unknown', ":unknown", $$:unknown$$, E'it\':unknown', :bytes::bytea, :filename, :bytes /* :unknown /* :unknown */ */ -- :unknown +` + got, args, err := BuildQuery(call, []byte("$1"), "File.txt", nil) + if err != nil { + t.Fatal(err) + } + if len(args) != 2 || !strings.Contains(got, "$1::bytea, $2, $1") || !strings.Contains(got, "$$:unknown$$") { + t.Fatalf("%s %#v", got, args) + } +} + +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 { + t.Fatal(err) + } + } + sql := `-- @embed: path=a.txt var=:a mode=text +-- @embed: path=b.txt var=:b mode=text +SELECT :a,:a,:b, ':a', ":a", $$:a$$, 1::integer; -- :a +/* :a */` + got, err := ProcessEmbedDirectives(filepath.Join(dir, "script.sql"), sql) + if err != nil { + t.Fatal(err) + } + literal, err := sqlStringLiteral(data) + if err != nil { + t.Fatal(err) + } + if strings.Count(got, literal) != 2 || !strings.Contains(got, "'Second'") || !strings.Contains(got, "':a'") || !strings.Contains(got, "$$:a$$") { + t.Fatalf("%s", got) + } + + for _, sql := range []string{"SELECT $body$\n-- @embed: path=missing var=:x mode=text\n$body$", "SELECT '\n-- @embed: path=missing var=:x mode=text\n'", "/*\n-- @embed: path=missing var=:x mode=text\n*/", "SELECT 1; -- @embed: path=missing var=:x mode=text"} { + if HasEmbedDirectives(sql) { + t.Fatalf("false directive: %s", sql) + } + got, err := ProcessEmbedDirectives("", sql) + if err != nil || got != sql { + t.Fatalf("%q: %v", got, err) + } + } + for _, sql := range []string{"-- @embed: path=a.txt var=:a mode=text\nSELECT ':a'", "-- @embed: path=a.txt var=:a mode=text\n-- @embed: path=b.txt var=:a mode=text\nSELECT :a"} { + if _, err := ProcessEmbedDirectives(filepath.Join(dir, "script.sql"), sql); err == nil { + t.Fatal("expected invalid directive use") + } + } +} diff --git a/pkg/jobs/jobs.go b/pkg/jobs/jobs.go index 81400d0..bcfeb50 100644 --- a/pkg/jobs/jobs.go +++ b/pkg/jobs/jobs.go @@ -277,9 +277,10 @@ func parseHumanSize(s string) (int64, error) { // Options carries the subset of command flags a job file may set. type Options struct { - FlattenSchema bool `yaml:"flatten_schema"` - Schema string `yaml:"schema"` - Package string `yaml:"package"` + LowercaseIdentifiers bool `yaml:"lowercase_identifiers"` + FlattenSchema bool `yaml:"flatten_schema"` + Schema string `yaml:"schema"` + Package string `yaml:"package"` // Types and ArrayNullable mirror the existing convert/split writer flags. // They are passed through only to writers that already support them. Types string `yaml:"types"` diff --git a/pkg/readers/sqldir/reader.go b/pkg/readers/sqldir/reader.go index 8036eea..83c7231 100644 --- a/pkg/readers/sqldir/reader.go +++ b/pkg/readers/sqldir/reader.go @@ -152,7 +152,7 @@ func (r *Reader) readScripts() ([]*models.Script, error) { if err != nil { return fmt.Errorf("failed to read file %s: %w", path, err) } - sql, err := assetloader.ProcessEmbedDirectives(path, string(content)) + sql, err := assetloader.ProcessEmbedDirectivesWithOptions(path, string(content), assetloader.EmbedOptions{AssetRoot: r.options.FilePath}) if err != nil { return err } diff --git a/pkg/safefile/root.go b/pkg/safefile/root.go new file mode 100644 index 0000000..124365e --- /dev/null +++ b/pkg/safefile/root.go @@ -0,0 +1,136 @@ +// Package safefile provides portable path validation and root-scoped file access. +package safefile + +import ( + "fmt" + "io" + "os" + "path/filepath" + "strings" + "unicode" +) + +// Relative validates artifact paths consistently on Unix and Windows. It rejects +// traversal rather than normalizing it, so aliases cannot hide filename collisions. +func Relative(name string) (string, error) { + if name == "" || !filepath.IsLocal(name) || strings.ContainsAny(name, `\:<>"|?*`) { + return "", fmt.Errorf("unsafe relative path %q", name) + } + for _, r := range name { + if unicode.IsControl(r) { + return "", fmt.Errorf("control character in path %q", name) + } + } + for _, part := range strings.Split(filepath.ToSlash(name), "/") { + if part == "" || part == "." || part == ".." || strings.HasSuffix(part, ".") || strings.HasSuffix(part, " ") { + return "", fmt.Errorf("unsafe path component in %q", name) + } + base := strings.ToUpper(strings.SplitN(part, ".", 2)[0]) + device := base == "CON" || base == "PRN" || base == "AUX" || base == "NUL" || base == "CONIN$" || base == "CONOUT$" + for _, prefix := range []string{"COM", "LPT"} { + if strings.HasPrefix(base, prefix) { + suffix := []rune(strings.TrimPrefix(base, prefix)) + device = device || (len(suffix) == 1 && strings.ContainsRune("123456789¹²³", suffix[0])) + } + } + if device { + return "", fmt.Errorf("reserved device path %q", name) + } + } + return filepath.FromSlash(name), nil +} + +// Check rejects existing symlinks and special files; os.Root separately enforces +// containment at open/rename time, including if a symlink changes after this check. +func Check(root *os.Root, name string) error { + if _, err := Relative(filepath.ToSlash(name)); err != nil { + return err + } + parts := strings.Split(name, string(filepath.Separator)) + current := "" + for i, part := range parts { + current = filepath.Join(current, part) + info, err := root.Lstat(current) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + if info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("symlink is not allowed in path %q", name) + } + if i < len(parts)-1 && !info.IsDir() { + return fmt.Errorf("parent is not a directory in path %q", name) + } + if i == len(parts)-1 && !info.Mode().IsRegular() { + return fmt.Errorf("path is not a regular file: %q", name) + } + } + return nil +} + +// Read opens through a pinned root and bounds both the declared and actual size. +func Read(root *os.Root, name string, maxBytes int64) ([]byte, error) { + if maxBytes <= 0 || maxBytes == (1<<63)-1 { + return nil, fmt.Errorf("file size limit must be positive and bounded") + } + if err := Check(root, name); err != nil { + return nil, err + } + file, err := root.Open(name) + if err != nil { + return nil, err + } + defer file.Close() + info, err := file.Stat() + if err != nil { + return nil, err + } + if !info.Mode().IsRegular() { + return nil, fmt.Errorf("asset is not a regular file: %q", name) + } + if info.Size() > maxBytes { + return nil, fmt.Errorf("asset %q exceeds %d byte limit", name, maxBytes) + } + data, err := io.ReadAll(io.LimitReader(file, maxBytes+1)) + if err != nil { + return nil, err + } + if int64(len(data)) > maxBytes { + return nil, fmt.Errorf("asset %q exceeds %d byte limit", name, maxBytes) + } + return data, nil +} + +// CheckRoot rejects symlinks in the configured root or its existing ancestors. +// The configured root is a trusted caller-selected boundary; OpenRoot pins it for +// subsequent operations. Missing components are allowed for output root creation. +func CheckRoot(path string) error { + absolute, err := filepath.Abs(path) + if err != nil { + return err + } + volume := filepath.VolumeName(absolute) + current := volume + string(filepath.Separator) + for _, part := range strings.Split(strings.TrimPrefix(absolute, current), string(filepath.Separator)) { + if part == "" { + continue + } + current = filepath.Join(current, part) + info, err := os.Lstat(current) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + if info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("symlink is not allowed in root path %q", path) + } + if !info.IsDir() { + return fmt.Errorf("root parent is not a directory: %q", current) + } + } + return nil +} diff --git a/pkg/sqllex/scan.go b/pkg/sqllex/scan.go new file mode 100644 index 0000000..24ff200 --- /dev/null +++ b/pkg/sqllex/scan.go @@ -0,0 +1,101 @@ +// Package sqllex recognizes PostgreSQL lexical boundaries without parsing SQL. +package sqllex + +import "strings" + +func start(c byte) bool { return c >= 128 || c == '_' || c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' } +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 { + var out strings.Builder + for i := 0; i < len(sql); { + begin := i + switch { + case strings.HasPrefix(sql[i:], "--"): + i += 2 + for i < len(sql) && sql[i] != '\n' && sql[i] != '\r' { + i++ + } + text := sql[begin:i] + if lineComment != nil && strings.TrimSpace(sql[strings.LastIndexAny(sql[:begin], "\r\n")+1:begin]) == "" { + text = lineComment(text) + } + out.WriteString(text) + continue + case strings.HasPrefix(sql[i:], "/*"): + i += 2 + depth := 1 + for i < len(sql) && depth > 0 { + if strings.HasPrefix(sql[i:], "/*") { + depth++ + i += 2 + } else if strings.HasPrefix(sql[i:], "*/") { + depth-- + i += 2 + } else { + i++ + } + } + case sql[i] == '\'' || sql[i] == '"': + q := sql[i] + escapes := q == '\'' && i > 0 && (sql[i-1] == 'E' || sql[i-1] == 'e') && (i == 1 || !ident(sql[i-2])) + i++ + for i < len(sql) { + if escapes && sql[i] == '\\' { + i++ + if i < len(sql) { + i++ + } + continue + } + if sql[i] == q { + i++ + if i < len(sql) && sql[i] == q { + i++ + continue + } + break + } + i++ + } + case sql[i] == '$' && (i == 0 || !ident(sql[i-1]) && sql[i-1] != '$'): + j := i + 1 + if j < len(sql) && start(sql[j]) { + for j < len(sql) && ident(sql[j]) { + j++ + } + } + if j < len(sql) && sql[j] == '$' { + tag := sql[i : j+1] + end := strings.Index(sql[j+1:], tag) + if end < 0 { + i = len(sql) + } else { + i = j + 1 + end + len(tag) + } + } else { + i++ + } + case strings.HasPrefix(sql[i:], "::"): + i += 2 + case sql[i] == ':' && i+1 < len(sql) && start(sql[i+1]) && (i == 0 || !ident(sql[i-1]) && sql[i-1] != ':'): + i += 2 + for i < len(sql) && ident(sql[i]) { + i++ + } + text := sql[begin:i] + if parameter != nil { + text = parameter(text) + } + out.WriteString(text) + continue + default: + i++ + } + out.WriteString(sql[begin:i]) + } + return out.String() +} diff --git a/pkg/sqllex/scan_test.go b/pkg/sqllex/scan_test.go new file mode 100644 index 0000000..5d3f3ae --- /dev/null +++ b/pkg/sqllex/scan_test.go @@ -0,0 +1,28 @@ +package sqllex + +import ( + "strings" + "testing" +) + +func TestWalkSQLContexts(t *testing.T) { + sql := `SELECT :x,:x, :x::text, ':hidden', 'it'':hidden', E'it\':hidden', ":hidden", $$:hidden$$, $名$:hidden$名$; +-- :hidden +/* :hidden /* :hidden */ :hidden */ SELECT :y, value::integer, x := 1;` + var names []string + got := Walk(sql, func(name string) string { names = append(names, name); return "$1 ${2} :inserted" }, nil) + if strings.Join(names, ",") != ":x,:x,:x,:y" { + t.Fatalf("parameters: %v", names) + } + if strings.Count(got, "$1 ${2} :inserted") != 4 || !strings.Contains(got, "value::integer") { + t.Fatalf("result: %s", got) + } +} + +func TestUnterminatedSQLContextsAreOpaque(t *testing.T) { + for _, sql := range []string{"SELECT ':x", `SELECT ":x`, "DO $tag$:x", "/* :x", "-- :x"} { + if got := Walk(sql, func(string) string { t.Fatal("unexpected parameter"); return "" }, nil); got != sql { + t.Fatalf("changed %q", sql) + } + } +} diff --git a/pkg/transform/identifiers.go b/pkg/transform/identifiers.go new file mode 100644 index 0000000..c0746f6 --- /dev/null +++ b/pkg/transform/identifiers.go @@ -0,0 +1,157 @@ +package transform + +import ( + "fmt" + "reflect" + "strings" + + "git.warky.dev/wdevs/relspecgo/pkg/models" +) + +// LowercaseIdentifiers copies the model and lowercases names and structured +// references. SQL expressions, defaults, enum values, comments and metadata are +// preserved. Callers must adjust identifiers inside their own SQL expressions. +// Collisions are rejected before any output is written. +func LowercaseIdentifiers(db *models.Database) (*models.Database, error) { + if db == nil { + return nil, fmt.Errorf("database is required") + } + seen := map[any]reflect.Value{} + nameFields := map[string]bool{} + for _, name := range strings.Fields("Name Schema Table Tablespace Collation TableName SchemaName FromTable FromSchema ToTable ToSchema ThroughTable ThroughSchema ForeignKey ReferencedTable ReferencedSchema OwnedByTable OwnedByColumn") { + nameFields[name] = true + } + listFields := map[string]bool{"Columns": true, "Include": true, "ReferencedColumns": true, "FromColumns": true, "ToColumns": true, "RunAfter": true} + var copyValue func(reflect.Value) (reflect.Value, error) + copyValue = func(v reflect.Value) (reflect.Value, error) { + if !v.IsValid() { + return v, nil + } + switch v.Kind() { + case reflect.Pointer: + if v.IsNil() || v.Type().Elem().PkgPath() != reflect.TypeOf(models.Database{}).PkgPath() { + return v, nil + } + if prior, ok := seen[v.Interface()]; ok { + return prior, nil + } + out := reflect.New(v.Type().Elem()) + seen[v.Interface()] = out + copied, err := copyValue(v.Elem()) + if err != nil { + return reflect.Value{}, err + } + out.Elem().Set(copied) + return out, nil + case reflect.Struct: + out := reflect.New(v.Type()).Elem() + out.Set(v) + for i := 0; i < v.NumField(); i++ { + field := v.Type().Field(i).Name + value := v.Field(i) + if nameFields[field] && value.Kind() == reflect.String { + 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 { + 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())) + } + out.Field(i).Set(list) + continue + } + if field == "Metadata" || field == "Default" || field == "Properties" || field == "Permissions" { + continue + } + copied, err := copyValue(value) + if err != nil { + return reflect.Value{}, err + } + out.Field(i).Set(copied) + } + if v.Type() == reflect.TypeOf(models.Table{}) { + original := v.Interface().(models.Table) + copied := out.Addr().Interface().(*models.Table) + for originalKey, index := range original.Indexes { + if index == nil { + continue + } + target := copied.Indexes[strings.ToLower(originalKey)] + target.Columns = append([]string(nil), index.Columns...) + for i, reference := range index.Columns { + for _, column := range original.Columns { + if column != nil && (reference == column.Name || reference == `"`+strings.ReplaceAll(column.Name, `"`, `""`)+`"`) { + target.Columns[i] = strings.ToLower(column.Name) + break + } + } + } + } + } + return out, nil + case reflect.Slice: + if v.IsNil() { + return v, nil + } + out := reflect.MakeSlice(v.Type(), v.Len(), v.Len()) + names := map[string]bool{} + for i := 0; i < v.Len(); i++ { + copied, err := copyValue(v.Index(i)) + if err != nil { + return reflect.Value{}, err + } + out.Index(i).Set(copied) + if copied.Kind() == reflect.Pointer && !copied.IsNil() && copied.Elem().Kind() == reflect.Struct { + name := copied.Elem().FieldByName("Name") + if name.IsValid() { + n := name.String() + if names[n] { + return reflect.Value{}, fmt.Errorf("lowercase identifier collision: %q", n) + } + names[n] = true + } + } + } + return out, nil + case reflect.Map: + if v.IsNil() || v.Type().Elem().Kind() != reflect.Pointer { + return v, nil + } + out := reflect.MakeMap(v.Type()) + names := map[string]bool{} + iter := v.MapRange() + for iter.Next() { + key := iter.Key() + if key.Kind() == reflect.String { + key = reflect.ValueOf(strings.ToLower(key.String())).Convert(key.Type()) + } + if out.MapIndex(key).IsValid() { + return reflect.Value{}, fmt.Errorf("lowercase identifier collision: %v", key.Interface()) + } + copied, err := copyValue(iter.Value()) + if err != nil { + return reflect.Value{}, err + } + if copied.Kind() == reflect.Pointer && !copied.IsNil() && copied.Elem().Kind() == reflect.Struct { + name := copied.Elem().FieldByName("Name") + if name.IsValid() { + n := name.String() + if names[n] { + return reflect.Value{}, fmt.Errorf("lowercase identifier collision: %q", n) + } + names[n] = true + } + } + out.SetMapIndex(key, copied) + } + return out, nil + } + return v, nil + } + value, err := copyValue(reflect.ValueOf(db)) + if err != nil { + return nil, err + } + return value.Interface().(*models.Database), nil +} diff --git a/pkg/transform/identifiers_test.go b/pkg/transform/identifiers_test.go new file mode 100644 index 0000000..1b347c7 --- /dev/null +++ b/pkg/transform/identifiers_test.go @@ -0,0 +1,51 @@ +package transform + +import ( + "git.warky.dev/wdevs/relspecgo/pkg/models" + "testing" +) + +func TestLowercaseIdentifiersReferencesAndData(t *testing.T) { + db := models.InitDatabase("DB") + s := models.InitSchema("App") + tbl := models.InitTable("Users", "App") + col := models.InitColumn("ID", "Users", "App") + tbl.Tablespace = "FastStore" + col.Collation = "MyCollation" + col.Default = "Keep CASE" + col.GenerationExpression = `"ID" + 1` + tbl.Columns[col.Name] = col + tbl.Constraints["FK"] = &models.Constraint{Name: "FK", Columns: []string{"ID"}, ReferencedSchema: "Other", ReferencedTable: "Target", ReferencedColumns: []string{"Key"}} + tbl.Indexes["IDX"] = &models.Index{Name: "IDX", Columns: []string{"ID", "lower('VALUE')"}, Include: []string{"ID"}, Where: `"ID" > 0`} + tbl.RefSchema = s + s.Tables = append(s.Tables, tbl) + db.Schemas = append(db.Schemas, s) + got, err := LowercaseIdentifiers(db) + if err != nil { + t.Fatal(err) + } + table := got.Schemas[0].Tables[0] + if table.Tablespace != "faststore" || table.Columns["id"].Collation != "mycollation" || got.Name != "db" || table.Name != "users" || table.Schema != "app" || table.Columns["id"].Name != "id" || table.Constraints["fk"].ReferencedColumns[0] != "key" || table.Indexes["idx"].Include[0] != "id" || table.RefSchema != got.Schemas[0] { + t.Fatalf("bad normalized model: %+v", table) + } + if table.Columns["id"].Default != "Keep CASE" || table.Columns["id"].GenerationExpression != `"ID" + 1` || table.Indexes["idx"].Where != `"ID" > 0` || table.Indexes["idx"].Columns[1] != "lower('VALUE')" { + t.Fatal("data/expression changed") + } + if tbl.Name != "Users" || tbl.Columns["ID"] != col { + t.Fatal("input mutated") + } +} +func TestLowercaseIdentifierCollisions(t *testing.T) { + db := models.InitDatabase("db") + s := models.InitSchema("app") + db.Schemas = append(db.Schemas, s) + s.Tables = []*models.Table{models.InitTable("Users", "app"), models.InitTable("users", "app")} + if _, err := LowercaseIdentifiers(db); err == nil { + t.Fatal("expected table collision") + } + s.Tables = s.Tables[:1] + s.Tables[0].Columns = map[string]*models.Column{"ID": {Name: "ID"}, "id": {Name: "id"}} + if _, err := LowercaseIdentifiers(db); err == nil { + t.Fatal("expected column collision") + } +} diff --git a/pkg/writers/mysql/writer.go b/pkg/writers/mysql/writer.go index 186d491..f37e721 100644 --- a/pkg/writers/mysql/writer.go +++ b/pkg/writers/mysql/writer.go @@ -94,6 +94,11 @@ func (w *Writer) WriteTable(t *models.Table) error { } func (w *Writer) writeTable(s *models.Schema, t *models.Table) error { + _, err := io.WriteString(w.writer, tableStatement(s, t)) + return err +} + +func tableStatement(s *models.Schema, t *models.Table) string { name := t.Name if s != nil { name = fmt.Sprintf("%s.%s", quote(s.Name), quote(t.Name)) @@ -155,32 +160,27 @@ func (w *Writer) writeTable(s *models.Schema, t *models.Table) error { defs = append(defs, fmt.Sprintf(" CONSTRAINT %s UNIQUE (%s)", quote(c.Name), quoted(c.Columns))) } } - sql := fmt.Sprintf("CREATE TABLE IF NOT EXISTS %s (\n%s\n) ENGINE=InnoDB;\n\n", name, strings.Join(defs, ",\n")) - if _, err := io.WriteString(w.writer, sql); err != nil { - return err + return fmt.Sprintf("CREATE TABLE IF NOT EXISTS %s (\n%s\n) ENGINE=InnoDB;\n\n", name, strings.Join(defs, ",\n")) +} + +func databaseStatements(db *models.Database) []string { + var statements []string + for _, schema := range db.Schemas { + for _, table := range schema.Tables { + statements = append(statements, tableStatement(schema, table)) + } } - return nil + return statements } func (w *Writer) execute(dbm *models.Database, conn string) error { - var b strings.Builder - old := w.writer - w.writer = &b - if err := w.writeDatabaseDDL(dbm); err != nil { - w.writer = old - return err - } - w.writer = old + statements := databaseStatements(dbm) db, err := sql.Open("mysql", conn) if err != nil { return fmt.Errorf("failed to connect: %w", err) } defer db.Close() - for _, stmt := range strings.Split(b.String(), ";\n") { - stmt = stripComments(strings.TrimSpace(stmt)) - if stmt == "" { - continue - } + for _, stmt := range statements { if _, err := db.ExecContext(context.Background(), stmt); err != nil { return fmt.Errorf("failed to execute SQL: %w", err) } @@ -188,17 +188,6 @@ func (w *Writer) execute(dbm *models.Database, conn string) error { return nil } -func stripComments(sqlText string) string { - lines := strings.Split(sqlText, "\n") - kept := lines[:0] - for _, line := range lines { - if !strings.HasPrefix(strings.TrimSpace(line), "--") { - kept = append(kept, line) - } - } - return strings.TrimSpace(strings.Join(kept, "\n")) -} - func quote(s string) string { return "`" + strings.ReplaceAll(s, "`", "``") + "`" } func quoted(xs []string) string { out := make([]string, len(xs)) diff --git a/pkg/writers/mysql/writer_test.go b/pkg/writers/mysql/writer_test.go index 1a53fad..ed88e27 100644 --- a/pkg/writers/mysql/writer_test.go +++ b/pkg/writers/mysql/writer_test.go @@ -9,13 +9,6 @@ import ( "git.warky.dev/wdevs/relspecgo/pkg/writers" ) -func TestStripComments(t *testing.T) { - got := stripComments("-- header\nCREATE TABLE `users` (\n `id` INT\n);") - if strings.HasPrefix(got, "--") || !strings.HasPrefix(got, "CREATE TABLE") { - t.Fatalf("stripComments() = %q", got) - } -} - func TestWriterGeneratesMySQLDDL(t *testing.T) { db := models.InitDatabase("app") s := models.InitSchema("app") @@ -42,3 +35,26 @@ func TestWriterGeneratesMySQLDDL(t *testing.T) { } } } + +func TestDatabaseStatementsKeepEmbeddedDelimiters(t *testing.T) { + db := models.InitDatabase("app") + s := models.InitSchema("app") + db.Schemas = append(db.Schemas, s) + for _, name := range []string{"one", "two"} { + table := models.InitTable(name, s.Name) + col := models.InitColumn("text", name, s.Name) + col.Type = "text" + col.Default = "first;\n-- data, not a comment\nlast" + table.Columns[col.Name] = col + s.Tables = append(s.Tables, table) + } + statements := databaseStatements(db) + if len(statements) != 2 { + t.Fatalf("got %d statements", len(statements)) + } + for _, stmt := range statements { + if !strings.Contains(stmt, "first;\n-- data, not a comment\nlast") { + t.Fatalf("default corrupted: %s", stmt) + } + } +} diff --git a/pkg/writers/pgsql/README.md b/pkg/writers/pgsql/README.md index b3b2c3d..3a0c2df 100644 --- a/pkg/writers/pgsql/README.md +++ b/pkg/writers/pgsql/README.md @@ -17,6 +17,28 @@ The PostgreSQL Writer converts RelSpec's internal database model representation - Includes audit triggers (optional) - Handles PostgreSQL-specific data types +## Identifier and value handling + +Generated identifiers are always double-quoted, including reserved words, and +retain the model's case by default. Supply raw model names, without SQL quoting. +Identifiers containing double quotes are escaped by doubling them; NUL is rejected +before database/file output. Catalog checks, comments, audit values, and regclass +names use SQL literal quoting; generated procedural/function bodies choose dollar +delimiters absent from their rendered content. Names in generated line comments +cannot create new SQL lines. + +Use CLI `--lowercase`, job option `lowercase_identifiers: true`, or PostgreSQL +`WriterOptions.LowercaseIdentifiers: true` to lowercase model names and structured +references. The transformation copies the model and rejects name collisions. Other +library writers can use `transform.LowercaseIdentifiers` before writing. + +SQL expressions are executable input: types, defaults, generated expressions, +checks, index expressions/predicates, dialect directive clauses, audit user +expressions, and `pgsql_using` / `pgsql_preflight` are not sanitized or case-folded. +Adjust identifier references inside these expressions yourself when lowercasing. +Enum values, comments, and other data retain their case. Migration matching respects +identifier case; `"Users"` and `"users"` are distinct objects. + ## Usage ### Basic Example diff --git a/pkg/writers/pgsql/column_comment_test.go b/pkg/writers/pgsql/column_comment_test.go index 4c218b2..9a15770 100644 --- a/pkg/writers/pgsql/column_comment_test.go +++ b/pkg/writers/pgsql/column_comment_test.go @@ -22,8 +22,9 @@ func TestCurrentColumnHasDescription(t *testing.T) { want bool }{ {"nil table", nil, &models.Column{Name: "email", Description: "x"}, false}, - {"match ignoring case and whitespace", table, &models.Column{Name: "email", Description: "the email"}, true}, - {"different description", table, &models.Column{Name: "email", Description: "other"}, false}, + {"match ignoring whitespace", table, &models.Column{Name: "Email", Description: "the email"}, true}, + {"different description", table, &models.Column{Name: "Email", Description: "other"}, false}, + {"different identifier case", table, &models.Column{Name: "email", Description: "the email"}, false}, {"column missing", table, &models.Column{Name: "age", Description: "x"}, false}, } for _, tt := range tests { @@ -41,12 +42,12 @@ func TestExecuteCommentColumn(t *testing.T) { t.Fatal(err) } got, err := te.ExecuteCommentColumn(CommentColumnData{ - SchemaName: "public", TableName: "users", ColumnName: "email", Comment: "it''s", + SchemaName: "public", TableName: "users", ColumnName: "email", Comment: "it's", }) if err != nil { t.Fatal(err) } - if !strings.Contains(got, "COMMENT ON COLUMN") || !strings.Contains(got, "public.users") || + if !strings.Contains(got, "COMMENT ON COLUMN") || !strings.Contains(got, "\"public\".\"users\"") || !strings.Contains(got, "email") || !strings.Contains(got, "IS 'it''s';") { t.Errorf("unexpected output: %s", got) } diff --git a/pkg/writers/pgsql/diff_statements_test.go b/pkg/writers/pgsql/diff_statements_test.go index d936fb2..9750018 100644 --- a/pkg/writers/pgsql/diff_statements_test.go +++ b/pkg/writers/pgsql/diff_statements_test.go @@ -57,8 +57,8 @@ func TestDiffStatements(t *testing.T) { current: func() *models.Database { return diffTestDB("public", map[string]map[string]string{"users": {"id": "integer"}}) }, - wantContain: []string{"CREATE TABLE IF NOT EXISTS public.posts"}, - wantAbsent: []string{"public.users"}, + wantContain: []string{"CREATE TABLE IF NOT EXISTS \"public\".\"posts\""}, + wantAbsent: []string{"\"public\".\"users\""}, }, { name: "missing column is added and existing columns are skipped", @@ -68,7 +68,7 @@ func TestDiffStatements(t *testing.T) { current: func() *models.Database { return diffTestDB("public", map[string]map[string]string{"users": {"id": "integer"}}) }, - wantContain: []string{"ADD COLUMN IF NOT EXISTS email text"}, + wantContain: []string{"ADD COLUMN IF NOT EXISTS \"email\" text"}, wantAbsent: []string{"COLUMN id"}, }, { @@ -79,7 +79,7 @@ func TestDiffStatements(t *testing.T) { current: func() *models.Database { return diffTestDB("public", map[string]map[string]string{"users": {"id": "integer"}}) }, - wantContain: []string{"ALTER COLUMN id TYPE bigint"}, + wantContain: []string{"ALTER COLUMN \"id\" TYPE bigint"}, }, { name: "missing non-public schema is created before its tables", @@ -88,8 +88,8 @@ func TestDiffStatements(t *testing.T) { }, current: func() *models.Database { return models.InitDatabase("testdb") }, wantContain: []string{ - "CREATE SCHEMA IF NOT EXISTS app", - "CREATE TABLE IF NOT EXISTS app.users", + "CREATE SCHEMA IF NOT EXISTS \"app\"", + "CREATE TABLE IF NOT EXISTS \"app\".\"users\"", }, }, } @@ -149,7 +149,7 @@ func TestDiffStatements_NewTableCreatesPrimaryKeySequence(t *testing.T) { id := models.InitColumn("id", "users", "public") id.Type = "integer" id.IsPrimaryKey = true - id.Default = "nextval('public.users_id_seq'::regclass)" + id.Default = "nextval('\"public\".\"users_id_seq\"'::regclass)" table.Columns["id"] = id schema.Tables = append(schema.Tables, table) model.Schemas = append(model.Schemas, schema) @@ -160,7 +160,7 @@ func TestDiffStatements_NewTableCreatesPrimaryKeySequence(t *testing.T) { t.Fatalf("diffStatements failed: %v", err) } out := diffJoin(stmts) - if !strings.Contains(out, "CREATE SEQUENCE IF NOT EXISTS public.users_id_seq") { + if !strings.Contains(out, "CREATE SEQUENCE IF NOT EXISTS \"public\".\"users_id_seq\"") { t.Fatalf("expected sequence creation, got:\n%s", out) } @@ -193,7 +193,7 @@ func TestDiffStatements_NewIndexAndChangedIndexRecreated(t *testing.T) { if err != nil { t.Fatalf("diffStatements failed: %v", err) } - if !strings.Contains(diffJoin(stmts), "CREATE INDEX IF NOT EXISTS idx_users_lookup") { + if !strings.Contains(diffJoin(stmts), "CREATE INDEX IF NOT EXISTS \"idx_users_lookup\"") { t.Fatalf("expected index creation, got:\n%s", diffJoin(stmts)) } @@ -329,7 +329,7 @@ func TestDiffStatements_ChangedPrimaryKeyIsRecreated(t *testing.T) { if err != nil { t.Fatalf("diffStatements failed: %v", err) } - if !strings.Contains(diffJoin(stmts), "DROP CONSTRAINT IF EXISTS pk_public_users") { + if !strings.Contains(diffJoin(stmts), "DROP CONSTRAINT IF EXISTS \"pk_public_users\"") { t.Fatalf("expected old primary key to be dropped, got:\n%s", diffJoin(stmts)) } } @@ -345,7 +345,7 @@ func TestDiffStatements_ChangedCommentEmitted(t *testing.T) { if err != nil { t.Fatalf("diffStatements failed: %v", err) } - if !strings.Contains(diffJoin(stmts), "COMMENT ON TABLE public.users IS 'new'") { + if !strings.Contains(diffJoin(stmts), "COMMENT ON TABLE \"public\".\"users\" IS 'new'") { t.Fatalf("expected changed comment, got:\n%s", diffJoin(stmts)) } } diff --git a/pkg/writers/pgsql/directives_test.go b/pkg/writers/pgsql/directives_test.go index 0df3049..a199477 100644 --- a/pkg/writers/pgsql/directives_test.go +++ b/pkg/writers/pgsql/directives_test.go @@ -55,7 +55,7 @@ func TestPgDirectives_WriteDatabasePath(t *testing.T) { for _, want := range []string{ ") PARTITION BY RANGE (created_at) TABLESPACE fast_data", "GENERATED ALWAYS AS IDENTITY", - "WITH (fillfactor=90) TABLESPACE idx_space", + "WITH (fillfactor=90) TABLESPACE \"idx_space\"", } { if !strings.Contains(out, want) { t.Errorf("missing %q in:\n%s", want, out) @@ -78,7 +78,7 @@ func TestPgDirectives_WriteSchemaPath(t *testing.T) { if !strings.Contains(out, ") PARTITION BY RANGE (created_at) TABLESPACE fast_data;") { t.Errorf("table suffix missing from WriteSchema path:\n%s", out) } - if !strings.Contains(out, "WITH (fillfactor=90) TABLESPACE idx_space") { + if !strings.Contains(out, "WITH (fillfactor=90) TABLESPACE \"idx_space\"") { t.Errorf("index clauses missing from WriteSchema path:\n%s", out) } } diff --git a/pkg/writers/pgsql/generated_column_test.go b/pkg/writers/pgsql/generated_column_test.go index ac90f0e..f984d39 100644 --- a/pkg/writers/pgsql/generated_column_test.go +++ b/pkg/writers/pgsql/generated_column_test.go @@ -88,8 +88,8 @@ func TestGeneratedColumns_FullDDL(t *testing.T) { } out := diffJoin(stmts) for _, s := range []string{ - "full_name text GENERATED ALWAYS AS (first || ' ' || last) STORED", - "seq bigint GENERATED ALWAYS AS IDENTITY NOT NULL", + "\"full_name\" text GENERATED ALWAYS AS (first || ' ' || last) STORED", + "\"seq\" bigint GENERATED ALWAYS AS IDENTITY NOT NULL", } { if !strings.Contains(out, s) { t.Errorf("missing %q:\n%s", s, out) diff --git a/pkg/writers/pgsql/identifiers.go b/pkg/writers/pgsql/identifiers.go new file mode 100644 index 0000000..a2c9da4 --- /dev/null +++ b/pkg/writers/pgsql/identifiers.go @@ -0,0 +1,116 @@ +package pgsql + +import ( + "fmt" + "reflect" + "strings" + + "git.warky.dev/wdevs/relspecgo/pkg/models" + "git.warky.dev/wdevs/relspecgo/pkg/transform" + "git.warky.dev/wdevs/relspecgo/pkg/writers" +) + +// prepareModel rejects NUL before creating files or executing SQL. SQL names +// cannot contain NUL; pgx's identifier sanitizer would otherwise remove it. +func prepareModel(db *models.Database, lower bool) (*models.Database, error) { + if db == nil { + return nil, fmt.Errorf("database is required") + } + seen := map[any]bool{} + var validate func(reflect.Value) error + validate = func(v reflect.Value) error { + switch v.Kind() { + case reflect.Pointer: + if v.IsNil() || v.Type().Elem().PkgPath() != reflect.TypeOf(models.Database{}).PkgPath() { + return nil + } + if seen[v.Interface()] { + return nil + } + seen[v.Interface()] = true + return validate(v.Elem()) + case reflect.Struct: + for i := 0; i < v.NumField(); i++ { + if v.Type().Field(i).Name == "Metadata" { + continue + } + if err := validate(v.Field(i)); err != nil { + return err + } + } + case reflect.String: + if strings.ContainsRune(v.String(), 0) { + return fmt.Errorf("PostgreSQL model contains a NUL byte") + } + case reflect.Slice: + for i := 0; i < v.Len(); i++ { + if err := validate(v.Index(i)); err != nil { + return err + } + } + case reflect.Map: + if v.Type().Elem().Kind() == reflect.Pointer { + iter := v.MapRange() + for iter.Next() { + if err := validate(iter.Value()); err != nil { + return err + } + } + } + } + return nil + } + if err := validate(reflect.ValueOf(db)); err != nil { + return nil, err + } + if lower { + return transform.LowercaseIdentifiers(db) + } + return db, nil +} + +func (w *Writer) prepareSchema(schema *models.Schema) (*models.Schema, error) { + db, err := prepareModel(&models.Database{Schemas: []*models.Schema{schema}}, w.options.LowercaseIdentifiers) + if err != nil { + return nil, err + } + return db.Schemas[0], nil +} + +// regclassLastName decodes quoted identifiers in a qualified regclass spelling. +func regclassLastName(text string) string { + var name strings.Builder + quoted := false + for i := 0; i < len(text); i++ { + c := text[i] + if c == '"' { + if quoted && i+1 < len(text) && text[i+1] == '"' { + name.WriteByte('"') + i++ + } else { + quoted = !quoted + } + continue + } + if c == '.' && !quoted { + name.Reset() + continue + } + name.WriteByte(c) + } + return name.String() +} + +// quoteDefaultValue keeps explicit SQL expressions intact and uses the same +// backslash-safe literal quoting as other PostgreSQL value contexts. +func quoteDefaultValue(value, sqlType string) string { + trimmed := strings.TrimSpace(value) + if len(trimmed) >= 3 && (trimmed[0] == 'E' || trimmed[0] == 'e') && trimmed[1] == '\'' && trimmed[len(trimmed)-1] == '\'' { + return trimmed + } + formatted := writers.QuoteDefaultValue(value, sqlType) + if formatted == "'"+strings.ReplaceAll(trimmed, "'", "''")+"'" { + return quote(trimmed) + } + return formatted +} diff --git a/pkg/writers/pgsql/migration_writer.go b/pkg/writers/pgsql/migration_writer.go index e1d872d..3cfdadd 100644 --- a/pkg/writers/pgsql/migration_writer.go +++ b/pkg/writers/pgsql/migration_writer.go @@ -61,6 +61,16 @@ func (w *MigrationWriter) GenerateScripts(model, current *models.Database) ([]Mi current = models.InitDatabase(model.Name) } + var err error + model, err = prepareModel(model, w.options.LowercaseIdentifiers) + if err != nil { + return nil, err + } + current, err = prepareModel(current, false) + if err != nil { + return nil, err + } + // Check if audit is configured in metadata var auditConfig *AuditConfig if w.options.Metadata != nil { @@ -93,7 +103,7 @@ func (w *MigrationWriter) GenerateScripts(model, current *models.Database) ([]Mi if cs == nil { continue } - if strings.EqualFold(cs.Name, modelSchema.Name) { + if cs.Name == modelSchema.Name { currentSchema = cs break } @@ -166,7 +176,7 @@ func (w *MigrationWriter) WriteMigration(model, current *models.Database) error // Write header fmt.Fprintf(w.writer, "-- PostgreSQL Migration Script\n") fmt.Fprintf(w.writer, "-- Generated by RelSpec\n") - fmt.Fprintf(w.writer, "-- Source: %s -> %s\n", current.Name, model.Name) + fmt.Fprintf(w.writer, "-- Source: %s -> %s\n", commentText(current.Name), commentText(model.Name)) if w.options.ContinueOnError { fmt.Fprintf(w.writer, "\\set ON_ERROR_STOP off\n") } @@ -175,7 +185,7 @@ func (w *MigrationWriter) WriteMigration(model, current *models.Database) error // Write scripts for _, script := range scripts { fmt.Fprintf(w.writer, "-- Priority: %d | Type: %s | Object: %s\n", - script.Priority, script.ObjectType, script.ObjectName) + script.Priority, commentText(script.ObjectType), commentText(script.ObjectName)) fmt.Fprintf(w.writer, "%s\n\n", script.Body) } @@ -256,12 +266,12 @@ func (w *MigrationWriter) generateDropScripts(model, current *models.Schema) ([] // Build map of model tables for quick lookup modelTables := make(map[string]*models.Table) for _, table := range model.Tables { - modelTables[strings.ToLower(table.Name)] = table + modelTables[table.Name] = table } // Find constraints to drop for _, currentTable := range current.Tables { - modelTable, existsInModel := modelTables[strings.ToLower(currentTable.Name)] + modelTable, existsInModel := modelTables[currentTable.Name] if !existsInModel { continue @@ -296,7 +306,7 @@ func (w *MigrationWriter) generateDropScripts(model, current *models.Schema) ([] if refSchema == "" { refSchema = current.Name } - if strings.EqualFold(refTable, currentTable.Name) && strings.EqualFold(refSchema, current.Name) { + if (refTable == currentTable.Name) && (refSchema == current.Name) { fkKey := fmt.Sprintf("%s.%s.%s", current.Name, otherTable.Name, fkName) if !droppedFKs[fkKey] { droppedFKs[fkKey] = true @@ -395,13 +405,13 @@ func (w *MigrationWriter) generateTableScripts(model, current *models.Schema) ([ currentTables := make(map[string]*models.Table) if current != nil { for _, table := range current.Tables { - currentTables[strings.ToLower(table.Name)] = table + currentTables[table.Name] = table } } // Process each model table for _, modelTable := range model.Tables { - currentTable, exists := currentTables[strings.ToLower(modelTable.Name)] + currentTable, exists := currentTables[modelTable.Name] if !exists { // Table doesn't exist, create it @@ -439,12 +449,12 @@ func (w *MigrationWriter) generateAlterTableScripts(schema *models.Schema, model // Build map of current columns currentColumns := make(map[string]*models.Column) for name, col := range currentTable.Columns { - currentColumns[strings.ToLower(name)] = col + currentColumns[name] = col } // Check each model column for _, modelCol := range sortColumns(modelTable.Columns) { - currentCol, exists := currentColumns[strings.ToLower(modelCol.Name)] + currentCol, exists := currentColumns[modelCol.Name] if !exists { // Column doesn't exist, add it @@ -574,13 +584,13 @@ func (w *MigrationWriter) generateIndexScripts(model, current *models.Schema) ([ currentTables := make(map[string]*models.Table) if current != nil { for _, table := range current.Tables { - currentTables[strings.ToLower(table.Name)] = table + currentTables[table.Name] = table } } // Process each model table for _, modelTable := range model.Tables { - currentTable := currentTables[strings.ToLower(modelTable.Name)] + currentTable := currentTables[modelTable.Name] // Process primary keys first - check explicit constraints foundExplicitPK := false @@ -603,7 +613,7 @@ func (w *MigrationWriter) generateIndexScripts(model, current *models.Schema) ([ SchemaName: model.Name, TableName: modelTable.Name, ConstraintName: constraintName, - Columns: strings.Join(constraint.Columns, ", "), + Columns: quotedIdentifiers(constraint.Columns), }) if err != nil { return nil, err @@ -627,12 +637,12 @@ func (w *MigrationWriter) generateIndexScripts(model, current *models.Schema) ([ pkColumns := []string{} for _, col := range modelTable.Columns { if col.IsPrimaryKey { - pkColumns = append(pkColumns, col.SQLName()) + pkColumns = append(pkColumns, col.Name) } } if len(pkColumns) > 0 { sort.Strings(pkColumns) - constraintName := fmt.Sprintf("pk_%s_%s", model.SQLName(), modelTable.SQLName()) + constraintName := fmt.Sprintf("pk_%s_%s", model.Name, modelTable.Name) shouldCreate := true if currentTable != nil { @@ -650,7 +660,7 @@ func (w *MigrationWriter) generateIndexScripts(model, current *models.Schema) ([ SchemaName: model.Name, TableName: modelTable.Name, ConstraintName: constraintName, - Columns: strings.Join(pkColumns, ", "), + Columns: quotedIdentifiers(pkColumns), }) if err != nil { return nil, err @@ -750,7 +760,7 @@ func buildIndexColumnExpressionsFiltered(table *models.Table, index *models.Inde continue } - colExpr := col.SQLName() + colExpr := quoteIdent(col.Name) if opClass := indexOperatorClassForColumn(col, indexType, index.Comment); opClass != "" { colExpr = fmt.Sprintf("%s %s", colExpr, opClass) } @@ -767,12 +777,12 @@ func (w *MigrationWriter) generateUniqueConstraintScripts(model, current *models currentTables := make(map[string]*models.Table) if current != nil { for _, table := range current.Tables { - currentTables[strings.ToLower(table.Name)] = table + currentTables[table.Name] = table } } for _, modelTable := range model.Tables { - currentTable := currentTables[strings.ToLower(modelTable.Name)] + currentTable := currentTables[modelTable.Name] for _, constraint := range sortConstraints(modelTable.Constraints) { if constraint.Type != models.UniqueConstraint { @@ -788,7 +798,7 @@ func (w *MigrationWriter) generateUniqueConstraintScripts(model, current *models columnExprs := make([]string, 0, len(constraint.Columns)) for _, colName := range constraint.Columns { if col, ok := modelTable.Columns[colName]; ok { - columnExprs = append(columnExprs, col.SQLName()) + columnExprs = append(columnExprs, quoteIdent(col.Name)) } } if len(columnExprs) == 0 { @@ -838,7 +848,7 @@ func adoptableUniqueIndex(table *models.Table, constraint *models.Constraint) st return "" } for i, col := range index.Columns { - if !strings.EqualFold(col, constraint.Columns[i]) { + if !(col == constraint.Columns[i]) { return "" } } @@ -855,13 +865,13 @@ func (w *MigrationWriter) generateForeignKeyScripts(model, current *models.Schem currentTables := make(map[string]*models.Table) if current != nil { for _, table := range current.Tables { - currentTables[strings.ToLower(table.Name)] = table + currentTables[table.Name] = table } } // Process each model table for _, modelTable := range model.Tables { - currentTable := currentTables[strings.ToLower(modelTable.Name)] + currentTable := currentTables[modelTable.Name] // Process each constraint for _, constraint := range sortConstraints(modelTable.Constraints) { @@ -904,10 +914,10 @@ func (w *MigrationWriter) generateForeignKeyScripts(model, current *models.Schem SchemaName: model.Name, TableName: modelTable.Name, ConstraintName: constraintName, - SourceColumns: strings.Join(constraint.Columns, ", "), + SourceColumns: quotedIdentifiers(constraint.Columns), TargetSchema: constraint.ReferencedSchema, TargetTable: constraint.ReferencedTable, - TargetColumns: strings.Join(constraint.ReferencedColumns, ", "), + TargetColumns: quotedIdentifiers(constraint.ReferencedColumns), OnDelete: onDelete, OnUpdate: onUpdate, Deferral: deferral, @@ -939,20 +949,20 @@ func (w *MigrationWriter) generateCommentScripts(model, current *models.Schema) currentTables := make(map[string]*models.Table) if current != nil { for _, table := range current.Tables { - currentTables[strings.ToLower(table.Name)] = table + currentTables[table.Name] = table } } // Process each model table for _, modelTable := range model.Tables { - currentTable := currentTables[strings.ToLower(modelTable.Name)] + currentTable := currentTables[modelTable.Name] // Table comment (skipped when the live table already carries the same comment) if modelTable.Description != "" && (currentTable == nil || strings.TrimSpace(currentTable.Description) != strings.TrimSpace(modelTable.Description)) { sql, err := w.executor.ExecuteCommentTable(CommentTableData{ SchemaName: model.Name, TableName: modelTable.Name, - Comment: escapeQuote(modelTable.Description), + Comment: modelTable.Description, }) if err != nil { return nil, err @@ -976,7 +986,7 @@ func (w *MigrationWriter) generateCommentScripts(model, current *models.Schema) SchemaName: model.Name, TableName: modelTable.Name, ColumnName: col.Name, - Comment: escapeQuote(col.Description), + Comment: col.Description, }) if err != nil { return nil, err @@ -1229,21 +1239,21 @@ func constraintsEqual(c1, c2 *models.Constraint) bool { return false } for i, col := range c1.Columns { - if !strings.EqualFold(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 !strings.EqualFold(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 !strings.EqualFold(col, c2.ReferencedColumns[i]) { + if !(col == c2.ReferencedColumns[i]) { return false } } @@ -1273,7 +1283,7 @@ func indexesEqual(idx1, idx2 *models.Index) bool { return false } for i, col := range idx1.Columns { - if !strings.EqualFold(col, idx2.Columns[i]) { + if !(col == idx2.Columns[i]) { return false } } @@ -1327,7 +1337,7 @@ func currentColumnHasDescription(currentTable *models.Table, col *models.Column) return false } for _, cc := range currentTable.Columns { - if strings.EqualFold(cc.Name, col.Name) { + if cc.Name == col.Name { return strings.TrimSpace(cc.Description) == strings.TrimSpace(col.Description) } } @@ -1355,7 +1365,7 @@ func primaryKeyColumnsMatch(modelTable *models.Table, current *models.Constraint return false } for i, col := range modelCols { - if !strings.EqualFold(col, current.Columns[i]) { + if !(col == current.Columns[i]) { return false } } diff --git a/pkg/writers/pgsql/migration_writer_test.go b/pkg/writers/pgsql/migration_writer_test.go index 687e660..4b183d7 100644 --- a/pkg/writers/pgsql/migration_writer_test.go +++ b/pkg/writers/pgsql/migration_writer_test.go @@ -89,7 +89,7 @@ func TestWriteMigration_ArrayDefault(t *testing.T) { } output := buf.String() - if !strings.Contains(output, "tags text[] DEFAULT '{}' NOT NULL") { + if !strings.Contains(output, "\"tags\" text[] DEFAULT '{}' NOT NULL") { t.Fatalf("expected normalized array default in migration, got:\n%s", output) } if strings.Contains(output, "'''{}'''") { @@ -128,10 +128,10 @@ func TestWriteMigration_AltersColumnTypeWhenActualTypeDiffers(t *testing.T) { } output := buf.String() - if !strings.Contains(output, "ALTER TABLE public.learnings") || !strings.Contains(output, "ALTER COLUMN details TYPE text") { + if !strings.Contains(output, "ALTER TABLE \"public\".\"learnings\"") || !strings.Contains(output, "ALTER COLUMN \"details\" TYPE text") { t.Fatalf("expected migration to alter mismatched column type, got:\n%s", output) } - if !strings.Contains(output, `ALTER COLUMN details TYPE text USING details::text;`) { + if !strings.Contains(output, `ALTER COLUMN "details" TYPE text USING "details"::text;`) { t.Fatalf("expected migration type alter to include USING cast, got:\n%s", output) } } @@ -167,7 +167,7 @@ func TestWriteMigration_AltersColumnTypePreservesConversionFailure(t *testing.T) } output := buf.String() - if !strings.Contains(output, "ALTER COLUMN details TYPE integer USING details::integer;") { + if !strings.Contains(output, "ALTER COLUMN \"details\" TYPE integer USING \"details\"::integer;") { t.Fatalf("expected explicit type conversion, got:\n%s", output) } for _, unsafe := range []string{"EXCEPTION WHEN OTHERS", "renamed_column", "RENAME COLUMN"} { @@ -210,7 +210,7 @@ func TestWriteMigration_AltersColumnNullabilityWhenNotNullDiffers(t *testing.T) } output := buf.String() - if !strings.Contains(output, "ALTER COLUMN rid_service_instance_type DROP NOT NULL") { + if !strings.Contains(output, "ALTER COLUMN \"rid_service_instance_type\" DROP NOT NULL") { t.Fatalf("expected migration to drop NOT NULL on existing column, got:\n%s", output) } } @@ -331,13 +331,13 @@ func TestWriteMigration_UsesStorageTypeForSerialAlterStatements(t *testing.T) { } output := buf.String() - if !strings.Contains(output, "ALTER COLUMN id TYPE bigint") { + if !strings.Contains(output, "ALTER COLUMN \"id\" TYPE bigint") { t.Fatalf("expected serial alter to use bigint storage type, got:\n%s", output) } - if strings.Contains(output, "ALTER COLUMN id TYPE bigserial;") { + if strings.Contains(output, "ALTER COLUMN \"id\" TYPE bigserial;") { t.Fatalf("did not expect invalid bigserial alter statement, got:\n%s", output) } - if !strings.Contains(output, `ALTER COLUMN id TYPE bigint USING id::bigint;`) { + if !strings.Contains(output, `ALTER COLUMN "id" TYPE bigint USING "id"::bigint;`) { t.Fatalf("expected serial alter to include USING cast, got:\n%s", output) } } @@ -373,7 +373,7 @@ func TestWriteMigration_ArrayAlterIncludesUsingCast(t *testing.T) { } output := buf.String() - if !strings.Contains(output, `ALTER COLUMN tags TYPE text[] USING tags::text[];`) { + if !strings.Contains(output, `ALTER COLUMN "tags" TYPE text[] USING "tags"::text[];`) { t.Fatalf("expected array alter to include USING cast, got:\n%s", output) } } @@ -410,7 +410,7 @@ func TestWriteMigration_DoesNotAlterEquivalentNormalizedColumnType(t *testing.T) } output := buf.String() - if strings.Contains(output, "ALTER COLUMN email TYPE") { + if strings.Contains(output, "ALTER COLUMN \"email\" TYPE") { t.Fatalf("did not expect alter type for equivalent normalized types, got:\n%s", output) } } @@ -493,7 +493,7 @@ func TestWriteMigration_GinIndexOnTextUsesTrigramOperatorClass(t *testing.T) { if !strings.Contains(output, "CREATE EXTENSION IF NOT EXISTS pg_trgm;") { t.Fatalf("expected trigram extension for text GIN migration index, got:\n%s", output) } - if !strings.Contains(output, "USING gin (title gin_trgm_ops)") { + if !strings.Contains(output, "USING gin (\"title\" gin_trgm_ops)") { t.Fatalf("expected GIN text index to include gin_trgm_ops, got:\n%s", output) } } @@ -514,7 +514,7 @@ func TestWriteMigration_GinIndexOnQuotedTextColumnUsesTrigramOperatorClass(t *te index := &models.Index{ Name: "idx_agent_personas_name_gin", Type: "gin", - Columns: []string{`"name"`}, + Columns: []string{"name"}, } table.Indexes[index.Name] = index @@ -533,7 +533,7 @@ func TestWriteMigration_GinIndexOnQuotedTextColumnUsesTrigramOperatorClass(t *te } output := buf.String() - if !strings.Contains(output, "USING gin (name gin_trgm_ops)") { + if !strings.Contains(output, "USING gin (\"name\" gin_trgm_ops)") { t.Fatalf("expected quoted text column GIN index to include gin_trgm_ops, got:\n%s", output) } } @@ -573,7 +573,7 @@ func TestWriteMigration_GinIndexOnTextArrayDoesNotUseTrigramOperatorClass(t *tes } output := buf.String() - if !strings.Contains(output, "USING gin (tags array_ops)") { + if !strings.Contains(output, "USING gin (\"tags\" array_ops)") { t.Fatalf("expected GIN array index with array_ops, got:\n%s", output) } if strings.Contains(output, "gin_trgm_ops") { @@ -616,7 +616,7 @@ func TestWriteMigration_GinIndexOnJSONBUsesJSONBOperatorClass(t *testing.T) { } output := buf.String() - if !strings.Contains(output, "USING gin (details jsonb_ops)") { + if !strings.Contains(output, "USING gin (\"details\" jsonb_ops)") { t.Fatalf("expected GIN jsonb index to include jsonb_ops, got:\n%s", output) } if strings.Contains(output, "gin_trgm_ops") { @@ -660,7 +660,7 @@ func TestWriteMigration_GinIndexOnJSONBIgnoresIncompatibleTrigramOperatorClass(t } output := buf.String() - if !strings.Contains(output, "USING gin (details jsonb_ops)") { + if !strings.Contains(output, "USING gin (\"details\" jsonb_ops)") { t.Fatalf("expected incompatible trigram hint on jsonb to fall back to jsonb_ops, got:\n%s", output) } } @@ -721,20 +721,20 @@ func TestWriteMigration_WithAudit(t *testing.T) { t.Logf("Generated migration with audit:\n%s", output) // Verify audit tables - if !strings.Contains(output, "CREATE TABLE IF NOT EXISTS public.atevent") { + if !strings.Contains(output, "CREATE TABLE IF NOT EXISTS \"public\".atevent") { t.Error("Migration missing atevent table") } - if !strings.Contains(output, "CREATE TABLE IF NOT EXISTS public.atdetail") { + if !strings.Contains(output, "CREATE TABLE IF NOT EXISTS \"public\".atdetail") { t.Error("Migration missing atdetail table") } // Verify audit function - if !strings.Contains(output, "CREATE OR REPLACE FUNCTION public.tf_audit_users()") { + if !strings.Contains(output, "CREATE OR REPLACE FUNCTION \"public\".\"tf_audit_users\"()") { t.Error("Migration missing audit function") } // Verify audit trigger - if !strings.Contains(output, "CREATE TRIGGER t_audit_users") { + if !strings.Contains(output, "CREATE TRIGGER \"t_audit_users\"") { t.Error("Migration missing audit trigger") } @@ -766,13 +766,13 @@ func TestTemplateExecutor_CreateTable(t *testing.T) { t.Logf("Generated SQL:\n%s", sql) - if !strings.Contains(sql, "CREATE TABLE IF NOT EXISTS public.test_table") { + if !strings.Contains(sql, "CREATE TABLE IF NOT EXISTS \"public\".\"test_table\"") { t.Error("SQL missing CREATE TABLE statement") } - if !strings.Contains(sql, "id integer NOT NULL") { + if !strings.Contains(sql, "\"id\" integer NOT NULL") { t.Error("SQL missing id column definition") } - if !strings.Contains(sql, "name text DEFAULT 'unknown'") { + if !strings.Contains(sql, "\"name\" text DEFAULT 'unknown'") { t.Error("SQL missing name column definition") } } @@ -810,7 +810,7 @@ func TestTemplateExecutor_AuditFunction(t *testing.T) { t.Logf("Generated SQL:\n%s", sql) - if !strings.Contains(sql, "CREATE OR REPLACE FUNCTION public.tf_audit_users()") { + if !strings.Contains(sql, "CREATE OR REPLACE FUNCTION \"public\".\"tf_audit_users\"()") { t.Error("SQL missing function definition") } if !strings.Contains(sql, "IF TG_OP = 'INSERT'") { @@ -983,8 +983,8 @@ func TestWriteMigration_VectorAndPostGISIndexes(t *testing.T) { "CREATE EXTENSION IF NOT EXISTS vector;", "vector(1536)", "geometry(Point,4326)", - "USING ivfflat (embedding vector_cosine_ops) WITH (lists = 100)", - "USING gist (location)", + "USING ivfflat (\"embedding\" vector_cosine_ops) WITH (lists = 100)", + "USING gist (\"location\")", } { if !strings.Contains(output, want) { t.Fatalf("expected migration to contain %q, got:\n%s", want, output) @@ -1181,7 +1181,7 @@ func TestWriteMigration_UniqueConstraintAdoptsSameNamedModelIndex(t *testing.T) if got := strings.Contains(output, "UNIQUE USING INDEX"); got != tt.wantAdopt { t.Errorf("USING INDEX present = %v, want %v; output:\n%s", got, tt.wantAdopt, output) } - if got := strings.Contains(output, "UNIQUE (guid)") || strings.Contains(output, "UNIQUE (\"guid\")"); got == tt.wantAdopt { + if got := strings.Contains(output, "UNIQUE (\"guid\")") || strings.Contains(output, "UNIQUE (\"guid\")"); got == tt.wantAdopt { t.Errorf("column-list ADD CONSTRAINT present = %v, want %v; output:\n%s", got, !tt.wantAdopt, output) } if got := strings.Contains(output, "DROP INDEX"); got != tt.wantDrop { diff --git a/pkg/writers/pgsql/quoting_test.go b/pkg/writers/pgsql/quoting_test.go new file mode 100644 index 0000000..1a3396f --- /dev/null +++ b/pkg/writers/pgsql/quoting_test.go @@ -0,0 +1,251 @@ +package pgsql + +import ( + "bytes" + "context" + "os" + "path/filepath" + "strings" + "testing" + + "git.warky.dev/wdevs/relspecgo/pkg/models" + "git.warky.dev/wdevs/relspecgo/pkg/writers" +) + +func TestQuotedNamesAndLowercaseOption(t *testing.T) { + db := models.InitDatabase("DB\nSELECT 'header';") + s := models.InitSchema("MySchema") + tbl := models.InitTable("Select", "MySchema") + col := models.InitColumn("Key", "Select", "MySchema") + col.Type = "text" + col.Default = "Keep CASE" + tbl.Columns[col.Name] = col + s.Tables = append(s.Tables, tbl) + db.Schemas = append(db.Schemas, s) + for _, lower := range []bool{false, true} { + w := NewWriter(&writers.WriterOptions{LowercaseIdentifiers: lower}) + stmts, err := w.GenerateDatabaseStatements(db) + if err != nil { + t.Fatal(err) + } + out := strings.Join(stmts, "\n") + want := `"MySchema"."Select"` + if lower { + want = `"myschema"."select"` + } + if !strings.Contains(out, want) || !strings.Contains(out, "'Keep CASE'") || strings.Contains(out, "\nSELECT 'header';") { + t.Fatalf("%s", out) + } + } + if db.Name != "DB\nSELECT 'header';" || tbl.Name != "Select" { + t.Fatal("input changed") + } + path := filepath.Join(t.TempDir(), "out.sql") + if err := os.WriteFile(path, []byte("keep"), 0600); err != nil { + t.Fatal(err) + } + col.Name = "bad\x00name" + if err := NewWriter(&writers.WriterOptions{OutputPath: path}).WriteDatabase(db); err == nil { + t.Fatal("expected NUL rejection") + } + data, _ := os.ReadFile(path) + if string(data) != "keep" { + t.Fatal("output changed before validation") + } +} + +func TestAllBlockTemplatesAvoidDelimiterCollisions(t *testing.T) { + te, err := NewTemplateExecutor(false) + if err != nil { + t.Fatal(err) + } + name := "名'\"\\\n$$ $body$ $relspec_0$ $relspec_1$" + cases := []struct { + name string + data any + }{ + {"alter_column_default_with_check.tmpl", AlterColumnDefaultWithCheckData{SchemaName: name, TableName: name, ColumnName: name, SetDefault: true, DefaultValue: quote(name)}}, + {"alter_column_nullability_with_check.tmpl", AlterColumnNullabilityWithCheckData{SchemaName: name, TableName: name, ColumnName: name}}, + {"create_check_constraint.tmpl", CreateCheckConstraintData{SchemaName: name, TableName: name, ConstraintName: name, Expression: "true"}}, + {"create_foreign_key_with_check.tmpl", CreateForeignKeyWithCheckData{SchemaName: name, TableName: name, ConstraintName: name}}, + {"create_primary_key.tmpl", CreatePrimaryKeyData{SchemaName: name, TableName: name, ConstraintName: name}}, + {"create_primary_key_with_autogen_check.tmpl", CreatePrimaryKeyWithAutoGenCheckData{SchemaName: name, TableName: name, ConstraintName: name}}, + {"create_unique_constraint.tmpl", CreateUniqueConstraintData{SchemaName: name, TableName: name, ConstraintName: name}}, + {"set_sequence_value.tmpl", SetSequenceValueData{SchemaName: name, TableName: name, SequenceName: name, ColumnName: name}}, + {"audit_function.tmpl", AuditFunctionData{SchemaName: name, FunctionName: name, TableName: name, PrimaryKey: name, AuditSchema: name}}, + {"audit_trigger.tmpl", AuditTriggerData{SchemaName: name, TableName: name, TriggerName: name, FunctionName: name}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + out, err := te.execute(tc.name, tc.data) + if err != nil { + t.Fatal(err) + } + if strings.Count(out, "$relspec_2$") != 2 || !strings.Contains(out, quoteIdent(name)) || !strings.Contains(out, quote(name)) { + t.Fatalf("%s", out) + } + }) + } +} + +func TestLive_AdversarialNamesAndAudit(t *testing.T) { + connString := liveWriterConn(t) + base, conn := liveWriterSchema(t, connString) + ctx := context.Background() + schemaName := base + "'\"\\\n名$relspec_0$" + tableName := "select" + key := "Key'\"\\\n$relspec_0$" + valueName := "名'\"\\\n$body$" + comment := "Comment ' \\ $$ $relspec_0$\nUnicode 名" + t.Cleanup(func() { _, _ = conn.Exec(ctx, "DROP SCHEMA IF EXISTS "+quoteIdent(schemaName)+" CASCADE") }) + db := models.InitDatabase("test\nSELECT invalid_header;") + s := models.InitSchema(schemaName) + tbl := models.InitTable(tableName, schemaName) + id := models.InitColumn(key, tableName, schemaName) + id.Type = "integer" + id.IsPrimaryKey = true + id.NotNull = true + tbl.Columns[key] = id + value := models.InitColumn(valueName, tableName, schemaName) + value.Type = "text" + value.Default = quote(comment) + value.Description = comment + value.Comment = comment + tbl.Columns[valueName] = value + tbl.Description = comment + tbl.Comment = comment + tbl.Constraints["unique"] = &models.Constraint{Name: "unique'\"\\\n$relspec_0$", Type: models.UniqueConstraint, Columns: []string{valueName}} + tbl.Constraints["check"] = &models.Constraint{Name: "check'\"\\\n$relspec_0$", Type: models.CheckConstraint, Expression: quoteIdent(key) + ">0"} + tbl.Indexes["index"] = &models.Index{Name: "index'\"\\\n$relspec_0$", Type: "btree", Columns: []string{valueName}} + child := models.InitTable("from", schemaName) + child.Columns[key] = &models.Column{Name: key, Type: "integer"} + child.Constraints["fk"] = &models.Constraint{Name: "foreign'\"\\\n$relspec_0$", Type: models.ForeignKeyConstraint, Columns: []string{key}, ReferencedSchema: schemaName, ReferencedTable: tableName, ReferencedColumns: []string{key}} + s.Tables = []*models.Table{tbl, child} + db.Schemas = []*models.Schema{s} + w := NewWriter(&writers.WriterOptions{}) + var output bytes.Buffer + w.writer = &output + if err := w.WriteDatabase(db); err != nil { + t.Fatal(err) + } + if _, err := conn.Exec(ctx, output.String()); err != nil { + t.Fatalf("file SQL: %v\n%s", err, output.String()) + } + if _, err := conn.Exec(ctx, "SET standard_conforming_strings=off"); err != nil { + t.Fatal(err) + } + statements, err := w.GenerateDatabaseStatements(db) + if err != nil { + t.Fatal(err) + } + for _, stmt := range statements { + if _, err := conn.Exec(ctx, stmt); err != nil { + t.Fatalf("statement: %v\n%s", err, stmt) + } + } + target := w.qualTable(schemaName, tableName) + if _, err := conn.Exec(ctx, "INSERT INTO "+target+" ("+quoteIdent(key)+") VALUES (1)"); err != nil { + t.Fatal(err) + } + var got string + if err := conn.QueryRow(ctx, "SELECT "+quoteIdent(valueName)+" FROM "+target).Scan(&got); err != nil || got != comment { + t.Fatalf("default: %q %v", got, err) + } + if err := conn.QueryRow(ctx, `SELECT col_description(c.oid,a.attnum) FROM pg_class c JOIN pg_namespace n ON n.oid=c.relnamespace JOIN pg_attribute a ON a.attrelid=c.oid WHERE n.nspname=$1 AND c.relname=$2 AND a.attname=$3`, schemaName, tableName, valueName).Scan(&got); err != nil || got != comment { + t.Fatalf("comment: %q %v", got, err) + } + // Migration templates must preserve the same identifier and literal boundaries. + current := models.InitDatabase("current") + currentSchema := models.InitSchema(schemaName) + currentTable := models.InitTable(tableName, schemaName) + currentValue := *value + currentValue.Type = "text" + currentTable.Columns[valueName] = ¤tValue + currentID := *id + currentTable.Columns[key] = ¤tID + currentTable.Constraints = tbl.Constraints + currentTable.Indexes = tbl.Indexes + currentSchema.Tables = []*models.Table{currentTable, child} + current.Schemas = []*models.Schema{currentSchema} + value.Type = "varchar" + value.Length = 200 + migration, err := NewMigrationWriter(&writers.WriterOptions{}) + if err != nil { + t.Fatal(err) + } + var migrationOutput bytes.Buffer + migration.writer = &migrationOutput + if err := migration.WriteMigration(db, current); err != nil { + t.Fatal(err) + } + if _, err := conn.Exec(ctx, migrationOutput.String()); err != nil { + t.Fatalf("migration: %v\n%s", err, migrationOutput.String()) + } + if err := conn.QueryRow(ctx, "SELECT "+quoteIdent(valueName)+" FROM "+target+" /* after migration */").Scan(&got); err != nil || got != comment { + t.Fatalf("migration value: %q %v", got, err) + } + te := w.executor + sequenceName := "Seq'\"\\\n$relspec_0$" + sequenceSQL, err := te.ExecuteCreateSequence(CreateSequenceData{SchemaName: schemaName, SequenceName: sequenceName, Increment: 1, MinValue: 1, MaxValue: 1000, StartValue: 1, CacheSize: 1}) + if err != nil { + t.Fatal(err) + } + if _, err := conn.Exec(ctx, sequenceSQL); err != nil { + t.Fatal(err) + } + setvalSQL, err := te.ExecuteSetSequenceValue(SetSequenceValueData{SchemaName: schemaName, TableName: tableName, SequenceName: sequenceName, ColumnName: key}) + if err != nil { + t.Fatal(err) + } + if _, err := conn.Exec(ctx, setvalSQL); err != nil { + t.Fatalf("sequence: %v\n%s", err, setvalSQL) + } + var next int + if err := conn.QueryRow(ctx, "SELECT nextval($1::regclass)", w.qualTable(schemaName, sequenceName)).Scan(&next); err != nil || next != 2 { + t.Fatalf("sequence value: %d %v", next, err) + } + autoTable := w.qualTable(schemaName, "auto") + if _, err := conn.Exec(ctx, "CREATE TABLE "+autoTable+" (old integer PRIMARY KEY, new integer)"); err != nil { + t.Fatal(err) + } + pkSQL, err := te.ExecuteCreatePrimaryKeyWithAutoGenCheck(CreatePrimaryKeyWithAutoGenCheckData{SchemaName: schemaName, TableName: "auto", ConstraintName: "new'key", AutoGenNames: formatStringList([]string{"auto_pkey"}), Columns: quotedIdentifiers([]string{"new"}), ColumnNames: formatStringList([]string{"new"})}) + if err != nil { + t.Fatal(err) + } + if _, err := conn.Exec(ctx, pkSQL); err != nil { + t.Fatalf("primary key: %v\n%s", err, pkSQL) + } + + auditTables, err := te.ExecuteAuditTables(AuditTablesData{AuditSchema: schemaName}) + if err != nil { + t.Fatal(err) + } + if _, err := conn.Exec(ctx, auditTables); err != nil { + t.Fatal(err) + } + config := &TableAuditConfig{AuditInsert: true, AuditUpdate: true, AuditDelete: true, TablePrefix: comment} + functionData := BuildAuditFunctionData(schemaName, tbl, id, config, schemaName, "current_user") + function, err := te.ExecuteAuditFunction(functionData) + if err != nil { + t.Fatal(err) + } + if _, err := conn.Exec(ctx, function); err != nil { + t.Fatalf("audit function: %v\n%s", err, function) + } + trigger, err := te.ExecuteAuditTrigger(AuditTriggerData{SchemaName: schemaName, TableName: tableName, FunctionName: functionData.FunctionName, TriggerName: "trigger'\"\\\n$relspec_0$", Events: "INSERT OR UPDATE OR DELETE"}) + if err != nil { + t.Fatal(err) + } + if _, err := conn.Exec(ctx, trigger); err != nil { + t.Fatalf("audit trigger: %v\n%s", err, trigger) + } + for _, sql := range []string{"INSERT INTO " + target + " VALUES (2, 'second')", "UPDATE " + target + " SET " + quoteIdent(valueName) + "='third' WHERE " + quoteIdent(key) + "=2", "DELETE FROM " + target + " WHERE " + quoteIdent(key) + "=2"} { + if _, err := conn.Exec(ctx, sql); err != nil { + t.Fatal(err) + } + } + var count int + if err := conn.QueryRow(ctx, "SELECT count(*) FROM "+w.qualTable(schemaName, "atevent")+" WHERE tableprefix=$1", comment).Scan(&count); err != nil || count != 3 { + t.Fatalf("audit events: %d %v", count, err) + } +} diff --git a/pkg/writers/pgsql/serial_sequence_test.go b/pkg/writers/pgsql/serial_sequence_test.go index ad23d85..ebc83c4 100644 --- a/pkg/writers/pgsql/serial_sequence_test.go +++ b/pkg/writers/pgsql/serial_sequence_test.go @@ -110,7 +110,7 @@ func TestDiffStatements_ExistingTableNewSequenceIsSetPastData(t *testing.T) { t.Fatalf("diffStatements failed: %v", err) } out := diffJoin(stmts) - seqIdx := strings.Index(out, "CREATE SEQUENCE IF NOT EXISTS identity.login_event_id_login_event_seq") + seqIdx := strings.Index(out, "CREATE SEQUENCE IF NOT EXISTS \"identity\".\"login_event_id_login_event_seq\"") setvalIdx := strings.Index(out, "setval(") if seqIdx < 0 || setvalIdx < 0 || seqIdx > setvalIdx { t.Fatalf("expected sequence creation before setval, got:\n%s", out) diff --git a/pkg/writers/pgsql/statement_helpers_test.go b/pkg/writers/pgsql/statement_helpers_test.go index 7bc372b..c482063 100644 --- a/pkg/writers/pgsql/statement_helpers_test.go +++ b/pkg/writers/pgsql/statement_helpers_test.go @@ -131,7 +131,7 @@ func TestFirstBareIdentAndHelpers(t *testing.T) { "abc,def": "abc", "abc;": "abc", "\n abc\tdef": "abc", - `"a b" c`: `"a`, + `"a b" c`: `"a b"`, " tbl (x int)": "tbl", } for in, want := range bare { diff --git a/pkg/writers/pgsql/template_functions.go b/pkg/writers/pgsql/template_functions.go index ae54b0a..5a06b22 100644 --- a/pkg/writers/pgsql/template_functions.go +++ b/pkg/writers/pgsql/template_functions.go @@ -24,6 +24,7 @@ func TemplateFunctions() map[string]interface{} { "escape": escape, "safe_identifier": safeIdentifier, "quote_ident": quoteIdent, + "comment_text": commentText, // Type conversion "goTypeToSQL": goTypeToSQL, @@ -99,6 +100,9 @@ func indent(spaces int, text string) string { // quote quotes a string value for SQL (escapes single quotes) func quote(s string) string { + if strings.Contains(s, "\\") { + return sqlLiteral(s) + } return "'" + strings.ReplaceAll(s, "'", "''") + "'" } @@ -125,41 +129,15 @@ func safeIdentifier(s string) string { return strings.ToLower(safe) } -// quoteIdent quotes a PostgreSQL identifier if necessary -// Identifiers need quoting if they: -// - Start with a digit -// - Contain special characters -// - Are reserved keywords -// - Contain uppercase letters (to preserve case) -func quoteIdent(s string) string { - if s == "" { - return `""` +// quoteIdent always quotes an identifier, preserving case and reserved words. +func quoteIdent(s string) string { return `"` + strings.ReplaceAll(s, `"`, `""`) + `"` } + +func quotedIdentifiers(names []string) string { + out := make([]string, len(names)) + for i, name := range names { + out[i] = quoteIdent(name) } - - // Check if quoting is needed - needsQuoting := unicode.IsDigit(rune(s[0])) - - // Starts with digit - - // Contains uppercase letters or special characters - for _, r := range s { - if unicode.IsUpper(r) { - needsQuoting = true - break - } - if !unicode.IsLetter(r) && !unicode.IsDigit(r) && r != '_' { - needsQuoting = true - break - } - } - - if needsQuoting { - // Escape double quotes by doubling them - escaped := strings.ReplaceAll(s, `"`, `""`) - return `"` + escaped + `"` - } - - return s + return strings.Join(out, ", ") } // Type conversion functions diff --git a/pkg/writers/pgsql/template_functions_test.go b/pkg/writers/pgsql/template_functions_test.go index 656a401..ac1800b 100644 --- a/pkg/writers/pgsql/template_functions_test.go +++ b/pkg/writers/pgsql/template_functions_test.go @@ -106,7 +106,7 @@ func TestQuoteIdent(t *testing.T) { input string expected string }{ - {"valid_name", "valid_name"}, + {"valid_name", `"valid_name"`}, {"ValidName", `"ValidName"`}, {"123column", `"123column"`}, {"215162_fk_constraint", `"215162_fk_constraint"`}, @@ -114,8 +114,8 @@ func TestQuoteIdent(t *testing.T) { {"user@domain", `"user@domain"`}, {`"quoted"`, `"""quoted"""`}, {"", `""`}, - {"lowercase", "lowercase"}, - {"with_underscore", "with_underscore"}, + {"lowercase", `"lowercase"`}, + {"with_underscore", `"with_underscore"`}, } for _, tt := range tests { diff --git a/pkg/writers/pgsql/templates.go b/pkg/writers/pgsql/templates.go index a418913..50404cd 100644 --- a/pkg/writers/pgsql/templates.go +++ b/pkg/writers/pgsql/templates.go @@ -8,7 +8,6 @@ import ( "text/template" "git.warky.dev/wdevs/relspecgo/pkg/models" - "git.warky.dev/wdevs/relspecgo/pkg/writers" ) //go:embed templates/*.tmpl @@ -29,7 +28,7 @@ func NewTemplateExecutor(flattenSchema bool) (*TemplateExecutor, error) { } // qual_table returns a quoted, schema-qualified identifier. - // With flatten=false: "schema"."table" (or unquoted equivalents). + // With flatten=false: "schema"."table". // With flatten=true: "schema_table". funcMap["qual_table"] = func(schema, name string) string { if schema == "" { @@ -41,20 +40,20 @@ func NewTemplateExecutor(flattenSchema bool) (*TemplateExecutor, error) { return quoteIdent(schema) + "." + quoteIdent(name) } - // qual_table_raw is the same as qual_table but without identifier quoting. - funcMap["qual_table_raw"] = func(schema, name string) string { - if schema == "" { - return name - } - if flattenSchema { - return schema + "_" + name - } - return schema + "." + name - } - - tmpl, err := template.New("").Funcs(funcMap).ParseFS(templateFS, "templates/*.tmpl") + funcMap["dollar_delimiter"] = func() string { return "$$" } + tmpl := template.New("").Funcs(funcMap) + entries, err := templateFS.ReadDir("templates") if err != nil { - return nil, fmt.Errorf("failed to parse templates: %w", err) + return nil, err + } + for _, entry := range entries { + source, err := templateFS.ReadFile("templates/" + entry.Name()) + if err != nil { + return nil, err + } + if _, err := tmpl.New(entry.Name()).Parse(string(source)); err != nil { + return nil, fmt.Errorf("parse %s: %w", entry.Name(), err) + } } return &TemplateExecutor{ @@ -62,6 +61,31 @@ func NewTemplateExecutor(flattenSchema bool) (*TemplateExecutor, error) { }, nil } +// execute chooses a delimiter absent from rendered data, including SQL expressions. +// Only explicit template calls emit delimiters; user text is never rewritten. +func (te *TemplateExecutor) execute(name string, data any) (string, error) { + for n := 0; ; n++ { + delimiter := fmt.Sprintf("$relspec_%d$", n) + count := 0 + tmpl, err := te.templates.Clone() + if err != nil { + return "", err + } + tmpl.Funcs(template.FuncMap{"dollar_delimiter": func() string { count++; return delimiter }}) + var buf bytes.Buffer + if err := tmpl.ExecuteTemplate(&buf, name, data); err != nil { + return "", fmt.Errorf("execute %s: %w", name, err) + } + rendered := buf.String() + if strings.ContainsRune(rendered, 0) { + return "", fmt.Errorf("%s contains a NUL byte", name) + } + if strings.Count(rendered, delimiter) == count { + return rendered, nil + } + } +} + // Template data structures // CreateTableData contains data for create table template @@ -305,230 +329,117 @@ type CreatePrimaryKeyWithAutoGenCheckData struct { // ExecuteCreateTable executes the create table template func (te *TemplateExecutor) ExecuteCreateTable(data CreateTableData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "create_table.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute create_table template: %w", err) - } - return buf.String(), nil + return te.execute("create_table.tmpl", data) } // ExecuteAddColumn executes the add column template func (te *TemplateExecutor) ExecuteAddColumn(data AddColumnData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "add_column.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute add_column template: %w", err) - } - return buf.String(), nil + return te.execute("add_column.tmpl", data) } // ExecuteAlterColumnTypeWithCheck executes the guarded alter column type // template shared by the full-schema writer and the diff-based migration // writer. func (te *TemplateExecutor) ExecuteAlterColumnTypeWithCheck(data AlterColumnTypeWithCheckData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "alter_column_type_with_check.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute alter_column_type_with_check template: %w", err) - } - body := strings.TrimSpace(buf.String()) - body = strings.TrimPrefix(body, "DO $$") - body = strings.TrimSuffix(body, "$$;") - return dollarBlock(strings.TrimSpace(body), "relspec_alter_type"), nil + return te.execute("alter_column_type_with_check.tmpl", data) } // ExecuteAlterColumnDefaultWithCheck executes the guarded alter column // default template shared by the full-schema writer and the diff-based // migration writer. func (te *TemplateExecutor) ExecuteAlterColumnDefaultWithCheck(data AlterColumnDefaultWithCheckData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "alter_column_default_with_check.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute alter_column_default_with_check template: %w", err) - } - return buf.String(), nil + return te.execute("alter_column_default_with_check.tmpl", data) } // ExecuteAlterColumnNullabilityWithCheck executes the guarded alter column // nullability template. func (te *TemplateExecutor) ExecuteAlterColumnNullabilityWithCheck(data AlterColumnNullabilityWithCheckData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "alter_column_nullability_with_check.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute alter_column_nullability_with_check template: %w", err) - } - return buf.String(), nil + return te.execute("alter_column_nullability_with_check.tmpl", data) } // ExecuteCreatePrimaryKey executes the create primary key template func (te *TemplateExecutor) ExecuteCreatePrimaryKey(data CreatePrimaryKeyData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "create_primary_key.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute create_primary_key template: %w", err) - } - return buf.String(), nil + return te.execute("create_primary_key.tmpl", data) } // ExecuteCreateIndex executes the create index template func (te *TemplateExecutor) ExecuteCreateIndex(data CreateIndexData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "create_index.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute create_index template: %w", err) - } - return buf.String(), nil + return te.execute("create_index.tmpl", data) } // ExecuteCreateForeignKey executes the create foreign key template func (te *TemplateExecutor) ExecuteCreateForeignKey(data CreateForeignKeyData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "create_foreign_key.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute create_foreign_key template: %w", err) - } - return buf.String(), nil + return te.execute("create_foreign_key.tmpl", data) } // ExecuteDropConstraint executes the drop constraint template func (te *TemplateExecutor) ExecuteDropConstraint(data DropConstraintData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "drop_constraint.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute drop_constraint template: %w", err) - } - return buf.String(), nil + return te.execute("drop_constraint.tmpl", data) } // ExecuteDropIndex executes the drop index template func (te *TemplateExecutor) ExecuteDropIndex(data DropIndexData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "drop_index.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute drop_index template: %w", err) - } - return buf.String(), nil + return te.execute("drop_index.tmpl", data) } // ExecuteCommentTable executes the table comment template func (te *TemplateExecutor) ExecuteCommentTable(data CommentTableData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "comment_table.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute comment_table template: %w", err) - } - return buf.String(), nil + return te.execute("comment_table.tmpl", data) } // ExecuteCommentColumn executes the column comment template func (te *TemplateExecutor) ExecuteCommentColumn(data CommentColumnData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "comment_column.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute comment_column template: %w", err) - } - return buf.String(), nil + return te.execute("comment_column.tmpl", data) } // ExecuteAuditTables executes the audit tables template func (te *TemplateExecutor) ExecuteAuditTables(data AuditTablesData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "audit_tables.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute audit_tables template: %w", err) - } - return buf.String(), nil + return te.execute("audit_tables.tmpl", data) } // ExecuteAuditFunction executes the audit function template func (te *TemplateExecutor) ExecuteAuditFunction(data AuditFunctionData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "audit_function.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute audit_function template: %w", err) - } - return buf.String(), nil + return te.execute("audit_function.tmpl", data) } // ExecuteAuditTrigger executes the audit trigger template func (te *TemplateExecutor) ExecuteAuditTrigger(data AuditTriggerData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "audit_trigger.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute audit_trigger template: %w", err) - } - return buf.String(), nil + return te.execute("audit_trigger.tmpl", data) } // ExecuteCreateUniqueConstraint executes the create unique constraint template func (te *TemplateExecutor) ExecuteCreateUniqueConstraint(data CreateUniqueConstraintData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "create_unique_constraint.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute create_unique_constraint template: %w", err) - } - return buf.String(), nil + return te.execute("create_unique_constraint.tmpl", data) } // ExecuteCreateCheckConstraint executes the create check constraint template func (te *TemplateExecutor) ExecuteCreateCheckConstraint(data CreateCheckConstraintData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "create_check_constraint.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute create_check_constraint template: %w", err) - } - return buf.String(), nil + return te.execute("create_check_constraint.tmpl", data) } // ExecuteCreateForeignKeyWithCheck executes the create foreign key with check template func (te *TemplateExecutor) ExecuteCreateForeignKeyWithCheck(data CreateForeignKeyWithCheckData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "create_foreign_key_with_check.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute create_foreign_key_with_check template: %w", err) - } - return buf.String(), nil + return te.execute("create_foreign_key_with_check.tmpl", data) } // ExecuteSetSequenceValue executes the set sequence value template func (te *TemplateExecutor) ExecuteSetSequenceValue(data SetSequenceValueData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "set_sequence_value.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute set_sequence_value template: %w", err) - } - return buf.String(), nil + return te.execute("set_sequence_value.tmpl", data) } // ExecuteCreateSequence executes the create sequence template func (te *TemplateExecutor) ExecuteCreateSequence(data CreateSequenceData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "create_sequence.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute create_sequence template: %w", err) - } - return buf.String(), nil + return te.execute("create_sequence.tmpl", data) } // ExecuteAddColumnWithCheck executes the add column with check template func (te *TemplateExecutor) ExecuteAddColumnWithCheck(data AddColumnWithCheckData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "add_column_with_check.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute add_column_with_check template: %w", err) - } - return buf.String(), nil + return te.execute("add_column_with_check.tmpl", data) } // ExecuteCreatePrimaryKeyWithAutoGenCheck executes the create primary key with auto-generated key check template func (te *TemplateExecutor) ExecuteCreatePrimaryKeyWithAutoGenCheck(data CreatePrimaryKeyWithAutoGenCheckData) (string, error) { - var buf bytes.Buffer - err := te.templates.ExecuteTemplate(&buf, "create_primary_key_with_autogen_check.tmpl", data) - if err != nil { - return "", fmt.Errorf("failed to execute create_primary_key_with_autogen_check template: %w", err) - } - return buf.String(), nil + return te.execute("create_primary_key_with_autogen_check.tmpl", data) } // Helper functions to build template data from models @@ -561,7 +472,7 @@ func BuildCreateTableData(schemaName string, table *models.Table) CreateTableDat colData.Generated, colData.Identity = columnGenerationClauses(col) if col.Default != nil && colData.Generated == "" && colData.Identity == "" { if value, ok := col.Default.(string); ok { - colData.Default = writers.QuoteDefaultValue(stripBackticks(value), col.Type) + colData.Default = quoteDefaultValue(stripBackticks(value), col.Type) } else { colData.Default = fmt.Sprintf("%v", col.Default) } @@ -612,7 +523,7 @@ func BuildAuditFunctionData( updateComparisons := make([]string, 0) for _, col := range auditedColumns { updateComparisons = append(updateComparisons, - fmt.Sprintf("old.%s IS DISTINCT FROM new.%s", col.Name, col.Name)) + fmt.Sprintf("old.%s IS DISTINCT FROM new.%s", quoteIdent(col.Name), quoteIdent(col.Name))) } updateCondition := strings.Join(updateComparisons, " OR ") @@ -627,8 +538,8 @@ func BuildAuditFunctionData( } } - oldValue := fmt.Sprintf("old.%s::text", col.Name) - newValue := fmt.Sprintf("new.%s::text", col.Name) + oldValue := fmt.Sprintf("old.%s::text", quoteIdent(col.Name)) + newValue := fmt.Sprintf("new.%s::text", quoteIdent(col.Name)) if isEncrypted { oldValue = "'****************'" @@ -653,7 +564,7 @@ func BuildAuditFunctionData( } } - oldValue := fmt.Sprintf("old.%s::text", col.Name) + oldValue := fmt.Sprintf("old.%s::text", quoteIdent(col.Name)) if isEncrypted { oldValue = "'****************'" } @@ -666,7 +577,7 @@ func BuildAuditFunctionData( tablePrefix := "NULL" if config.TablePrefix != "" { - tablePrefix = fmt.Sprintf("'%s'", config.TablePrefix) + tablePrefix = quote(config.TablePrefix) } return AuditFunctionData{ diff --git a/pkg/writers/pgsql/templates/alter_column_default_with_check.tmpl b/pkg/writers/pgsql/templates/alter_column_default_with_check.tmpl index 27e977c..7e2a743 100644 --- a/pkg/writers/pgsql/templates/alter_column_default_with_check.tmpl +++ b/pkg/writers/pgsql/templates/alter_column_default_with_check.tmpl @@ -1,4 +1,4 @@ -DO $$ +DO {{dollar_delimiter}} DECLARE current_default text; BEGIN @@ -8,9 +8,9 @@ BEGIN JOIN pg_class t ON t.oid = a.attrelid JOIN pg_namespace n ON n.oid = t.relnamespace LEFT JOIN pg_attrdef d ON d.adrelid = a.attrelid AND d.adnum = a.attnum - WHERE n.nspname = '{{.SchemaName}}' - AND t.relname = '{{.TableName}}' - AND a.attname = '{{.ColumnName}}' + WHERE n.nspname = {{quote .SchemaName}} + AND t.relname = {{quote .TableName}} + AND a.attname = {{quote .ColumnName}} AND a.attnum > 0 AND NOT a.attisdropped; @@ -26,4 +26,4 @@ BEGIN END IF; {{- end }} END; -$$; +{{dollar_delimiter}}; diff --git a/pkg/writers/pgsql/templates/alter_column_nullability_with_check.tmpl b/pkg/writers/pgsql/templates/alter_column_nullability_with_check.tmpl index d560d6e..468ed7f 100644 --- a/pkg/writers/pgsql/templates/alter_column_nullability_with_check.tmpl +++ b/pkg/writers/pgsql/templates/alter_column_nullability_with_check.tmpl @@ -1,4 +1,4 @@ -DO $$ +DO {{dollar_delimiter}} DECLARE current_not_null boolean; BEGIN @@ -7,9 +7,9 @@ BEGIN FROM pg_attribute a JOIN pg_class t ON t.oid = a.attrelid JOIN pg_namespace n ON n.oid = t.relnamespace - WHERE n.nspname = '{{.SchemaName}}' - AND t.relname = '{{.TableName}}' - AND a.attname = '{{.ColumnName}}' + WHERE n.nspname = {{quote .SchemaName}} + AND t.relname = {{quote .TableName}} + AND a.attname = {{quote .ColumnName}} AND a.attnum > 0 AND NOT a.attisdropped; @@ -23,4 +23,4 @@ BEGIN {{- end }} END IF; END; -$$; +{{dollar_delimiter}}; diff --git a/pkg/writers/pgsql/templates/alter_column_type_with_check.tmpl b/pkg/writers/pgsql/templates/alter_column_type_with_check.tmpl index 61434a5..785b2a2 100644 --- a/pkg/writers/pgsql/templates/alter_column_type_with_check.tmpl +++ b/pkg/writers/pgsql/templates/alter_column_type_with_check.tmpl @@ -1,4 +1,4 @@ -DO $$ +DO {{dollar_delimiter}} DECLARE current_type text; BEGIN @@ -19,4 +19,4 @@ BEGIN ALTER COLUMN {{quote_ident .ColumnName}} TYPE {{.NewType}}{{if .UsingExpr}} USING {{.UsingExpr}}{{end}}; END IF; END; -$$; +{{dollar_delimiter}}; diff --git a/pkg/writers/pgsql/templates/audit_function.tmpl b/pkg/writers/pgsql/templates/audit_function.tmpl index 7954d69..f96f274 100644 --- a/pkg/writers/pgsql/templates/audit_function.tmpl +++ b/pkg/writers/pgsql/templates/audit_function.tmpl @@ -1,8 +1,8 @@ -CREATE OR REPLACE FUNCTION {{qual_table_raw .SchemaName .FunctionName}}() +CREATE OR REPLACE FUNCTION {{qual_table .SchemaName .FunctionName}}() RETURNS trigger AS -$body$ +{{dollar_delimiter}} DECLARE - m_funcname text = '{{.FunctionName}}'; + m_funcname text = {{quote .FunctionName}}; m_user text; m_atevent integer; BEGIN @@ -23,8 +23,8 @@ BEGIN {{- if .AuditInsert}} IF TG_OP = 'INSERT' THEN -- Record INSERT - INSERT INTO {{.AuditSchema}}.atevent (tablename, tableprefix, rid_parent, changeuser, changedate, changetime, actionx) - VALUES ('{{.TableName}}', {{.TablePrefix}}, new.{{.PrimaryKey}}, m_user, CURRENT_DATE, CURRENT_TIME, 1) + INSERT INTO {{quote_ident .AuditSchema}}.atevent (tablename, tableprefix, rid_parent, changeuser, changedate, changetime, actionx) + VALUES ({{quote .TableName}}, {{.TablePrefix}}, new.{{quote_ident .PrimaryKey}}, m_user, CURRENT_DATE, CURRENT_TIME, 1) RETURNING rid_atevent INTO m_atevent; {{- end}} @@ -32,15 +32,15 @@ BEGIN ELSIF TG_OP = 'UPDATE' THEN -- Check if any audited columns changed IF ({{.UpdateCondition}}) THEN - INSERT INTO {{.AuditSchema}}.atevent (tablename, tableprefix, rid_parent, changeuser, changedate, changetime, actionx) - VALUES ('{{.TableName}}', {{.TablePrefix}}, new.{{.PrimaryKey}}, m_user, CURRENT_DATE, CURRENT_TIME, 2) + INSERT INTO {{quote_ident .AuditSchema}}.atevent (tablename, tableprefix, rid_parent, changeuser, changedate, changetime, actionx) + VALUES ({{quote .TableName}}, {{.TablePrefix}}, new.{{quote_ident .PrimaryKey}}, m_user, CURRENT_DATE, CURRENT_TIME, 2) RETURNING rid_atevent INTO m_atevent; -- Record column changes {{- range .UpdateColumns}} - IF (old.{{.Name}} IS DISTINCT FROM new.{{.Name}}) THEN - INSERT INTO {{$.AuditSchema}}.atdetail(rid_atevent, datacolumn, changedfrom, changedto) - VALUES (m_atevent, '{{.Name}}', substr({{.OldValue}}, 1, 1000), substr({{.NewValue}}, 1, 1000)); + IF (old.{{quote_ident .Name}} IS DISTINCT FROM new.{{quote_ident .Name}}) THEN + INSERT INTO {{quote_ident $.AuditSchema}}.atdetail(rid_atevent, datacolumn, changedfrom, changedto) + VALUES (m_atevent, {{quote .Name}}, substr({{.OldValue}}, 1, 1000), substr({{.NewValue}}, 1, 1000)); END IF; {{- end}} END IF; @@ -49,14 +49,14 @@ BEGIN {{- if .AuditDelete}} ELSIF TG_OP = 'DELETE' THEN -- Record DELETE - INSERT INTO {{.AuditSchema}}.atevent (tablename, tableprefix, rid_parent, rid_deletedparent, changeuser, changedate, changetime, actionx) - VALUES ('{{.TableName}}', {{.TablePrefix}}, old.{{.PrimaryKey}}, old.{{.PrimaryKey}}, m_user, CURRENT_DATE, CURRENT_TIME, 3) + INSERT INTO {{quote_ident .AuditSchema}}.atevent (tablename, tableprefix, rid_parent, rid_deletedparent, changeuser, changedate, changetime, actionx) + VALUES ({{quote .TableName}}, {{.TablePrefix}}, old.{{quote_ident .PrimaryKey}}, old.{{quote_ident .PrimaryKey}}, m_user, CURRENT_DATE, CURRENT_TIME, 3) RETURNING rid_atevent INTO m_atevent; -- Record deleted column values {{- range .DeleteColumns}} - INSERT INTO {{$.AuditSchema}}.atdetail(rid_atevent, datacolumn, changedfrom, changedto) - VALUES (m_atevent, '{{.Name}}', substr({{.OldValue}}, 1, 1000), NULL); + INSERT INTO {{quote_ident $.AuditSchema}}.atdetail(rid_atevent, datacolumn, changedfrom, changedto) + VALUES (m_atevent, {{quote .Name}}, substr({{.OldValue}}, 1, 1000), NULL); {{- end}} {{- end}} END IF; @@ -76,9 +76,9 @@ EXCEPTION RAISE WARNING 'Audit function % failed: %', m_funcname, SQLERRM; RETURN NULL; END; -$body$ +{{dollar_delimiter}} LANGUAGE plpgsql VOLATILE SECURITY DEFINER; -COMMENT ON FUNCTION {{qual_table_raw .SchemaName .FunctionName}}() IS 'Audit trigger function for table {{qual_table_raw .SchemaName .TableName}}'; \ No newline at end of file +COMMENT ON FUNCTION {{qual_table .SchemaName .FunctionName}}() IS {{quote (printf "Audit trigger function for table %s" (qual_table .SchemaName .TableName))}}; \ No newline at end of file diff --git a/pkg/writers/pgsql/templates/audit_tables.tmpl b/pkg/writers/pgsql/templates/audit_tables.tmpl index ae3f2ca..04668ec 100644 --- a/pkg/writers/pgsql/templates/audit_tables.tmpl +++ b/pkg/writers/pgsql/templates/audit_tables.tmpl @@ -1,5 +1,5 @@ -- Audit Event Header Table -CREATE TABLE IF NOT EXISTS {{.AuditSchema}}.atevent ( +CREATE TABLE IF NOT EXISTS {{quote_ident .AuditSchema}}.atevent ( rid_atevent serial PRIMARY KEY, tablename text NOT NULL, tableprefix text, @@ -12,38 +12,38 @@ CREATE TABLE IF NOT EXISTS {{.AuditSchema}}.atevent ( CONSTRAINT ck_atevent_action CHECK (actionx IN (1, 2, 3)) ); -CREATE INDEX IF NOT EXISTS idx_atevent_tablename ON {{.AuditSchema}}.atevent(tablename); -CREATE INDEX IF NOT EXISTS idx_atevent_rid_parent ON {{.AuditSchema}}.atevent(rid_parent); -CREATE INDEX IF NOT EXISTS idx_atevent_changedate ON {{.AuditSchema}}.atevent(changedate); -CREATE INDEX IF NOT EXISTS idx_atevent_changeuser ON {{.AuditSchema}}.atevent(changeuser); +CREATE INDEX IF NOT EXISTS idx_atevent_tablename ON {{quote_ident .AuditSchema}}.atevent(tablename); +CREATE INDEX IF NOT EXISTS idx_atevent_rid_parent ON {{quote_ident .AuditSchema}}.atevent(rid_parent); +CREATE INDEX IF NOT EXISTS idx_atevent_changedate ON {{quote_ident .AuditSchema}}.atevent(changedate); +CREATE INDEX IF NOT EXISTS idx_atevent_changeuser ON {{quote_ident .AuditSchema}}.atevent(changeuser); -COMMENT ON TABLE {{.AuditSchema}}.atevent IS 'Audit trail header table - tracks all data changes'; -COMMENT ON COLUMN {{.AuditSchema}}.atevent.rid_atevent IS 'Audit event ID'; -COMMENT ON COLUMN {{.AuditSchema}}.atevent.tablename IS 'Name of the table that was modified'; -COMMENT ON COLUMN {{.AuditSchema}}.atevent.rid_parent IS 'Primary key value of the modified record'; -COMMENT ON COLUMN {{.AuditSchema}}.atevent.rid_deletedparent IS 'Parent reference for deleted records'; -COMMENT ON COLUMN {{.AuditSchema}}.atevent.changeuser IS 'User who made the change'; -COMMENT ON COLUMN {{.AuditSchema}}.atevent.changedate IS 'Date of change'; -COMMENT ON COLUMN {{.AuditSchema}}.atevent.changetime IS 'Time of change'; -COMMENT ON COLUMN {{.AuditSchema}}.atevent.actionx IS 'Action type: 1=INSERT, 2=UPDATE, 3=DELETE'; +COMMENT ON TABLE {{quote_ident .AuditSchema}}.atevent IS 'Audit trail header table - tracks all data changes'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atevent.rid_atevent IS 'Audit event ID'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atevent.tablename IS 'Name of the table that was modified'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atevent.rid_parent IS 'Primary key value of the modified record'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atevent.rid_deletedparent IS 'Parent reference for deleted records'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atevent.changeuser IS 'User who made the change'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atevent.changedate IS 'Date of change'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atevent.changetime IS 'Time of change'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atevent.actionx IS 'Action type: 1=INSERT, 2=UPDATE, 3=DELETE'; -- Audit Event Detail Table -CREATE TABLE IF NOT EXISTS {{.AuditSchema}}.atdetail ( +CREATE TABLE IF NOT EXISTS {{quote_ident .AuditSchema}}.atdetail ( rid_atdetail serial PRIMARY KEY, rid_atevent integer NOT NULL, datacolumn text NOT NULL, changedfrom text, changedto text, CONSTRAINT fk_atdetail_atevent FOREIGN KEY (rid_atevent) - REFERENCES {{.AuditSchema}}.atevent(rid_atevent) ON DELETE CASCADE + REFERENCES {{quote_ident .AuditSchema}}.atevent(rid_atevent) ON DELETE CASCADE ); -CREATE INDEX IF NOT EXISTS idx_atdetail_rid_atevent ON {{.AuditSchema}}.atdetail(rid_atevent); -CREATE INDEX IF NOT EXISTS idx_atdetail_datacolumn ON {{.AuditSchema}}.atdetail(datacolumn); +CREATE INDEX IF NOT EXISTS idx_atdetail_rid_atevent ON {{quote_ident .AuditSchema}}.atdetail(rid_atevent); +CREATE INDEX IF NOT EXISTS idx_atdetail_datacolumn ON {{quote_ident .AuditSchema}}.atdetail(datacolumn); -COMMENT ON TABLE {{.AuditSchema}}.atdetail IS 'Audit trail detail table - stores individual column changes'; -COMMENT ON COLUMN {{.AuditSchema}}.atdetail.rid_atdetail IS 'Audit detail ID'; -COMMENT ON COLUMN {{.AuditSchema}}.atdetail.rid_atevent IS 'Reference to audit event'; -COMMENT ON COLUMN {{.AuditSchema}}.atdetail.datacolumn IS 'Name of the column that changed'; -COMMENT ON COLUMN {{.AuditSchema}}.atdetail.changedfrom IS 'Old value before change'; -COMMENT ON COLUMN {{.AuditSchema}}.atdetail.changedto IS 'New value after change'; \ No newline at end of file +COMMENT ON TABLE {{quote_ident .AuditSchema}}.atdetail IS 'Audit trail detail table - stores individual column changes'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atdetail.rid_atdetail IS 'Audit detail ID'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atdetail.rid_atevent IS 'Reference to audit event'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atdetail.datacolumn IS 'Name of the column that changed'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atdetail.changedfrom IS 'Old value before change'; +COMMENT ON COLUMN {{quote_ident .AuditSchema}}.atdetail.changedto IS 'New value after change'; \ No newline at end of file diff --git a/pkg/writers/pgsql/templates/audit_trigger.tmpl b/pkg/writers/pgsql/templates/audit_trigger.tmpl index 119c020..7bec725 100644 --- a/pkg/writers/pgsql/templates/audit_trigger.tmpl +++ b/pkg/writers/pgsql/templates/audit_trigger.tmpl @@ -1,16 +1,16 @@ -DO $$ +DO {{dollar_delimiter}} BEGIN IF NOT EXISTS ( SELECT 1 FROM pg_trigger - WHERE tgname = '{{.TriggerName}}' - AND tgrelid = '{{qual_table_raw .SchemaName .TableName}}'::regclass + WHERE tgname = {{quote .TriggerName}} + AND tgrelid = {{quote (qual_table .SchemaName .TableName)}}::regclass ) THEN - CREATE TRIGGER {{.TriggerName}} + CREATE TRIGGER {{quote_ident .TriggerName}} AFTER {{.Events}} - ON {{qual_table_raw .SchemaName .TableName}} + ON {{qual_table .SchemaName .TableName}} FOR EACH ROW - EXECUTE FUNCTION {{qual_table_raw .SchemaName .FunctionName}}(); + EXECUTE FUNCTION {{qual_table .SchemaName .FunctionName}}(); END IF; END; -$$; \ No newline at end of file +{{dollar_delimiter}}; \ No newline at end of file diff --git a/pkg/writers/pgsql/templates/base_constraint.tmpl b/pkg/writers/pgsql/templates/base_constraint.tmpl index 97dbf07..f93ad06 100644 --- a/pkg/writers/pgsql/templates/base_constraint.tmpl +++ b/pkg/writers/pgsql/templates/base_constraint.tmpl @@ -1,39 +1,39 @@ {{/* Base constraint template */}} {{- define "constraint_base" -}} -ALTER TABLE {{qual_table_raw .SchemaName .TableName}} - ADD CONSTRAINT {{.ConstraintName}} +ALTER TABLE {{qual_table .SchemaName .TableName}} + ADD CONSTRAINT {{quote_ident .ConstraintName}} {{block "constraint_definition" .}}{{end}}; {{- end -}} {{/* Drop constraint with check */}} {{- define "drop_constraint_safe" -}} -DO $$ +DO {{dollar_delimiter}} BEGIN IF EXISTS ( SELECT 1 FROM information_schema.table_constraints - WHERE table_schema = '{{.SchemaName}}' - AND table_name = '{{.TableName}}' - AND constraint_name = '{{.ConstraintName}}' + WHERE table_schema = {{quote .SchemaName}} + AND table_name = {{quote .TableName}} + AND constraint_name = {{quote .ConstraintName}} ) THEN - ALTER TABLE {{qual_table_raw .SchemaName .TableName}} - DROP CONSTRAINT {{.ConstraintName}}; + ALTER TABLE {{qual_table .SchemaName .TableName}} + DROP CONSTRAINT {{quote_ident .ConstraintName}}; END IF; END; -$$; +{{dollar_delimiter}}; {{- end -}} {{/* Add constraint with existence check */}} {{- define "add_constraint_safe" -}} -DO $$ +DO {{dollar_delimiter}} BEGIN IF NOT EXISTS ( SELECT 1 FROM information_schema.table_constraints - WHERE table_schema = '{{.SchemaName}}' - AND table_name = '{{.TableName}}' - AND constraint_name = '{{.ConstraintName}}' + WHERE table_schema = {{quote .SchemaName}} + AND table_name = {{quote .TableName}} + AND constraint_name = {{quote .ConstraintName}} ) THEN {{template "constraint_base" .}} END IF; END; -$$; +{{dollar_delimiter}}; {{- end -}} \ No newline at end of file diff --git a/pkg/writers/pgsql/templates/base_ddl.tmpl b/pkg/writers/pgsql/templates/base_ddl.tmpl index c5d5014..1cd48c0 100644 --- a/pkg/writers/pgsql/templates/base_ddl.tmpl +++ b/pkg/writers/pgsql/templates/base_ddl.tmpl @@ -1,23 +1,23 @@ {{/* Base DDL template with common structure */}} {{- define "ddl_header" -}} --- DDL Operation: {{.Operation}} --- Schema: {{.Schema}} --- Object: {{.ObjectName}} +-- DDL Operation: {{comment_text .Operation}} +-- Schema: {{comment_text .Schema}} +-- Object: {{comment_text .ObjectName}} {{- end -}} {{- define "ddl_footer" -}} --- End of {{.Operation}} +-- End of {{comment_text .Operation}} {{- end -}} {{/* Base ALTER TABLE structure */}} {{- define "alter_table_base" -}} -ALTER TABLE {{qual_table_raw .SchemaName .TableName}} +ALTER TABLE {{qual_table .SchemaName .TableName}} {{block "alter_operation" .}}{{end}}; {{- end -}} {{/* Common existence check pattern */}} {{- define "exists_check" -}} -DO $$ +DO {{dollar_delimiter}} BEGIN IF NOT EXISTS ( {{block "exists_query" .}}{{end}} @@ -25,10 +25,10 @@ BEGIN {{block "create_statement" .}}{{end}} END IF; END; -$$; +{{dollar_delimiter}}; {{- end -}} {{/* Common drop pattern */}} {{- define "drop_if_exists" -}} -{{block "drop_type" .}}{{end}} IF EXISTS {{qual_table_raw .SchemaName .ObjectName}}; +{{block "drop_type" .}}{{end}} IF EXISTS {{qual_table .SchemaName .ObjectName}}; {{- end -}} \ No newline at end of file diff --git a/pkg/writers/pgsql/templates/comment_column.tmpl b/pkg/writers/pgsql/templates/comment_column.tmpl index 8ac61ac..467dd72 100644 --- a/pkg/writers/pgsql/templates/comment_column.tmpl +++ b/pkg/writers/pgsql/templates/comment_column.tmpl @@ -1 +1 @@ -COMMENT ON COLUMN {{qual_table .SchemaName .TableName}}.{{quote_ident .ColumnName}} IS '{{.Comment}}'; \ No newline at end of file +COMMENT ON COLUMN {{qual_table .SchemaName .TableName}}.{{quote_ident .ColumnName}} IS {{quote .Comment}}; \ No newline at end of file diff --git a/pkg/writers/pgsql/templates/comment_table.tmpl b/pkg/writers/pgsql/templates/comment_table.tmpl index 816002e..dad62c6 100644 --- a/pkg/writers/pgsql/templates/comment_table.tmpl +++ b/pkg/writers/pgsql/templates/comment_table.tmpl @@ -1 +1 @@ -COMMENT ON TABLE {{qual_table .SchemaName .TableName}} IS '{{.Comment}}'; \ No newline at end of file +COMMENT ON TABLE {{qual_table .SchemaName .TableName}} IS {{quote .Comment}}; \ No newline at end of file diff --git a/pkg/writers/pgsql/templates/create_check_constraint.tmpl b/pkg/writers/pgsql/templates/create_check_constraint.tmpl index dd86186..3963ce1 100644 --- a/pkg/writers/pgsql/templates/create_check_constraint.tmpl +++ b/pkg/writers/pgsql/templates/create_check_constraint.tmpl @@ -1,12 +1,12 @@ -DO $$ +DO {{dollar_delimiter}} BEGIN IF NOT EXISTS ( SELECT 1 FROM information_schema.table_constraints - WHERE table_schema = '{{.SchemaName}}' - AND table_name = '{{.TableName}}' - AND constraint_name = '{{.ConstraintName}}' + WHERE table_schema = {{quote .SchemaName}} + AND table_name = {{quote .TableName}} + AND constraint_name = {{quote .ConstraintName}} ) THEN ALTER TABLE {{qual_table .SchemaName .TableName}} ADD CONSTRAINT {{quote_ident .ConstraintName}} CHECK ({{.Expression}}); END IF; END; -$$; \ No newline at end of file +{{dollar_delimiter}}; \ No newline at end of file diff --git a/pkg/writers/pgsql/templates/create_foreign_key_with_check.tmpl b/pkg/writers/pgsql/templates/create_foreign_key_with_check.tmpl index 20c1e30..67c5104 100644 --- a/pkg/writers/pgsql/templates/create_foreign_key_with_check.tmpl +++ b/pkg/writers/pgsql/templates/create_foreign_key_with_check.tmpl @@ -1,10 +1,10 @@ -DO $$ +DO {{dollar_delimiter}} BEGIN IF NOT EXISTS ( SELECT 1 FROM information_schema.table_constraints - WHERE table_schema = '{{.SchemaName}}' - AND table_name = '{{.TableName}}' - AND constraint_name = '{{.ConstraintName}}' + WHERE table_schema = {{quote .SchemaName}} + AND table_name = {{quote .TableName}} + AND constraint_name = {{quote .ConstraintName}} ) THEN ALTER TABLE {{qual_table .SchemaName .TableName}} ADD CONSTRAINT {{quote_ident .ConstraintName}} @@ -15,4 +15,4 @@ BEGIN {{.Deferral}}{{end}}; END IF; END; -$$; \ No newline at end of file +{{dollar_delimiter}}; \ No newline at end of file diff --git a/pkg/writers/pgsql/templates/create_primary_key.tmpl b/pkg/writers/pgsql/templates/create_primary_key.tmpl index 864e345..9dc6838 100644 --- a/pkg/writers/pgsql/templates/create_primary_key.tmpl +++ b/pkg/writers/pgsql/templates/create_primary_key.tmpl @@ -1,13 +1,13 @@ -DO $$ +DO {{dollar_delimiter}} BEGIN IF NOT EXISTS ( SELECT 1 FROM information_schema.table_constraints - WHERE table_schema = '{{.SchemaName}}' - AND table_name = '{{.TableName}}' - AND constraint_name = '{{.ConstraintName}}' + WHERE table_schema = {{quote .SchemaName}} + AND table_name = {{quote .TableName}} + AND constraint_name = {{quote .ConstraintName}} ) THEN ALTER TABLE {{qual_table .SchemaName .TableName}} ADD CONSTRAINT {{quote_ident .ConstraintName}} PRIMARY KEY ({{.Columns}}); END IF; END; -$$; \ No newline at end of file +{{dollar_delimiter}}; \ No newline at end of file diff --git a/pkg/writers/pgsql/templates/create_primary_key_with_autogen_check.tmpl b/pkg/writers/pgsql/templates/create_primary_key_with_autogen_check.tmpl index 4f1564a..ed07f2e 100644 --- a/pkg/writers/pgsql/templates/create_primary_key_with_autogen_check.tmpl +++ b/pkg/writers/pgsql/templates/create_primary_key_with_autogen_check.tmpl @@ -1,4 +1,4 @@ -DO $$ +DO {{dollar_delimiter}} DECLARE current_pk_name text; current_pk_matches boolean := false; @@ -16,22 +16,22 @@ BEGIN ON a.attrelid = t.oid AND a.attnum = cols.attnum WHERE c.contype = 'p' - AND n.nspname = '{{.SchemaName}}' - AND t.relname = '{{.TableName}}' + AND n.nspname = {{quote .SchemaName}} + AND t.relname = {{quote .TableName}} ORDER BY cols.ord ), ARRAY[]::text[] ) = ARRAY[{{.ColumnNames}}] INTO current_pk_name, current_pk_matches FROM information_schema.table_constraints tc - WHERE tc.table_schema = '{{.SchemaName}}' - AND tc.table_name = '{{.TableName}}' + WHERE tc.table_schema = {{quote .SchemaName}} + AND tc.table_name = {{quote .TableName}} AND tc.constraint_type = 'PRIMARY KEY'; IF current_pk_name IS NOT NULL AND NOT current_pk_matches AND current_pk_name IN ({{.AutoGenNames}}) THEN - EXECUTE 'ALTER TABLE {{qual_table .SchemaName .TableName}} DROP CONSTRAINT ' || quote_ident(current_pk_name) || ' CASCADE'; + EXECUTE {{quote (printf "ALTER TABLE %s DROP CONSTRAINT " (qual_table .SchemaName .TableName))}} || quote_ident(current_pk_name) || ' CASCADE'; END IF; -- Add the desired primary key only when no matching primary key already exists. @@ -40,4 +40,4 @@ BEGIN ALTER TABLE {{qual_table .SchemaName .TableName}} ADD CONSTRAINT {{quote_ident .ConstraintName}} PRIMARY KEY ({{.Columns}}); END IF; END; -$$; +{{dollar_delimiter}}; diff --git a/pkg/writers/pgsql/templates/create_unique_constraint.tmpl b/pkg/writers/pgsql/templates/create_unique_constraint.tmpl index 7521fc2..ff6fc49 100644 --- a/pkg/writers/pgsql/templates/create_unique_constraint.tmpl +++ b/pkg/writers/pgsql/templates/create_unique_constraint.tmpl @@ -1,12 +1,12 @@ -DO $$ +DO {{dollar_delimiter}} BEGIN IF NOT EXISTS ( SELECT 1 FROM information_schema.table_constraints - WHERE table_schema = '{{.SchemaName}}' - AND table_name = '{{.TableName}}' - AND constraint_name = '{{.ConstraintName}}' + WHERE table_schema = {{quote .SchemaName}} + AND table_name = {{quote .TableName}} + AND constraint_name = {{quote .ConstraintName}} ) THEN ALTER TABLE {{qual_table .SchemaName .TableName}} ADD CONSTRAINT {{quote_ident .ConstraintName}} UNIQUE {{if .UsingIndex}}USING INDEX {{quote_ident .UsingIndex}}{{else}}({{.Columns}}){{end}}; END IF; END; -$$; +{{dollar_delimiter}}; diff --git a/pkg/writers/pgsql/templates/fragments.tmpl b/pkg/writers/pgsql/templates/fragments.tmpl index f755711..4d1f8e6 100644 --- a/pkg/writers/pgsql/templates/fragments.tmpl +++ b/pkg/writers/pgsql/templates/fragments.tmpl @@ -2,7 +2,7 @@ {{/* Column definition fragment */}} {{- define "column_definition" -}} -{{.Name}} {{.Type}} +{{quote_ident .Name}} {{.Type}} {{- if .Generated}} GENERATED ALWAYS AS ({{.Generated}}) STORED{{end}} {{- if .Identity}} GENERATED {{.Identity}} AS IDENTITY{{end}} {{- if and .Default (not .Generated) (not .Identity)}} DEFAULT {{.Default}}{{end}} @@ -18,7 +18,7 @@ {{/* Qualified table name */}} {{- define "qualified_table" -}} -{{qual_table_raw .SchemaName .TableName}} +{{qual_table .SchemaName .TableName}} {{- end -}} {{/* Index method clause */}} diff --git a/pkg/writers/pgsql/templates/set_sequence_value.tmpl b/pkg/writers/pgsql/templates/set_sequence_value.tmpl index aaee5a0..541a321 100644 --- a/pkg/writers/pgsql/templates/set_sequence_value.tmpl +++ b/pkg/writers/pgsql/templates/set_sequence_value.tmpl @@ -1,4 +1,4 @@ -DO $$ +DO {{dollar_delimiter}} DECLARE m_cnt bigint; m_next bigint; @@ -6,8 +6,8 @@ BEGIN IF EXISTS ( SELECT 1 FROM pg_class c INNER JOIN pg_namespace n ON n.oid = c.relnamespace - WHERE c.relname = '{{.SequenceName}}' - AND n.nspname = '{{.SchemaName}}' + WHERE c.relname = {{quote .SequenceName}} + AND n.nspname = {{quote .SchemaName}} AND c.relkind = 'S' ) THEN SELECT COALESCE(MAX({{quote_ident .ColumnName}}), 0) + 1 @@ -20,8 +20,8 @@ BEGIN -- Only move the sequence forward; never rewind one that is already past the data. IF m_cnt > m_next THEN - PERFORM setval('{{qual_table_raw .SchemaName .SequenceName}}'::regclass, m_cnt, false); + PERFORM setval({{quote (qual_table .SchemaName .SequenceName)}}::regclass, m_cnt, false); END IF; END IF; END; -$$; \ No newline at end of file +{{dollar_delimiter}}; \ No newline at end of file diff --git a/pkg/writers/pgsql/type_conversion_test.go b/pkg/writers/pgsql/type_conversion_test.go index 5af2e2d..0af1c0d 100644 --- a/pkg/writers/pgsql/type_conversion_test.go +++ b/pkg/writers/pgsql/type_conversion_test.go @@ -20,7 +20,7 @@ func TestConversionMetadataValidationAndBlockDelimiters(t *testing.T) { delete(col.Metadata, key) } } - col.Metadata["pgsql_using"] = "length('$relspec_alter_type_0$ $relspec_preflight_0$')" + col.Metadata["pgsql_using"] = "length('$relspec_0$ $relspec_preflight_0$')" data, err := conversionData("public", "accounts", col) if err != nil { t.Fatal(err) @@ -33,7 +33,7 @@ func TestConversionMetadataValidationAndBlockDelimiters(t *testing.T) { if err != nil { t.Fatal(err) } - if !strings.HasPrefix(alter, "DO $relspec_alter_type_1$") { + if !strings.HasPrefix(alter, "DO $relspec_1$") { t.Fatalf("unsafe alter delimiter: %s", alter) } preflight := preflightColumnType(data, false) diff --git a/pkg/writers/pgsql/writer.go b/pkg/writers/pgsql/writer.go index 9d16451..8bc584e 100644 --- a/pkg/writers/pgsql/writer.go +++ b/pkg/writers/pgsql/writer.go @@ -37,6 +37,9 @@ type ExecutionError = writers.ExecutionError // NewWriter creates a new PostgreSQL SQL writer func NewWriter(options *writers.WriterOptions) *Writer { + if options == nil { + options = &writers.WriterOptions{} + } executor, _ := NewTemplateExecutor(options.FlattenSchema) return &Writer{ options: options, @@ -46,11 +49,22 @@ func NewWriter(options *writers.WriterOptions) *Writer { // qualTable returns a schema-qualified name using the writer's FlattenSchema setting. func (w *Writer) qualTable(schema, name string) string { - return writers.QualifiedTableName(schema, name, w.options.FlattenSchema) + if schema == "" { + return quoteIdent(name) + } + if w.options.FlattenSchema { + return quoteIdent(schema + "_" + name) + } + return quoteIdent(schema) + "." + quoteIdent(name) } // WriteDatabase writes the entire database schema as SQL func (w *Writer) WriteDatabase(db *models.Database) error { + var preparationErr error + db, preparationErr = prepareModel(db, w.options.LowercaseIdentifiers) + if preparationErr != nil { + return preparationErr + } // Check if we should execute SQL directly on a database if connString, ok := w.options.Metadata["connection_string"].(string); ok && connString != "" { return w.executeDatabaseSQL(db, connString) @@ -79,7 +93,7 @@ func (w *Writer) WriteDatabase(db *models.Database) error { // Write header comment fmt.Fprintf(w.writer, "-- PostgreSQL Database Schema\n") - fmt.Fprintf(w.writer, "-- Database: %s\n", db.Name) + fmt.Fprintf(w.writer, "-- Database: %s\n", commentText(db.Name)) fmt.Fprintf(w.writer, "-- Generated by RelSpec\n") if w.options.ContinueOnError { fmt.Fprintf(w.writer, "\\set ON_ERROR_STOP off\n") @@ -110,11 +124,16 @@ func (w *Writer) WriteDatabase(db *models.Database) error { // GenerateDatabaseStatements generates SQL statements as a list for the entire database // Returns a slice of SQL statements that can be executed independently func (w *Writer) GenerateDatabaseStatements(db *models.Database) ([]string, error) { + var preparationErr error + db, preparationErr = prepareModel(db, w.options.LowercaseIdentifiers) + if preparationErr != nil { + return nil, preparationErr + } statements := []string{} // Add header comment statements = append(statements, "-- PostgreSQL Database Schema") - statements = append(statements, fmt.Sprintf("-- Database: %s", db.Name)) + statements = append(statements, fmt.Sprintf("-- Database: %s", commentText(db.Name))) statements = append(statements, "-- Generated by RelSpec") for _, schema := range db.Schemas { @@ -161,7 +180,7 @@ func (w *Writer) primaryKeySequenceStatement(schema *models.Schema, table *model } return fmt.Sprintf("CREATE SEQUENCE IF NOT EXISTS %s\n INCREMENT 1\n MINVALUE 1\n MAXVALUE 9223372036854775807\n START 1\n CACHE 1", - w.qualTable(schema.SQLName(), seqName)) + w.qualTable(schema.Name, seqName)) } // primaryKeySetvalStatement returns the statement that moves a table's primary key @@ -182,6 +201,11 @@ func (w *Writer) primaryKeySetvalStatement(schema *models.Schema, table *models. // GenerateSchemaStatements generates SQL statements as a list for a single schema func (w *Writer) GenerateSchemaStatements(schema *models.Schema) ([]string, error) { + var err error + schema, err = w.prepareSchema(schema) + if err != nil { + return nil, err + } preflights, err := w.generateTypePreflights(schema) if err != nil { return nil, err @@ -199,8 +223,8 @@ func (w *Writer) generateSchemaStatements(schema *models.Schema) ([]string, erro // Phase 1: Create schema (skip entirely when flattening) if schema.Name != "public" && !w.options.FlattenSchema { - statements = append(statements, fmt.Sprintf("-- Schema: %s", schema.Name)) - statements = append(statements, fmt.Sprintf("CREATE SCHEMA IF NOT EXISTS %s", schema.SQLName())) + statements = append(statements, fmt.Sprintf("-- Schema: %s", commentText(schema.Name))) + statements = append(statements, fmt.Sprintf("CREATE SCHEMA IF NOT EXISTS %s", quoteIdent(schema.Name))) } for _, extension := range requiredExtensions(schema) { @@ -258,14 +282,14 @@ func (w *Writer) generateSchemaStatements(schema *models.Schema) ([]string, erro pkCols := []string{} for _, col := range table.Columns { if col.IsPrimaryKey { - pkCols = append(pkCols, col.SQLName()) + pkCols = append(pkCols, col.Name) } } if len(pkCols) > 0 { // Sort for consistent output sort.Strings(pkCols) pkColumns = pkCols - pkName = fmt.Sprintf("pk_%s_%s", schema.SQLName(), table.SQLName()) + pkName = fmt.Sprintf("pk_%s_%s", schema.Name, table.Name) } } @@ -282,7 +306,7 @@ func (w *Writer) generateSchemaStatements(schema *models.Schema) ([]string, erro TableName: table.Name, ConstraintName: pkName, AutoGenNames: formatStringList(autoGenPKNames), - Columns: strings.Join(pkColumns, ", "), + Columns: quotedIdentifiers(pkColumns), ColumnNames: formatStringList(pkColumns), } @@ -322,7 +346,7 @@ func (w *Writer) generateSchemaStatements(schema *models.Schema) ([]string, erro tablespaceClause := "" if ts := pgIndexDirectiveTablespace(index); ts != "" { - tablespaceClause = fmt.Sprintf(" TABLESPACE %s", ts) + tablespaceClause = fmt.Sprintf(" TABLESPACE %s", quoteIdent(ts)) } whereClause := "" @@ -331,7 +355,7 @@ func (w *Writer) generateSchemaStatements(schema *models.Schema) ([]string, erro } stmt := fmt.Sprintf("CREATE %sINDEX IF NOT EXISTS %s ON %s USING %s (%s)%s%s%s", - uniqueStr, quoteIdentifier(index.Name), w.qualTable(schema.SQLName(), table.SQLName()), indexType, strings.Join(columnExprs, ", "), withClause, tablespaceClause, whereClause) + uniqueStr, quoteIdentifier(index.Name), w.qualTable(schema.Name, table.Name), indexType, strings.Join(columnExprs, ", "), withClause, tablespaceClause, whereClause) statements = append(statements, stmt) } } @@ -348,7 +372,7 @@ func (w *Writer) generateSchemaStatements(schema *models.Schema) ([]string, erro SchemaName: schema.Name, TableName: table.Name, ConstraintName: constraint.Name, - Columns: strings.Join(constraint.Columns, ", "), + Columns: quotedIdentifiers(constraint.Columns), UsingIndex: adoptableUniqueIndex(table, constraint), } @@ -410,10 +434,10 @@ func (w *Writer) generateSchemaStatements(schema *models.Schema) ([]string, erro SchemaName: schema.Name, TableName: table.Name, ConstraintName: constraint.Name, - SourceColumns: strings.Join(constraint.Columns, ", "), + SourceColumns: quotedIdentifiers(constraint.Columns), TargetSchema: refSchema, TargetTable: constraint.ReferencedTable, - TargetColumns: strings.Join(constraint.ReferencedColumns, ", "), + TargetColumns: quotedIdentifiers(constraint.ReferencedColumns), OnDelete: onDelete, OnUpdate: onUpdate, Deferral: constraint.DeferralClause(), @@ -441,15 +465,15 @@ func (w *Writer) generateSchemaStatements(schema *models.Schema) ([]string, erro // Phase 7: Comments for _, table := range schema.Tables { if table.Comment != "" { - stmt := fmt.Sprintf("COMMENT ON TABLE %s IS '%s'", - w.qualTable(schema.SQLName(), table.SQLName()), escapeQuote(table.Comment)) + stmt := fmt.Sprintf("COMMENT ON TABLE %s IS %s", + w.qualTable(schema.Name, table.Name), quote(table.Comment)) statements = append(statements, stmt) } for _, column := range sortColumns(table.Columns) { if column.Comment != "" { - stmt := fmt.Sprintf("COMMENT ON COLUMN %s.%s IS '%s'", - w.qualTable(schema.SQLName(), table.SQLName()), column.SQLName(), escapeQuote(column.Comment)) + stmt := fmt.Sprintf("COMMENT ON COLUMN %s.%s IS %s", + w.qualTable(schema.Name, table.Name), quoteIdent(column.Name), quote(column.Comment)) statements = append(statements, stmt) } } @@ -461,9 +485,14 @@ func (w *Writer) generateSchemaStatements(schema *models.Schema) ([]string, erro // GenerateAddColumnStatements generates ALTER TABLE ADD COLUMN statements for existing tables // This is useful for schema evolution when new columns are added to existing tables func (w *Writer) GenerateAddColumnStatements(schema *models.Schema) ([]string, error) { + var err error + schema, err = w.prepareSchema(schema) + if err != nil { + return nil, err + } statements := []string{} - statements = append(statements, fmt.Sprintf("-- Add missing columns for schema: %s", schema.Name)) + statements = append(statements, fmt.Sprintf("-- Add missing columns for schema: %s", commentText(schema.Name))) for _, table := range schema.Tables { // Sort columns by sequence or name for consistent output @@ -501,9 +530,14 @@ func (w *Writer) GenerateAddColumnStatements(schema *models.Schema) ([]string, e } func (w *Writer) GenerateAlterColumnTypeStatements(schema *models.Schema) ([]string, error) { + var err error + schema, err = w.prepareSchema(schema) + if err != nil { + return nil, err + } statements := []string{} - statements = append(statements, fmt.Sprintf("-- Alter column types for schema: %s", schema.Name)) + statements = append(statements, fmt.Sprintf("-- Alter column types for schema: %s", commentText(schema.Name))) for _, table := range schema.Tables { columns := getSortedColumns(table.Columns) @@ -533,9 +567,14 @@ func (w *Writer) GenerateAlterColumnTypeStatements(schema *models.Schema) ([]str // statements to bring existing columns' DEFAULT clause in line with the // model, safe to run against a database that already has the columns. func (w *Writer) GenerateAlterColumnDefaultStatements(schema *models.Schema) ([]string, error) { + var err error + schema, err = w.prepareSchema(schema) + if err != nil { + return nil, err + } statements := []string{} - statements = append(statements, fmt.Sprintf("-- Alter column defaults for schema: %s", schema.Name)) + statements = append(statements, fmt.Sprintf("-- Alter column defaults for schema: %s", commentText(schema.Name))) for _, table := range schema.Tables { columns := getSortedColumns(table.Columns) @@ -592,7 +631,7 @@ func formatColumnDefaultSQL(col *models.Column) (setDefault bool, defaultVal str return false, "" } if value, ok := col.Default.(string); ok { - return true, writers.QuoteDefaultValue(stripBackticks(value), col.Type) + return true, quoteDefaultValue(stripBackticks(value), col.Type) } return true, fmt.Sprintf("%v", col.Default) } @@ -601,9 +640,14 @@ func formatColumnDefaultSQL(col *models.Column) (setDefault bool, defaultVal str // statements to bring existing columns' NOT NULL state in line with the // model, safe to run against a database that already has the columns. func (w *Writer) GenerateAlterColumnNullabilityStatements(schema *models.Schema) ([]string, error) { + var err error + schema, err = w.prepareSchema(schema) + if err != nil { + return nil, err + } statements := []string{} - statements = append(statements, fmt.Sprintf("-- Alter column nullability for schema: %s", schema.Name)) + statements = append(statements, fmt.Sprintf("-- Alter column nullability for schema: %s", commentText(schema.Name))) for _, table := range schema.Tables { columns := getSortedColumns(table.Columns) @@ -626,10 +670,15 @@ func (w *Writer) GenerateAlterColumnNullabilityStatements(schema *models.Schema) // GenerateAddColumnsForDatabase generates ALTER TABLE ADD COLUMN statements for the entire database func (w *Writer) GenerateAddColumnsForDatabase(db *models.Database) ([]string, error) { + prepared, preparationErr := prepareModel(db, w.options.LowercaseIdentifiers) + if preparationErr != nil { + return nil, preparationErr + } + db = prepared statements := []string{} statements = append(statements, "-- Add missing columns to existing tables") - statements = append(statements, fmt.Sprintf("-- Database: %s", db.Name)) + statements = append(statements, fmt.Sprintf("-- Database: %s", commentText(db.Name))) statements = append(statements, "-- Generated by RelSpec") for _, schema := range db.Schemas { @@ -666,7 +715,7 @@ func (w *Writer) generateCreateTableStatement(schema *models.Schema, table *mode } stmt := fmt.Sprintf("CREATE TABLE IF NOT EXISTS %s (\n%s\n)%s", - w.qualTable(schema.SQLName(), table.SQLName()), strings.Join(columnDefs, ",\n"), + w.qualTable(schema.Name, table.Name), strings.Join(columnDefs, ",\n"), pgTableDirectiveSuffix(table)) statements = append(statements, stmt) @@ -675,7 +724,7 @@ func (w *Writer) generateCreateTableStatement(schema *models.Schema, table *mode // generateColumnDefinition generates column definition func (w *Writer) generateColumnDefinition(col *models.Column) string { - parts := []string{col.SQLName()} + parts := []string{quoteIdent(col.Name)} parts = append(parts, effectiveColumnSQLType(col)) @@ -700,7 +749,7 @@ func (w *Writer) generateColumnDefinition(col *models.Column) string { if !col.Generated && !col.Identity && col.Default != nil { switch v := col.Default.(type) { case string: - parts = append(parts, fmt.Sprintf("DEFAULT %s", writers.QuoteDefaultValue(stripBackticks(v), col.Type))) + parts = append(parts, fmt.Sprintf("DEFAULT %s", quoteDefaultValue(stripBackticks(v), col.Type))) case bool: parts = append(parts, fmt.Sprintf("DEFAULT %v", v)) default: @@ -764,13 +813,18 @@ func equivalentTypeListSQL(sqlType string) string { variants := pgsql.EquivalentSQLTypeVariants(sqlType) quoted := make([]string, 0, len(variants)) for _, variant := range variants { - quoted = append(quoted, fmt.Sprintf("'%s'", escapeQuote(variant))) + quoted = append(quoted, quote(variant)) } return strings.Join(quoted, ", ") } // WriteSchema writes a single schema and all its tables func (w *Writer) WriteSchema(schema *models.Schema) error { + var err error + schema, err = w.prepareSchema(schema) + if err != nil { + return err + } if w.writer == nil { w.writer = os.Stdout } @@ -885,6 +939,11 @@ func (w *Writer) WriteTable(table *models.Table) error { // WriteAddColumnStatements writes ALTER TABLE ADD COLUMN statements for a database // This is used for schema evolution/migration when new columns are added func (w *Writer) WriteAddColumnStatements(db *models.Database) error { + prepared, preparationErr := prepareModel(db, w.options.LowercaseIdentifiers) + if preparationErr != nil { + return preparationErr + } + db = prepared var writer io.Writer var file *os.File var err error @@ -926,8 +985,8 @@ func (w *Writer) writeCreateSchema(schema *models.Schema) error { return nil } - fmt.Fprintf(w.writer, "-- Schema: %s\n", schema.Name) - fmt.Fprintf(w.writer, "CREATE SCHEMA IF NOT EXISTS %s;\n\n", schema.SQLName()) + fmt.Fprintf(w.writer, "-- Schema: %s\n", commentText(schema.Name)) + fmt.Fprintf(w.writer, "CREATE SCHEMA IF NOT EXISTS %s;\n\n", quoteIdent(schema.Name)) return nil } @@ -946,7 +1005,7 @@ func (w *Writer) writeRequiredExtensions(schema *models.Schema) error { // writeSequences generates CREATE SEQUENCE statements for identity columns func (w *Writer) writeSequences(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Sequences for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Sequences for schema: %s\n", commentText(schema.Name)) for _, table := range schema.Tables { // Only create the sequence the primary key's nextval() default uses @@ -978,11 +1037,11 @@ func (w *Writer) writeSequences(schema *models.Schema) error { // writeCreateTables generates CREATE TABLE statements func (w *Writer) writeCreateTables(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Tables for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Tables for schema: %s\n", commentText(schema.Name)) for _, table := range schema.Tables { fmt.Fprintf(w.writer, "CREATE TABLE IF NOT EXISTS %s (\n", - w.qualTable(schema.SQLName(), table.SQLName())) + w.qualTable(schema.Name, table.Name)) // Write columns columns := getSortedColumns(table.Columns) @@ -1003,7 +1062,7 @@ func (w *Writer) writeCreateTables(schema *models.Schema) error { // writeAddColumns generates ALTER TABLE ADD COLUMN statements for missing columns func (w *Writer) writeAddColumns(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Add missing columns for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Add missing columns for schema: %s\n", commentText(schema.Name)) for _, table := range schema.Tables { // Sort columns by sequence or name for consistent output @@ -1032,7 +1091,7 @@ func (w *Writer) writeAddColumns(schema *models.Schema) error { } func (w *Writer) writeAlterColumnTypes(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Alter column types for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Alter column types for schema: %s\n", commentText(schema.Name)) statements, err := w.GenerateAlterColumnTypeStatements(schema) if err != nil { @@ -1047,7 +1106,7 @@ func (w *Writer) writeAlterColumnTypes(schema *models.Schema) error { } func (w *Writer) writeAlterColumnDefaults(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Alter column defaults for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Alter column defaults for schema: %s\n", commentText(schema.Name)) statements, err := w.GenerateAlterColumnDefaultStatements(schema) if err != nil { @@ -1062,7 +1121,7 @@ func (w *Writer) writeAlterColumnDefaults(schema *models.Schema) error { } func (w *Writer) writeAlterColumnNullability(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Alter column nullability for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Alter column nullability for schema: %s\n", commentText(schema.Name)) statements, err := w.GenerateAlterColumnNullabilityStatements(schema) if err != nil { @@ -1078,7 +1137,7 @@ func (w *Writer) writeAlterColumnNullability(schema *models.Schema) error { // writePrimaryKeys generates ALTER TABLE statements for primary keys func (w *Writer) writePrimaryKeys(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Primary keys for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Primary keys for schema: %s\n", commentText(schema.Name)) for _, table := range schema.Tables { // Find primary key constraint @@ -1091,21 +1150,21 @@ func (w *Writer) writePrimaryKeys(schema *models.Schema) error { } var columnNames []string - pkName := fmt.Sprintf("pk_%s_%s", schema.SQLName(), table.SQLName()) + pkName := fmt.Sprintf("pk_%s_%s", schema.Name, table.Name) if pkConstraint != nil { // Build column list from explicit constraint columnNames = make([]string, 0, len(pkConstraint.Columns)) for _, colName := range pkConstraint.Columns { if col, ok := table.Columns[colName]; ok { - columnNames = append(columnNames, col.SQLName()) + columnNames = append(columnNames, col.Name) } } } else { // No explicit PK constraint, check for columns with IsPrimaryKey = true for _, col := range table.Columns { if col.IsPrimaryKey { - columnNames = append(columnNames, col.SQLName()) + columnNames = append(columnNames, col.Name) } } // Sort for consistent output @@ -1127,7 +1186,7 @@ func (w *Writer) writePrimaryKeys(schema *models.Schema) error { TableName: table.Name, ConstraintName: pkName, AutoGenNames: formatStringList(autoGenPKNames), - Columns: strings.Join(columnNames, ", "), + Columns: quotedIdentifiers(columnNames), ColumnNames: formatStringList(columnNames), } @@ -1144,7 +1203,7 @@ func (w *Writer) writePrimaryKeys(schema *models.Schema) error { // writeIndexes generates CREATE INDEX statements func (w *Writer) writeIndexes(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Indexes for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Indexes for schema: %s\n", commentText(schema.Name)) for _, table := range schema.Tables { // Sort indexes by name for consistent output @@ -1170,7 +1229,7 @@ func (w *Writer) writeIndexes(schema *models.Schema) error { indexType = "ukey" } columnSuffix := strings.Join(index.Columns, "_") - indexName = fmt.Sprintf("%s_%s_%s", indexType, table.SQLName(), strings.ToLower(columnSuffix)) + indexName = fmt.Sprintf("%s_%s_%s", indexType, table.Name, strings.ToLower(columnSuffix)) } indexType := index.Type @@ -1196,7 +1255,7 @@ func (w *Writer) writeIndexes(schema *models.Schema) error { tablespaceClause := "" if ts := pgIndexDirectiveTablespace(index); ts != "" { - tablespaceClause = fmt.Sprintf(" TABLESPACE %s", ts) + tablespaceClause = fmt.Sprintf(" TABLESPACE %s", quoteIdent(ts)) } whereClause := "" @@ -1210,9 +1269,9 @@ func (w *Writer) writeIndexes(schema *models.Schema) error { } fmt.Fprintf(w.writer, "CREATE %sINDEX %sIF NOT EXISTS %s\n", - unique, concurrently, indexName) + unique, concurrently, quoteIdent(indexName)) fmt.Fprintf(w.writer, " ON %s USING %s (%s)%s%s%s;\n\n", - w.qualTable(schema.SQLName(), table.SQLName()), indexType, strings.Join(columnExprs, ", "), withClause, tablespaceClause, whereClause) + w.qualTable(schema.Name, table.Name), indexType, strings.Join(columnExprs, ", "), withClause, tablespaceClause, whereClause) } } @@ -1221,7 +1280,7 @@ func (w *Writer) writeIndexes(schema *models.Schema) error { // writeUniqueConstraints generates ALTER TABLE statements for unique constraints func (w *Writer) writeUniqueConstraints(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Unique constraints for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Unique constraints for schema: %s\n", commentText(schema.Name)) for _, table := range schema.Tables { // Sort constraints by name for consistent output @@ -1240,7 +1299,7 @@ func (w *Writer) writeUniqueConstraints(schema *models.Schema) error { columnExprs := make([]string, 0, len(constraint.Columns)) for _, colName := range constraint.Columns { if col, ok := table.Columns[colName]; ok { - columnExprs = append(columnExprs, col.SQLName()) + columnExprs = append(columnExprs, quoteIdent(col.Name)) } } @@ -1268,7 +1327,7 @@ func (w *Writer) writeUniqueConstraints(schema *models.Schema) error { // writeCheckConstraints generates ALTER TABLE statements for check constraints func (w *Writer) writeCheckConstraints(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Check constraints for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Check constraints for schema: %s\n", commentText(schema.Name)) for _, table := range schema.Tables { // Sort constraints by name for consistent output @@ -1307,7 +1366,7 @@ func (w *Writer) writeCheckConstraints(schema *models.Schema) error { // writeForeignKeys generates ALTER TABLE statements for foreign keys func (w *Writer) writeForeignKeys(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Foreign keys for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Foreign keys for schema: %s\n", commentText(schema.Name)) for _, table := range schema.Tables { // Sort relationships by name for consistent output @@ -1327,7 +1386,7 @@ func (w *Writer) writeForeignKeys(schema *models.Schema) error { fkName = name } if fkName == "" { - fkName = fmt.Sprintf("fk_%s_%s", table.SQLName(), rel.ToTable) + fkName = fmt.Sprintf("fk_%s_%s", table.Name, rel.ToTable) } // Find the foreign key constraint that matches this relationship. @@ -1354,13 +1413,13 @@ func (w *Writer) writeForeignKeys(schema *models.Schema) error { sourceColumns := make([]string, 0, len(fkConstraint.Columns)) for _, colName := range fkConstraint.Columns { if col, ok := table.Columns[colName]; ok { - sourceColumns = append(sourceColumns, col.SQLName()) + sourceColumns = append(sourceColumns, quoteIdent(col.Name)) } } targetColumns := make([]string, 0, len(fkConstraint.ReferencedColumns)) for _, colName := range fkConstraint.ReferencedColumns { - targetColumns = append(targetColumns, strings.ToLower(colName)) + targetColumns = append(targetColumns, quoteIdent(colName)) } if len(sourceColumns) == 0 || len(targetColumns) == 0 { @@ -1435,15 +1494,15 @@ func (w *Writer) writeForeignKeys(schema *models.Schema) error { sourceColumns := make([]string, 0, len(constraint.Columns)) for _, colName := range constraint.Columns { if col, ok := table.Columns[colName]; ok { - sourceColumns = append(sourceColumns, col.SQLName()) + sourceColumns = append(sourceColumns, quoteIdent(col.Name)) } else { - sourceColumns = append(sourceColumns, colName) + sourceColumns = append(sourceColumns, quoteIdent(colName)) } } targetColumns := make([]string, 0, len(constraint.ReferencedColumns)) for _, colName := range constraint.ReferencedColumns { - targetColumns = append(targetColumns, strings.ToLower(colName)) + targetColumns = append(targetColumns, quoteIdent(colName)) } if len(sourceColumns) == 0 || len(targetColumns) == 0 { @@ -1492,7 +1551,7 @@ func (w *Writer) writeForeignKeys(schema *models.Schema) error { // writeSetSequenceValues generates statements to set sequence current values func (w *Writer) writeSetSequenceValues(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Set sequence values for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Set sequence values for schema: %s\n", commentText(schema.Name)) for _, table := range schema.Tables { seqName := primaryKeySequenceName(table) @@ -1521,22 +1580,22 @@ func (w *Writer) writeSetSequenceValues(schema *models.Schema) error { // writeComments generates COMMENT statements for tables and columns func (w *Writer) writeComments(schema *models.Schema) error { - fmt.Fprintf(w.writer, "-- Comments for schema: %s\n", schema.Name) + fmt.Fprintf(w.writer, "-- Comments for schema: %s\n", commentText(schema.Name)) for _, table := range schema.Tables { // Table comment if table.Description != "" { - fmt.Fprintf(w.writer, "COMMENT ON TABLE %s IS '%s';\n", - w.qualTable(schema.SQLName(), table.SQLName()), - escapeQuote(table.Description)) + fmt.Fprintf(w.writer, "COMMENT ON TABLE %s IS %s;\n", + w.qualTable(schema.Name, table.Name), + quote(table.Description)) } // Column comments for _, col := range getSortedColumns(table.Columns) { if col.Description != "" { - fmt.Fprintf(w.writer, "COMMENT ON COLUMN %s.%s IS '%s';\n", - w.qualTable(schema.SQLName(), table.SQLName()), col.SQLName(), - escapeQuote(col.Description)) + fmt.Fprintf(w.writer, "COMMENT ON COLUMN %s.%s IS %s;\n", + w.qualTable(schema.Name, table.Name), quoteIdent(col.Name), + quote(col.Description)) } } @@ -1897,6 +1956,9 @@ func nativeGistBaseType(baseType string) bool { } func resolveIndexColumn(table *models.Table, colName string) (*models.Column, bool) { + if len(colName) >= 2 && colName[0] == '"' && colName[len(colName)-1] == '"' { + colName = strings.ReplaceAll(colName[1:len(colName)-1], `""`, `"`) + } if table == nil { return nil, false } @@ -1904,18 +1966,8 @@ func resolveIndexColumn(table *models.Table, colName string) (*models.Column, bo return col, true } - normalized := strings.ToLower(strings.Trim(colName, `"`)) - for key, col := range table.Columns { - if col == nil { - continue - } - if strings.ToLower(strings.Trim(key, `"`)) == normalized { - return col, true - } - if strings.ToLower(strings.Trim(col.Name, `"`)) == normalized { - return col, true - } - if strings.ToLower(strings.Trim(col.SQLName(), `"`)) == normalized { + for _, col := range table.Columns { + if col != nil && col.Name == colName { return col, true } } @@ -1972,7 +2024,7 @@ func sortIndexes(indexes map[string]*models.Index) []*models.Index { func formatStringList(items []string) string { quoted := make([]string, len(items)) for i, item := range items { - quoted[i] = fmt.Sprintf("'%s'", escapeQuote(item)) + quoted[i] = quote(item) } return strings.Join(quoted, ", ") } @@ -2049,19 +2101,20 @@ func extractSequenceName(defaultExpr string) string { if start == -1 { return "" } - end := strings.Index(defaultExpr[start+1:], "'") - if end == -1 { - return "" + var name strings.Builder + for i := start + 1; i < len(defaultExpr); i++ { + if defaultExpr[i] == '\'' { + if i+1 < len(defaultExpr) && defaultExpr[i+1] == '\'' { + name.WriteByte('\'') + i++ + continue + } + return regclassLastName(name.String()) + } + name.WriteByte(defaultExpr[i]) } + return "" - fullName := defaultExpr[start+1 : start+1+end] - - // Remove schema prefix if present - parts := strings.Split(fullName, ".") - if len(parts) > 1 { - return parts[len(parts)-1] - } - return fullName } // executeDatabaseSQL applies db to a PostgreSQL database. By default it reads the live schema @@ -2126,7 +2179,7 @@ func (w *Writer) diffStatements(model, current *models.Database) ([]string, erro currentSchemas := make(map[string]*models.Schema) for _, cs := range current.Schemas { if cs != nil { - currentSchemas[strings.ToLower(cs.Name)] = cs + currentSchemas[cs.Name] = cs } } @@ -2142,25 +2195,25 @@ func (w *Writer) diffStatements(model, current *models.Database) ([]string, erro return nil, err } - cs := currentSchemas[strings.ToLower(schema.Name)] + cs := currentSchemas[schema.Name] if cs == nil && schema.Name != "public" { statements = append(statements, - fmt.Sprintf("-- Schema: %s", schema.Name), - fmt.Sprintf("CREATE SCHEMA IF NOT EXISTS %s", schema.SQLName())) + fmt.Sprintf("-- Schema: %s", commentText(schema.Name)), + fmt.Sprintf("CREATE SCHEMA IF NOT EXISTS %s", quoteIdent(schema.Name))) } existing := make(map[string]*models.Table) knownSequences := make(map[string]bool) if cs != nil { for _, t := range cs.Tables { - existing[strings.ToLower(t.Name)] = t + existing[t.Name] = t } for _, seq := range cs.Sequences { - knownSequences[strings.ToLower(seq.Name)] = true + knownSequences[seq.Name] = true } } for _, table := range schema.Tables { - currentTable, ok := existing[strings.ToLower(table.Name)] + currentTable, ok := existing[table.Name] if !ok { // New table: no rows yet, so the sequence only needs creating. if stmt := w.primaryKeySequenceStatement(schema, table); stmt != "" { @@ -2173,7 +2226,7 @@ func (w *Writer) diffStatements(model, current *models.Database) ([]string, erro // database does not have yet: create it and move it past the data, // otherwise it restarts at 1 and collides with existing rows. seqName := primaryKeySequenceName(table) - if seqName == "" || knownSequences[strings.ToLower(seqName)] { + if seqName == "" || knownSequences[seqName] { continue } if cpk := currentTable.GetPrimaryKey(); cpk != nil && columnDefaultsEqual(table.GetPrimaryKey().Default, cpk.Default) { @@ -2209,7 +2262,7 @@ func (w *Writer) diffStatements(model, current *models.Database) ([]string, erro continue } if script.Schema != "" && script.Schema != lastSchema { - statements = append(statements, fmt.Sprintf("-- Schema: %s", script.Schema)) + statements = append(statements, fmt.Sprintf("-- Schema: %s", commentText(script.Schema))) lastSchema = script.Schema } statements = append(statements, body) @@ -2311,23 +2364,8 @@ func extractTableNameFromCreate(stmt string) string { rest = strings.TrimSpace(rest[13:]) } - // Get the table name (first token before '(' or whitespace) - tokens := strings.FieldsFunc(rest, func(r rune) bool { - return r == '(' || r == ' ' || r == '\n' || r == '\t' - }) - - if len(tokens) == 0 { - return "" - } - - // Handle schema.table format - fullName := tokens[0] - parts := strings.Split(fullName, ".") - if len(parts) > 1 { - return parts[len(parts)-1] - } - - return fullName + _, name := parseQualifiedIdent(rest) + return name } // truncateStatement truncates long SQL statements for error messages @@ -2445,11 +2483,21 @@ func extractSQLStringValue(stmt, key string) string { // parseQualifiedIdent extracts (schema, name) from the start of s, handling optional "schema"."table" quoting. func parseQualifiedIdent(s string) (schema, name string) { token := firstBareIdent(s) - parts := strings.SplitN(token, ".", 2) - if len(parts) == 2 { - return stripQuotes(parts[0]), stripQuotes(parts[1]) + quoted := false + for i := 0; i < len(token); i++ { + if token[i] == '"' { + if quoted && i+1 < len(token) && token[i+1] == '"' { + i++ + continue + } + quoted = !quoted + } + if token[i] == '.' && !quoted { + return stripQuotes(token[:i]), stripQuotes(token[i+1:]) + } } return "", stripQuotes(token) + } // firstBareIdent returns the first whitespace/delimiter-terminated token, stripping outer quotes. @@ -2458,11 +2506,21 @@ func firstBareIdent(s string) string { if s == "" { return "" } - end := strings.IndexAny(s, " \t\n(,;") - if end == -1 { - return s + quoted := false + for i := 0; i < len(s); i++ { + if s[i] == '"' { + if quoted && i+1 < len(s) && s[i+1] == '"' { + i++ + continue + } + quoted = !quoted + } + if !quoted && strings.ContainsRune(" \t\r\n(,;", rune(s[i])) { + return s[:i] + } } - return s[:end] + return s + } // firstIdentAfterKeyword returns the first identifier token after keyword (matched case-insensitively via upperStmt). @@ -2476,7 +2534,10 @@ func firstIdentAfterKeyword(stmt, upperStmt, keyword string) string { // stripQuotes removes surrounding double-quotes from an identifier. func stripQuotes(s string) string { - return strings.Trim(s, "\"") + if len(s) >= 2 && s[0] == '"' && s[len(s)-1] == '"' { + return strings.ReplaceAll(s[1:len(s)-1], `""`, `"`) + } + return s } // buildStmtContext assembles a display string from available identifiers. @@ -2598,3 +2659,5 @@ func detectStatementType(stmt string) string { func quoteIdentifier(s string) string { return quoteIdent(s) } + +func commentText(s string) string { return strings.NewReplacer("\r", " ", "\n", " ").Replace(s) } diff --git a/pkg/writers/pgsql/writer_test.go b/pkg/writers/pgsql/writer_test.go index be00e8e..eb1a02c 100644 --- a/pkg/writers/pgsql/writer_test.go +++ b/pkg/writers/pgsql/writer_test.go @@ -22,7 +22,7 @@ func TestWriteDatabase(t *testing.T) { idCol := models.InitColumn("id", "users", "public") idCol.Type = "integer" idCol.Description = "Primary key" - idCol.Default = "nextval('public.identity_users_id'::regclass)" + idCol.Default = "nextval('\"public\".\"identity_users_id\"'::regclass)" table.Columns["id"] = idCol nameCol := models.InitColumn("name", "users", "public") @@ -117,7 +117,7 @@ func TestWriteDatabase_ConcurrentIndex(t *testing.T) { output := buf.String() - if !strings.Contains(output, "CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_users_email") { + if !strings.Contains(output, "CREATE INDEX CONCURRENTLY IF NOT EXISTS \"idx_users_email\"") { t.Errorf("Output missing CONCURRENTLY index creation:\n%s", output) } } @@ -151,7 +151,7 @@ func TestWriteDatabase_GinIndexOnTextArrayDoesNotUseTrigramOperatorClass(t *test } output := buf.String() - if !strings.Contains(output, `USING gin (tags array_ops)`) { + if !strings.Contains(output, `USING gin ("tags" array_ops)`) { t.Fatalf("expected GIN index on array column with array_ops, got:\n%s", output) } if strings.Contains(output, "gin_trgm_ops") { @@ -172,7 +172,7 @@ func TestWriteDatabase_GinIndexOnQuotedTextColumnUsesTrigramOperatorClass(t *tes index := &models.Index{ Name: "idx_agent_personas_name_gin", Type: "gin", - Columns: []string{`"name"`}, + Columns: []string{"name"}, } table.Indexes[index.Name] = index @@ -191,7 +191,7 @@ func TestWriteDatabase_GinIndexOnQuotedTextColumnUsesTrigramOperatorClass(t *tes if !strings.Contains(output, `CREATE EXTENSION IF NOT EXISTS pg_trgm`) { t.Fatalf("expected trigram extension for text GIN index, got:\n%s", output) } - if !strings.Contains(output, `USING gin (name gin_trgm_ops)`) { + if !strings.Contains(output, `USING gin ("name" gin_trgm_ops)`) { t.Fatalf("expected quoted text GIN index to include gin_trgm_ops, got:\n%s", output) } } @@ -225,7 +225,7 @@ func TestWriteDatabase_GinIndexOnJSONBUsesJSONBOperatorClass(t *testing.T) { } output := buf.String() - if !strings.Contains(output, `USING gin (details jsonb_ops)`) { + if !strings.Contains(output, `USING gin ("details" jsonb_ops)`) { t.Fatalf("expected GIN jsonb index to include jsonb_ops, got:\n%s", output) } if strings.Contains(output, "gin_trgm_ops") { @@ -372,10 +372,10 @@ func TestWriteUniqueConstraints(t *testing.T) { if !strings.Contains(output, "-- Unique constraints for schema: public") { t.Errorf("Output missing unique constraints header") } - if !strings.Contains(output, "ADD CONSTRAINT uq_email UNIQUE (email)") { + if !strings.Contains(output, "ADD CONSTRAINT \"uq_email\" UNIQUE (\"email\")") { t.Errorf("Output missing uq_email unique constraint\nFull output:\n%s", output) } - if !strings.Contains(output, "ADD CONSTRAINT uq_guid UNIQUE (guid)") { + if !strings.Contains(output, "ADD CONSTRAINT \"uq_guid\" UNIQUE (\"guid\")") { t.Errorf("Output missing uq_guid unique constraint\nFull output:\n%s", output) } } @@ -453,13 +453,13 @@ func TestWriteCheckConstraints(t *testing.T) { if !strings.Contains(output, "-- Check constraints for schema: public") { t.Errorf("Output missing check constraints header") } - if !strings.Contains(output, "ADD CONSTRAINT ck_price_positive CHECK (price >= 0)") { + if !strings.Contains(output, "ADD CONSTRAINT \"ck_price_positive\" CHECK (price >= 0)") { t.Errorf("Output missing ck_price_positive check constraint\nFull output:\n%s", output) } - if !strings.Contains(output, "ADD CONSTRAINT ck_status_valid CHECK (status IN ('active', 'inactive', 'discontinued'))") { + if !strings.Contains(output, "ADD CONSTRAINT \"ck_status_valid\" CHECK (status IN ('active', 'inactive', 'discontinued'))") { t.Errorf("Output missing ck_status_valid check constraint\nFull output:\n%s", output) } - if !strings.Contains(output, "ADD CONSTRAINT ck_quantity_nonnegative CHECK (quantity >= 0)") { + if !strings.Contains(output, "ADD CONSTRAINT \"ck_quantity_nonnegative\" CHECK (quantity >= 0)") { t.Errorf("Output missing ck_quantity_nonnegative check constraint\nFull output:\n%s", output) } } @@ -573,9 +573,9 @@ func TestWriteAllConstraintTypes(t *testing.T) { // Verify all constraint types are present expectedConstraints := map[string]string{ "Primary Key": "PRIMARY KEY", - "Unique": "ADD CONSTRAINT uq_order_number UNIQUE (order_number)", - "Check (total)": "ADD CONSTRAINT ck_total_positive CHECK (total > 0)", - "Check (status)": "ADD CONSTRAINT ck_status_valid CHECK (status IN ('pending', 'completed', 'cancelled'))", + "Unique": "ADD CONSTRAINT \"uq_order_number\" UNIQUE (\"order_number\")", + "Check (total)": "ADD CONSTRAINT \"ck_total_positive\" CHECK (total > 0)", + "Check (status)": "ADD CONSTRAINT \"ck_status_valid\" CHECK (status IN ('pending', 'completed', 'cancelled'))", "Foreign Key": "FOREIGN KEY", } @@ -640,7 +640,7 @@ func TestEscapeQuote(t *testing.T) { input string expected string }{ - {"simple text", "simple text"}, + {"\"simple\" text", "\"simple\" text"}, {"text with 'quote'", "text with ''quote''"}, {"multiple 'quotes' here", "multiple ''quotes'' here"}, {"", ""}, @@ -1016,9 +1016,9 @@ func TestGenerateAddColumnStatements(t *testing.T) { // Verify expected elements expectedStrings := []string{ - "ALTER TABLE public.users ADD COLUMN IF NOT EXISTS id integer NOT NULL", - "ALTER TABLE public.users ADD COLUMN IF NOT EXISTS email varchar(255) NOT NULL", - "ALTER TABLE public.users ADD COLUMN IF NOT EXISTS status text DEFAULT 'active'", + "ALTER TABLE \"public\".\"users\" ADD COLUMN IF NOT EXISTS \"id\" integer NOT NULL", + "ALTER TABLE \"public\".\"users\" ADD COLUMN IF NOT EXISTS \"email\" varchar(255) NOT NULL", + "ALTER TABLE \"public\".\"users\" ADD COLUMN IF NOT EXISTS \"status\" text DEFAULT 'active'", } for _, expected := range expectedStrings { @@ -1071,10 +1071,10 @@ func TestWriteAddColumnStatements(t *testing.T) { t.Logf("Generated output:\n%s", output) // Verify output contains expected elements - if !strings.Contains(output, "ALTER TABLE public.products ADD COLUMN IF NOT EXISTS id integer") { + if !strings.Contains(output, "ALTER TABLE \"public\".\"products\" ADD COLUMN IF NOT EXISTS \"id\" integer") { t.Errorf("Output missing ADD COLUMN for id\nFull output:\n%s", output) } - if !strings.Contains(output, "ALTER TABLE public.products ADD COLUMN IF NOT EXISTS description text") { + if !strings.Contains(output, "ALTER TABLE \"public\".\"products\" ADD COLUMN IF NOT EXISTS \"description\" text") { t.Errorf("Output missing ADD COLUMN for description\nFull output:\n%s", output) } if strings.Contains(output, "information_schema.columns") { @@ -1115,16 +1115,16 @@ func TestWriteSchema_EmitsGuardedAlterColumnTypeStatements(t *testing.T) { if !strings.Contains(output, "pg_catalog.format_type") { t.Fatalf("expected guarded live-type check, got:\n%s", output) } - if !strings.Contains(output, "ALTER COLUMN name TYPE character varying(255)") { + if !strings.Contains(output, "ALTER COLUMN \"name\" TYPE character varying(255)") { t.Fatalf("expected guarded alter for character varying(255), got:\n%s", output) } if !strings.Contains(output, "ARRAY['varchar(255)', 'character varying(255)']") { t.Fatalf("expected equivalent type spellings for varchar guard, got:\n%s", output) } - if !strings.Contains(output, "ALTER COLUMN tags TYPE text[]") { + if !strings.Contains(output, "ALTER COLUMN \"tags\" TYPE text[]") { t.Fatalf("expected guarded alter for array type, got:\n%s", output) } - if !strings.Contains(output, `ALTER COLUMN tags TYPE text[] USING tags::text[];`) { + if !strings.Contains(output, `ALTER COLUMN "tags" TYPE text[] USING "tags"::text[];`) { t.Fatalf("expected guarded alter for array type to include USING cast, got:\n%s", output) } } @@ -1158,7 +1158,7 @@ func TestWriteSchema_EmitsGuardedAlterColumnDefaultStatements(t *testing.T) { if !strings.Contains(output, "pg_get_expr(d.adbin, d.adrelid)") { t.Fatalf("expected guarded live-default check, got:\n%s", output) } - if !strings.Contains(output, "ALTER COLUMN status SET DEFAULT 'active'") { + if !strings.Contains(output, "ALTER COLUMN \"status\" SET DEFAULT 'active'") { t.Fatalf("expected guarded SET DEFAULT for status column, got:\n%s", output) } } @@ -1189,7 +1189,7 @@ func TestWriteSchema_AlterColumnDefaultStripsBackticksFromFunctionExpression(t * if strings.Contains(output, "`") { t.Fatalf("expected no backticks in generated SQL, got:\n%s", output) } - if !strings.Contains(output, "ALTER COLUMN updatedat SET DEFAULT now()") { + if !strings.Contains(output, "ALTER COLUMN \"updatedat\" SET DEFAULT now()") { t.Fatalf("expected guarded SET DEFAULT now() without backticks, got:\n%s", output) } } @@ -1216,7 +1216,7 @@ func TestWriteSchema_GuardedAlterColumnTypePreservesConversionFailure(t *testing } output := buf.String() - if !strings.Contains(output, "ALTER COLUMN name TYPE integer USING name::integer;") { + if !strings.Contains(output, "ALTER COLUMN \"name\" TYPE integer USING \"name\"::integer;") { t.Fatalf("expected explicit type conversion, got:\n%s", output) } for _, unsafe := range []string{"EXCEPTION WHEN OTHERS", "renamed_column", "RENAME COLUMN"} { @@ -1258,7 +1258,7 @@ func TestWriteSchema_EmitsGuardedAlterColumnNullabilityStatements(t *testing.T) if !strings.Contains(output, "current_not_null IS DISTINCT FROM false") { t.Fatalf("expected guard comparing live nullability against desired value, got:\n%s", output) } - if !strings.Contains(output, "ALTER COLUMN rid_service_instance_type DROP NOT NULL") { + if !strings.Contains(output, "ALTER COLUMN \"rid_service_instance_type\" DROP NOT NULL") { t.Fatalf("expected guarded DROP NOT NULL for nullable column, got:\n%s", output) } } @@ -1284,13 +1284,13 @@ func TestWriteSchema_UsesStorageTypeForSerialAlterStatements(t *testing.T) { } output := buf.String() - if !strings.Contains(output, "ALTER COLUMN id TYPE bigint") { + if !strings.Contains(output, "ALTER COLUMN \"id\" TYPE bigint") { t.Fatalf("expected serial alter to use bigint storage type, got:\n%s", output) } - if strings.Contains(output, "ALTER COLUMN id TYPE bigserial;") { + if strings.Contains(output, "ALTER COLUMN \"id\" TYPE bigserial;") { t.Fatalf("did not expect invalid bigserial alter statement, got:\n%s", output) } - if !strings.Contains(output, `ALTER COLUMN id TYPE bigint USING id::bigint;`) { + if !strings.Contains(output, `ALTER COLUMN "id" TYPE bigint USING "id"::bigint;`) { t.Fatalf("expected serial alter to include USING cast, got:\n%s", output) } } @@ -1361,7 +1361,7 @@ func TestWriteDatabase_VectorAndPostGISColumnsCreateExtensions(t *testing.T) { func TestWriteDatabase_HNSWIndexUsesDefaultVectorOperatorClass(t *testing.T) { output := writeDatabaseOutput(t, buildVectorSpatialSchema("hnsw", "")) - if !strings.Contains(output, "USING hnsw (embedding vector_cosine_ops)") { + if !strings.Contains(output, "USING hnsw (\"embedding\" vector_cosine_ops)") { t.Fatalf("expected hnsw index with default vector operator class, got:\n%s", output) } if !strings.Contains(output, "CREATE EXTENSION IF NOT EXISTS vector;") { @@ -1372,7 +1372,7 @@ func TestWriteDatabase_HNSWIndexUsesDefaultVectorOperatorClass(t *testing.T) { func TestWriteDatabase_VectorIndexHonoursRequestedOperatorClassAndStorageParameters(t *testing.T) { output := writeDatabaseOutput(t, buildVectorSpatialSchema("ivfflat", "opclass=vector_l2_ops; with (lists=100)")) - if !strings.Contains(output, "USING ivfflat (embedding vector_l2_ops) WITH (lists = 100)") { + if !strings.Contains(output, "USING ivfflat (\"embedding\" vector_l2_ops) WITH (lists = 100)") { t.Fatalf("expected ivfflat index with requested opclass and storage parameters, got:\n%s", output) } } @@ -1380,7 +1380,7 @@ func TestWriteDatabase_VectorIndexHonoursRequestedOperatorClassAndStorageParamet func TestWriteDatabase_VectorIndexIgnoresIncompatibleOperatorClass(t *testing.T) { output := writeDatabaseOutput(t, buildVectorSpatialSchema("hnsw", "opclass=halfvec_l2_ops")) - if !strings.Contains(output, "USING hnsw (embedding vector_cosine_ops)") { + if !strings.Contains(output, "USING hnsw (\"embedding\" vector_cosine_ops)") { t.Fatalf("expected halfvec operator class to be rejected for a vector column, got:\n%s", output) } } @@ -1416,7 +1416,7 @@ func TestWriteDatabase_GistIndexOnGeometryUsesDefaultOperatorClass(t *testing.T) output := writeDatabaseOutput(t, db) - if !strings.Contains(output, "USING gist (geom)") { + if !strings.Contains(output, "USING gist (\"geom\")") { t.Fatalf("expected gist index to rely on the PostGIS default operator class, got:\n%s", output) } } @@ -1442,7 +1442,7 @@ func TestWriteDatabase_GistIndexHonoursRequestedSpatialOperatorClass(t *testing. output := writeDatabaseOutput(t, db) - if !strings.Contains(output, "USING gist (geom gist_geometry_ops_nd)") { + if !strings.Contains(output, "USING gist (\"geom\" gist_geometry_ops_nd)") { t.Fatalf("expected requested spatial operator class, got:\n%s", output) } } @@ -1460,7 +1460,7 @@ func TestGenerateDatabaseStatements_VectorIndexIncludesOperatorClassAndParameter for _, want := range []string{ "CREATE EXTENSION IF NOT EXISTS vector", "CREATE EXTENSION IF NOT EXISTS postgis", - "USING hnsw (embedding vector_ip_ops) WITH (m = 16)", + "USING hnsw (\"embedding\" vector_ip_ops) WITH (m = 16)", } { if !strings.Contains(joined, want) { t.Fatalf("expected statements to contain %q, got:\n%s", want, joined) @@ -1509,7 +1509,7 @@ func TestGenerateColumnDefinition_GeneratedColumnEmitsGeneratedClauseNotDefault( got := w.generateColumnDefinition(col) - want := "geom text GENERATED ALWAYS AS (st_makepoint(lon, lat)) STORED NOT NULL" + want := "\"geom\" text GENERATED ALWAYS AS (st_makepoint(lon, lat)) STORED NOT NULL" if got != want { t.Fatalf("generateColumnDefinition() = %q, want %q", got, want) } @@ -1528,7 +1528,7 @@ func TestGenerateColumnDefinition_IdentityColumnEmitsIdentityClauseNotDefault(t got := w.generateColumnDefinition(col) - want := "id bigint GENERATED ALWAYS AS IDENTITY NOT NULL" + want := "\"id\" bigint GENERATED ALWAYS AS IDENTITY NOT NULL" if got != want { t.Fatalf("generateColumnDefinition() = %q, want %q", got, want) } @@ -1575,7 +1575,7 @@ func TestWriteDatabase_MultipleForeignKeysToSameTable(t *testing.T) { } output := buf.String() for _, name := range []string{"rid_department", "rid_manager"} { - want := "ADD CONSTRAINT fk_employee_" + name + "\n FOREIGN KEY (" + name + ")" + want := "ADD CONSTRAINT " + quoteIdent("fk_employee_"+name) + "\n FOREIGN KEY (" + quoteIdent(name) + ")" if !strings.Contains(output, want) { t.Fatalf("run %d: missing %q in output:\n%s", i, want, output) } diff --git a/pkg/writers/sqlexec/README.md b/pkg/writers/sqlexec/README.md index 661b585..1532eb2 100644 --- a/pkg/writers/sqlexec/README.md +++ b/pkg/writers/sqlexec/README.md @@ -264,3 +264,11 @@ docker rm postgres-test - **SQL Directory Reader**: `pkg/readers/sqldir/` - Read scripts from filesystem - **Script Model**: `pkg/models/models.go` - Script structure definition - **pgx Documentation**: https://github.com/jackc/pgx - PostgreSQL driver docs + +## Model embed access + +SQL directory readers expand embed directives within their trusted input root. +When executing unexpanded directives from a model, explicitly configure +`WriterOptions.Metadata["asset_root"]` with the allowed directory. Script +`source_path` metadata alone never authorizes file reads. Paths, symlinks, regular +file types, and the 64 MiB asset-size limit are checked before reading. diff --git a/pkg/writers/sqlexec/writer.go b/pkg/writers/sqlexec/writer.go index 8d7f75d..62dd081 100644 --- a/pkg/writers/sqlexec/writer.go +++ b/pkg/writers/sqlexec/writer.go @@ -120,7 +120,8 @@ func (w *Writer) runScripts(ctx context.Context, conn *pgx.Conn, scripts []*mode backend := writers.ExecutionBackend{ScriptManaged: true, Exec: func(ctx context.Context, i int, _ string) error { script := scripts[i] fmt.Printf("Executing script: %s (Priority=%d, Sequence=%d)\n", script.Name, script.Priority, script.Sequence) - sql, err := processEmbedDirectives(script) + assetRoot, _ := w.options.Metadata["asset_root"].(string) + sql, err := processEmbedDirectives(script, assetRoot) if err != nil { return fmt.Errorf("preparing script %s: %w", script.Name, err) } @@ -141,10 +142,16 @@ func scriptOperations(scripts []*models.Script) []writers.ExecutionOperation { return operations } -func processEmbedDirectives(script *models.Script) (string, error) { - sqlPath, _ := script.Metadata[assetloader.ScriptSourcePathMetadataKey].(string) - if sqlPath == "" { +func processEmbedDirectives(script *models.Script, assetRoot string) (string, error) { + if !assetloader.HasEmbedDirectives(script.SQL) { return script.SQL, nil } - return assetloader.ProcessEmbedDirectives(sqlPath, script.SQL) + if assetRoot == "" { + return "", fmt.Errorf("asset_root is required to resolve embed directives from a model; source_path metadata does not authorize file access") + } + sqlPath, _ := script.Metadata[assetloader.ScriptSourcePathMetadataKey].(string) + if sqlPath == "" { + return "", fmt.Errorf("source_path is required for model embed directives") + } + return assetloader.ProcessEmbedDirectivesWithOptions(sqlPath, script.SQL, assetloader.EmbedOptions{AssetRoot: assetRoot}) } diff --git a/pkg/writers/sqlexec/writer_test.go b/pkg/writers/sqlexec/writer_test.go index dce74fa..d7544f7 100644 --- a/pkg/writers/sqlexec/writer_test.go +++ b/pkg/writers/sqlexec/writer_test.go @@ -232,7 +232,7 @@ func TestProcessEmbedDirectives_UsesScriptSourcePath(t *testing.T) { script.SQL = "-- @embed: path=body.txt var=:body mode=text\nSELECT :body;" script.Metadata[assetloader.ScriptSourcePathMetadataKey] = sqlPath - got, err := processEmbedDirectives(script) + got, err := processEmbedDirectives(script, dir) if err != nil { t.Fatalf("processEmbedDirectives failed: %v", err) } @@ -241,15 +241,28 @@ func TestProcessEmbedDirectives_UsesScriptSourcePath(t *testing.T) { } } -func TestProcessEmbedDirectives_NoSourcePathLeavesSQLUnchanged(t *testing.T) { +func TestProcessEmbedDirectives_NoSourcePathCannotAuthorizeRead(t *testing.T) { script := models.InitScript("seed") script.SQL = "-- @embed: path=missing.txt var=:body mode=text\nSELECT :body;" - got, err := processEmbedDirectives(script) - if err != nil { - t.Fatalf("processEmbedDirectives failed: %v", err) + got, err := processEmbedDirectives(script, t.TempDir()) + if err == nil || got != "" { + t.Fatalf("expected missing source_path failure, got %q %v", got, err) } - if got != script.SQL { - t.Fatalf("expected unchanged SQL without source path, got:\n%s", got) +} + +func TestProcessEmbedDirectivesModelSourcePathIsNotAuthorization(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "secret.txt"), []byte("secret"), 0o600); err != nil { + t.Fatal(err) + } + script := models.InitScript("untrusted") + script.SQL = "-- @embed: path=secret.txt var=:body mode=text\nSELECT :body;" + script.Metadata[assetloader.ScriptSourcePathMetadataKey] = filepath.Join(dir, "fake.sql") + if _, err := processEmbedDirectives(script, ""); err == nil || !strings.Contains(err.Error(), "asset_root is required") { + t.Fatalf("source metadata must not authorize file read: %v", err) + } + if _, err := processEmbedDirectives(script, t.TempDir()); err == nil { + t.Fatal("source path outside trusted root must fail") } } diff --git a/pkg/writers/template/README.md b/pkg/writers/template/README.md index f3785b8..3546977 100644 --- a/pkg/writers/template/README.md +++ b/pkg/writers/template/README.md @@ -168,15 +168,22 @@ relspec templ --from json --from-path schema.json --template test.tmpl ## Multi-file Output -For multi-file modes, the writer: +For multi-file modes, `OutputPath` is always the output directory, including when +it does not yet exist or has no trailing slash. An empty path uses the current +directory. Database mode continues to treat `OutputPath` as one explicit file. -1. Iterates through items (schemas/scripts/tables) -2. Creates `TemplateData` for each item -3. Executes template with item data -4. Generates filename using `filename_pattern` template -5. Writes output to generated filename +The writer renders and validates all filenames and contents before writing. Names +must be relative, may contain safe subdirectories, and cannot contain traversal, +absolute/drive/UNC paths, backslashes, Windows device names, alternate streams, +control characters, or trailing dots/spaces. Duplicate filenames (including case +variants) and file/directory collisions fail before any artifact is replaced. -Output directory is created automatically if it doesn't exist. +Output directories are created automatically. Existing symlink roots, parents, and +files are rejected. Writes and renames use `os.Root` to retain containment if paths +change during generation. Each file is staged exclusively, closed successfully, +and renamed into place; existing permissions are preserved on replacement. +Multi-file publication is per file, so an I/O error partway through finalization can +still leave a mixed generation (the broader audit A16 staging/manifest work remains). ## Filename Pattern Execution diff --git a/pkg/writers/template/writer.go b/pkg/writers/template/writer.go index 4a38ddb..ef158dc 100644 --- a/pkg/writers/template/writer.go +++ b/pkg/writers/template/writer.go @@ -2,6 +2,9 @@ package template import ( "bytes" + "crypto/rand" + "encoding/hex" + "errors" "fmt" "os" "path/filepath" @@ -9,6 +12,7 @@ import ( "text/template" "git.warky.dev/wdevs/relspecgo/pkg/models" + "git.warky.dev/wdevs/relspecgo/pkg/safefile" "git.warky.dev/wdevs/relspecgo/pkg/writers" ) @@ -125,96 +129,132 @@ func (w *Writer) executeDatabaseMode(db *models.Database) error { return w.writeOutput(output, w.options.OutputPath) } -// executeSchemaMode executes the template once per schema +// Multi-file generation renders and validates every artifact before touching output. func (w *Writer) executeSchemaMode(db *models.Database) error { + var data []*TemplateData for _, schema := range db.Schemas { - data := NewSchemaData(schema, w.options.Metadata) - output, err := w.executeTemplate(data) - if err != nil { - return fmt.Errorf("failed to execute template for schema %s: %w", schema.Name, err) - } - - filename, err := w.generateFilename(data) - if err != nil { - return fmt.Errorf("failed to generate filename for schema %s: %w", schema.Name, err) - } - - if err := w.writeOutput(output, filename); err != nil { - return fmt.Errorf("failed to write output for schema %s: %w", schema.Name, err) - } + data = append(data, NewSchemaData(schema, w.options.Metadata)) } - - return nil + return w.executeFiles(data) } - -// executeDomainMode executes the template once per domain func (w *Writer) executeDomainMode(db *models.Database) error { + var data []*TemplateData for _, domain := range db.Domains { - data := NewDomainData(domain, db, w.options.Metadata) - output, err := w.executeTemplate(data) - if err != nil { - return fmt.Errorf("failed to execute template for domain %s: %w", domain.Name, err) - } - - filename, err := w.generateFilename(data) - if err != nil { - return fmt.Errorf("failed to generate filename for domain %s: %w", domain.Name, err) - } - - if err := w.writeOutput(output, filename); err != nil { - return fmt.Errorf("failed to write output for domain %s: %w", domain.Name, err) - } + data = append(data, NewDomainData(domain, db, w.options.Metadata)) } - - return nil + return w.executeFiles(data) } - -// executeScriptMode executes the template once per script func (w *Writer) executeScriptMode(db *models.Database) error { + var data []*TemplateData for _, schema := range db.Schemas { for _, script := range schema.Scripts { - data := NewScriptData(script, schema, db, w.options.Metadata) - output, err := w.executeTemplate(data) - if err != nil { - return fmt.Errorf("failed to execute template for script %s: %w", script.Name, err) - } + data = append(data, NewScriptData(script, schema, db, w.options.Metadata)) + } + } + return w.executeFiles(data) +} +func (w *Writer) executeTableMode(db *models.Database) error { + var data []*TemplateData + for _, schema := range db.Schemas { + for _, table := range schema.Tables { + data = append(data, NewTableData(table, schema, db, w.options.Metadata)) + } + } + return w.executeFiles(data) +} - filename, err := w.generateFilename(data) - if err != nil { - return fmt.Errorf("failed to generate filename for script %s: %w", script.Name, err) - } +type artifact struct{ name, content string } - if err := w.writeOutput(output, filename); err != nil { - return fmt.Errorf("failed to write output for script %s: %w", script.Name, err) +func (w *Writer) outputRoot() string { + if w.options.OutputPath == "" { + return "." + } + return w.options.OutputPath +} + +func (w *Writer) executeFiles(data []*TemplateData) error { + var files []artifact + seen := make(map[string]bool) + for _, item := range data { + name, err := w.renderFilename(item) + if err != nil { + return err + } + key := strings.ToLower(filepath.ToSlash(name)) + if seen[key] { + return fmt.Errorf("generated filename collision: %q", name) + } + seen[key] = true + content, err := w.executeTemplate(item) + if err != nil { + return err + } + files = append(files, artifact{name, content}) + } + // Reject file/directory conflicts before creating any output. + for name := range seen { + for parent := filepath.ToSlash(filepath.Dir(name)); parent != "."; parent = filepath.ToSlash(filepath.Dir(parent)) { + if seen[parent] { + return fmt.Errorf("generated file/directory collision: %q", name) } } } - + if len(files) == 0 { + return nil + } + rootPath := w.outputRoot() + if err := safefile.CheckRoot(rootPath); err != nil { + return err + } + if err := os.MkdirAll(rootPath, 0o755); err != nil { + return err + } + root, err := os.OpenRoot(rootPath) + if err != nil { + return err + } + defer root.Close() + // Preflight every destination, including existing directory and symlink conflicts. + for _, file := range files { + if err := safefile.Check(root, file.name); err != nil { + return err + } + } + for _, file := range files { + if err := w.writeArtifact(root, file); err != nil { + return fmt.Errorf("write generated file %q: %w", file.name, err) + } + } return nil } -// executeTableMode executes the template once per table -func (w *Writer) executeTableMode(db *models.Database) error { - for _, schema := range db.Schemas { - for _, table := range schema.Tables { - data := NewTableData(table, schema, db, w.options.Metadata) - output, err := w.executeTemplate(data) - if err != nil { - return fmt.Errorf("failed to execute template for table %s.%s: %w", schema.Name, table.Name, err) - } - - filename, err := w.generateFilename(data) - if err != nil { - return fmt.Errorf("failed to generate filename for table %s.%s: %w", schema.Name, table.Name, err) - } - - if err := w.writeOutput(output, filename); err != nil { - return fmt.Errorf("failed to write output for table %s.%s: %w", schema.Name, table.Name, err) - } - } +func (w *Writer) writeArtifact(root *os.Root, file artifact) error { + if err := root.MkdirAll(filepath.Dir(file.name), 0o755); err != nil { + return err } - - return nil + // Exclusive unpredictable staging avoids truncating a destination (or a link target). + var token [16]byte + if _, err := rand.Read(token[:]); err != nil { + return err + } + temporary := ".relspec-" + hex.EncodeToString(token[:]) + ".tmp" + mode := os.FileMode(0o644) + if info, err := root.Lstat(file.name); err == nil && info.Mode().IsRegular() { + mode = info.Mode().Perm() + } + staged, err := root.OpenFile(temporary, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode) + if err != nil { + return err + } + defer root.Remove(temporary) + _, writeErr := staged.WriteString(file.content) + if err := errors.Join(writeErr, staged.Close()); err != nil { + return err + } + if err := safefile.Check(root, file.name); err != nil { + return err + } + return root.Rename(temporary, file.name) } // loadTemplate loads and parses the template file @@ -245,43 +285,25 @@ func (w *Writer) executeTemplate(data *TemplateData) (string, error) { return buf.String(), nil } -// generateFilename generates a filename from the filename pattern -func (w *Writer) generateFilename(data *TemplateData) (string, error) { - // Parse filename pattern as a template +// renderFilename returns a validated path relative to the explicit output root. +func (w *Writer) renderFilename(data *TemplateData) (string, error) { tmpl, err := template.New("filename").Funcs(w.funcMap).Parse(w.filenamePattern) if err != nil { return "", fmt.Errorf("invalid filename pattern: %w", err) } - - // Execute filename template var buf bytes.Buffer if err := tmpl.Execute(&buf, data); err != nil { return "", fmt.Errorf("failed to generate filename: %w", err) } + return safefile.Relative(buf.String()) +} - filename := buf.String() - - // If output path is a directory, join with generated filename - if w.options.OutputPath != "" { - // Check if output path is a directory - info, err := os.Stat(w.options.OutputPath) - if err == nil && info.IsDir() { - filename = filepath.Join(w.options.OutputPath, filename) - } else { - // If it doesn't exist, check if it looks like a directory (ends with /) - if strings.HasSuffix(w.options.OutputPath, string(filepath.Separator)) { - filename = filepath.Join(w.options.OutputPath, filename) - } else { - // Use output path as base directory - dir := filepath.Dir(w.options.OutputPath) - if dir != "." { - filename = filepath.Join(dir, filename) - } - } - } +func (w *Writer) generateFilename(data *TemplateData) (string, error) { + name, err := w.renderFilename(data) + if err != nil { + return "", err } - - return filename, nil + return filepath.Join(w.outputRoot(), name), nil } // writeOutput writes the output to a file or stdout diff --git a/pkg/writers/template/writer_modes_test.go b/pkg/writers/template/writer_modes_test.go index d1fb2f5..cbf2f74 100644 --- a/pkg/writers/template/writer_modes_test.go +++ b/pkg/writers/template/writer_modes_test.go @@ -180,8 +180,8 @@ func TestWriterGenerateFilenameOutputPathForms(t *testing.T) { {"no output path", "", "users.txt"}, {"existing dir", dir, filepath.Join(dir, "users.txt")}, {"trailing separator", filepath.Join(dir, "new") + string(filepath.Separator), filepath.Join(dir, "new", "users.txt")}, - {"file path uses its dir", filepath.Join(dir, "x.out"), filepath.Join(dir, "users.txt")}, - {"bare file name", "x.out", "users.txt"}, + {"nonexistent path is root", filepath.Join(dir, "x.out"), filepath.Join(dir, "x.out", "users.txt")}, + {"bare path is root", "x.out", filepath.Join("x.out", "users.txt")}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -217,3 +217,123 @@ func TestWriterWriteSchemaAndTable(t *testing.T) { t.Errorf("WriteTable: %q", data) } } + +func TestMultiFileRejectsUnsafeFilenamesBeforeWriting(t *testing.T) { + for _, name := range []string{"../escape", "/absolute", `..\escape`, `C:\escape`, `\\server\share\file`, "a/../escape", "a//escape", "CON", "nul.txt", "COM¹.txt", "file:stream", "trailing.", "trailing ", "line\nbreak"} { + t.Run(name, func(t *testing.T) { + out := filepath.Join(t.TempDir(), "new-output") + w, err := newTestWriter(t, "content", "schema", "{{.Name}}", out) + if err != nil { + t.Fatal(err) + } + db := models.InitDatabase("unsafe") + db.Schemas = []*models.Schema{models.InitSchema("valid"), models.InitSchema(name)} + if err := w.WriteDatabase(db); err == nil { + t.Fatal("expected rejected filename") + } + if _, err := os.Stat(out); !os.IsNotExist(err) { + t.Fatalf("validation failure must not create output: %v", err) + } + }) + } +} + +func TestMultiFileCollisionsDoNotOverwrite(t *testing.T) { + for _, names := range [][]string{{"same.txt", "same.txt"}, {"SAME.txt", "same.txt"}, {"file", "file/child.txt"}} { + out := t.TempDir() + seed := filepath.Join(out, "same.txt") + if err := os.WriteFile(seed, []byte("original"), 0o600); err != nil { + t.Fatal(err) + } + w, err := newTestWriter(t, "replacement", "schema", "{{.Name}}", out) + if err != nil { + t.Fatal(err) + } + db := models.InitDatabase("collision") + for _, name := range names { + db.Schemas = append(db.Schemas, models.InitSchema(name)) + } + if err := w.WriteDatabase(db); err == nil || !strings.Contains(err.Error(), "collision") { + t.Fatalf("expected collision error: %v", err) + } + got, err := os.ReadFile(seed) + if err != nil || string(got) != "original" { + t.Fatalf("existing output changed: %q %v", got, err) + } + } +} + +func TestMultiFileRejectsSymlinksBeforeAnyWrites(t *testing.T) { + for _, kind := range []string{"parent", "leaf", "root"} { + t.Run(kind, func(t *testing.T) { + out, external := t.TempDir(), t.TempDir() + secret := filepath.Join(external, "secret.txt") + if err := os.WriteFile(secret, []byte("original"), 0o600); err != nil { + t.Fatal(err) + } + link, target := filepath.Join(out, "link"), external + pattern := "{{.Name}}/secret.txt" + if kind == "leaf" { + target = secret + pattern = "{{.Name}}" + } + if kind == "root" { + link = filepath.Join(out, "root") + out = link + pattern = "{{.Name}}" + } + if err := os.Symlink(target, link); err != nil { + t.Skip(err) + } + w, err := newTestWriter(t, "replacement", "schema", pattern, out) + if err != nil { + t.Fatal(err) + } + db := models.InitDatabase("links") + db.Schemas = []*models.Schema{models.InitSchema("first"), models.InitSchema("link")} + if err := w.WriteDatabase(db); err == nil { + t.Fatal("expected symlink rejection") + } + got, err := os.ReadFile(secret) + if err != nil || string(got) != "original" { + t.Fatalf("symlink target changed: %q %v", got, err) + } + if _, err := os.Stat(filepath.Join(out, "first")); !os.IsNotExist(err) { + t.Fatalf("preflight must precede writes: %v", err) + } + }) + } +} + +func TestMultiFileNewDirectoryAndNestedFiles(t *testing.T) { + out := filepath.Join(t.TempDir(), "new-output") + w, err := newTestWriter(t, "{{.Table.Name}}", "table", "{{.ParentSchema.Name}}/{{.Name}}.txt", out) + if err != nil { + t.Fatal(err) + } + if err := w.WriteDatabase(modeDB()); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(filepath.Join(out, "a", "t1.txt")) + if err != nil || string(got) != "t1" { + t.Fatalf("nested output: %q %v", got, err) + } +} + +func TestMultiFileRootAncestorSymlinkRejected(t *testing.T) { + parent, external := t.TempDir(), t.TempDir() + link := filepath.Join(parent, "link") + if err := os.Symlink(external, link); err != nil { + t.Skip(err) + } + w, err := newTestWriter(t, "content", "schema", "{{.Name}}.txt", filepath.Join(link, "new")) + if err != nil { + t.Fatal(err) + } + if err := w.WriteDatabase(modeDB()); err == nil { + t.Fatal("root ancestor symlink must fail") + } + if _, err := os.Stat(filepath.Join(external, "new")); !os.IsNotExist(err) { + t.Fatalf("must not create external directories: %v", err) + } +} diff --git a/pkg/writers/writer.go b/pkg/writers/writer.go index eb96f70..ff8caca 100644 --- a/pkg/writers/writer.go +++ b/pkg/writers/writer.go @@ -67,6 +67,9 @@ type WriterOptions struct { // Useful for databases like SQLite that do not support schemas. FlattenSchema bool + // LowercaseIdentifiers lowercases PostgreSQL names and structured references before generation. + LowercaseIdentifiers bool + // NullableTypes selects the Go type package used for nullable columns in // code-generation writers (bun, gorm). Accepted values: // "sqltypes" — git.warky.dev/wdevs/relspecgo/pkg/sqltypes