fix: address audit execution failures and unsafe type conversions

This commit is contained in:
2026-10-12 19:53:19 +02:00
parent bfe9852826
commit 2fa48335d3
29 changed files with 1630 additions and 421 deletions
+1
View File
@@ -68,6 +68,7 @@ func init() {
batchCmd.Flags().StringVar(&batchNullableTypes, "types", "", "Nullable type package for code-gen writers (bun/gorm)")
batchCmd.Flags().StringVar(&batchNullableArrays, "array-nullable", "", "Nullable array representation for the Bun writer")
batchCmd.Flags().BoolVar(&batchContinueOnError, "continue-on-error", false, "Prepend \\set ON_ERROR_STOP off to generated SQL (pgsql output only)")
batchCmd.Flags().BoolVar(&batchContinueOnError, "coe", false, "Alias for --continue-on-error")
batchCmd.Flags().BoolVar(&batchKeepGoing, "keep-going", false, "Process remaining inputs after a failure; still exits non-zero")
batchCmd.Flags().BoolVar(&batchDryRun, "dry-run", false, "Read and validate every input and print the plan without writing any output")
+1
View File
@@ -193,6 +193,7 @@ func init() {
convertCmd.Flags().StringVar(&convertNullableTypes, "types", "", "Nullable type package for code-gen writers (bun/gorm): 'baselib' (default, Go pointer types), 'stdlib' (database/sql), or 'sqltypes'")
convertCmd.Flags().StringVar(&convertNullableArrays, "array-nullable", "", "Nullable PostgreSQL array representation for the Bun writer in stdlib/baselib --types mode: 'slice' (default, plain slice) or 'pointer_slice' (*[]T, distinguishes NULL from '{}')")
convertCmd.Flags().BoolVar(&convertContinueOnError, "continue-on-error", false, "Prepend \\set ON_ERROR_STOP off to generated SQL so psql continues past errors (pgsql output only)")
convertCmd.Flags().BoolVar(&convertContinueOnError, "coe", false, "Alias for --continue-on-error")
convertCmd.Flags().StringVar(&convertExtraFields, "extra-fields", "", "Path to JSON file containing extra Bun model fields to inject (bun output only); fields support target_table, name, type, bun_tag, json_tag, comment")
convertCmd.Flags().BoolVar(&convertDryRun, "dry-run", false, "Read and validate the input and print the plan without writing any output")
+4 -1
View File
@@ -777,6 +777,8 @@ func runScriptsExecJob(rj *resolvedJob, lg *jobLogger) error {
lg.logf("executing %d script(s) against database env:%s", len(combined.Scripts), rj.outputConnEnv)
writer := sqlexec.NewWriter(&writers.WriterOptions{
ContinueOnError: rj.job.Options.ContinueOnError,
AtomicExecution: rj.job.Options.AtomicExecution,
Metadata: map[string]any{
"connection_string": rj.outputConn,
"ignore_errors": rj.job.Options.ContinueOnError,
@@ -791,7 +793,7 @@ func runScriptsExecJob(rj *resolvedJob, lg *jobLogger) error {
success, _ := opts.Metadata["execution_success"].(int)
failed, _ := opts.Metadata["execution_failed"].(int)
lg.logf("executed %d script(s): %d succeeded, %d failed", total, success, failed)
if failed > 0 && !rj.job.Options.ContinueOnError {
if failed > 0 {
return fmt.Errorf("%d script(s) failed", failed)
}
return nil
@@ -844,6 +846,7 @@ func writeJobOutput(rj *resolvedJob, db *models.Database, lg *jobLogger) error {
}
lg.logf("writing output to database env:%s", rj.outputConnEnv)
writerOpts := newWriterOptions("", o.Package, o.FlattenSchema, o.Types, o.ArrayNullable, o.ContinueOnError)
writerOpts.AtomicExecution = o.AtomicExecution
writerOpts.Metadata = map[string]interface{}{"connection_string": rj.outputConn, "full_ddl": o.FullDDL}
return wpgsql.NewWriter(writerOpts).WriteDatabase(db)
}
+105
View File
@@ -2,11 +2,15 @@ package main
import (
"bytes"
"context"
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"time"
"github.com/jackc/pgx/v5"
"github.com/spf13/cobra"
"git.warky.dev/wdevs/relspecgo/pkg/jobs"
@@ -741,3 +745,104 @@ jobs:
t.Fatalf("expected templ format rejection, got %v", err)
}
}
func TestLive_JobPostgresFailureStopsDependents(t *testing.T) {
connString := os.Getenv("RELSPEC_TEST_PG_CONN")
if connString == "" {
t.Skip("RELSPEC_TEST_PG_CONN not set")
}
t.Setenv("RELSPEC_TEST_JOB_OUTPUT_CONN", connString)
for _, continueOnError := range []string{"false", "true"} {
t.Run("continue="+continueOnError, func(t *testing.T) {
manifest := `version: 1
jobs:
apply:
command: convert
inputs:
- path: schema/core.dbml
format: dbml
output:
format: pgsql
conn_env: RELSPEC_TEST_JOB_OUTPUT_CONN
options:
full_ddl: true
continue_on_error: ` + continueOnError + `
logfile: .relspec/apply.log
dependent:
command: convert
depends_on: [apply]
inputs:
- path: schema/core.dbml
format: dbml
output:
format: json
path: build/dependent.json
logfile: .relspec/dependent.log
`
dir := jobFixture(t, manifest)
schema := fmt.Sprintf("job_audit_%d", time.Now().UnixNano())
conn, err := pgx.Connect(context.Background(), connString)
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() {
_, _ = conn.Exec(context.Background(), "DROP SCHEMA IF EXISTS "+schema+" CASCADE")
_ = conn.Close(context.Background())
})
// An invalid column type prevents table creation in this isolated schema.
writeFile(t, filepath.Join(dir, "schema", "core.dbml"), "Table "+schema+".users {\n id no_such_type_audit_xyz\n}\n")
set := mustLoadSet(t, filepath.Join(dir, "relspec.yml"))
err = executeJobPlan(set, "dependent", false, false, &bytes.Buffer{})
if err == nil || !strings.Contains(err.Error(), `job "apply" failed`) {
t.Fatalf("expected database failure propagation, got %v", err)
}
for _, path := range []string{"build/dependent.json", ".relspec/dependent.log"} {
if _, err := os.Stat(filepath.Join(dir, path)); !os.IsNotExist(err) {
t.Fatalf("dependent must not start: path=%s err=%v", path, err)
}
}
logData, err := os.ReadFile(filepath.Join(dir, ".relspec/apply.log"))
if err != nil || !strings.Contains(string(logData), "FAILED") {
t.Fatalf("failed application must be logged: %s err=%v", logData, err)
}
})
}
}
func TestLive_JobScriptContinuationStillStopsDependents(t *testing.T) {
connection := os.Getenv("RELSPEC_TEST_PG_CONN")
if connection == "" {
t.Skip("RELSPEC_TEST_PG_CONN not set")
}
t.Setenv("RELSPEC_TEST_EXEC_CONN", connection)
dir := jobFixture(t, `version: 1
jobs:
migrate:
command: scripts-exec
script_dirs: [migrations]
output:
conn_env: RELSPEC_TEST_EXEC_CONN
options:
continue_on_error: true
logfile: .relspec/migrate.log
dependent:
command: convert
depends_on: [migrate]
inputs:
- path: schema/core.dbml
format: dbml
output:
format: json
path: build/dependent.json
`)
writeFile(t, filepath.Join(dir, "migrations", "1_001_bad.sql"), "SELECT 1 / 0;\n")
writeFile(t, filepath.Join(dir, "migrations", "1_002_good.sql"), "SELECT 1;\n")
set := mustLoadSet(t, filepath.Join(dir, "relspec.yml"))
err := executeJobPlan(set, "dependent", false, false, &bytes.Buffer{})
if err == nil || !strings.Contains(err.Error(), `job "migrate" failed`) {
t.Fatalf("failed scripts must fail job: %v", err)
}
if _, err := os.Stat(filepath.Join(dir, "build", "dependent.json")); !os.IsNotExist(err) {
t.Fatalf("dependent ran: %v", err)
}
}
+3 -1
View File
@@ -63,7 +63,7 @@ var scriptsExecuteCmd = &cobra.Command{
Long: `Execute SQL scripts from a directory against a PostgreSQL database.
Scripts are executed in order: Priority (ascending), Sequence (ascending), Name (alphabetical).
By default, execution stops immediately on the first error. Use --ignore-errors to continue execution.
By default, execution stops immediately on the first error. Use --ignore-errors (or --continue-on-error / --coe) to attempt later scripts; any failure still exits nonzero.
The directory is scanned recursively for all subdirectories and files matching the patterns:
{priority}_{sequence}_{name}.sql or .pgsql (underscore format)
@@ -112,6 +112,8 @@ func init() {
scriptsExecuteCmd.Flags().StringVar(&scriptsSchemaName, "schema", "public", "Schema name (optional, default: public)")
scriptsExecuteCmd.Flags().StringVar(&scriptsDBName, "database", "database", "Database name (optional, default: database)")
scriptsExecuteCmd.Flags().BoolVar(&scriptsIgnoreErrors, "ignore-errors", false, "Continue executing scripts even if errors occur")
scriptsExecuteCmd.Flags().BoolVar(&scriptsIgnoreErrors, "continue-on-error", false, "Attempt later scripts after failures; still exit nonzero")
scriptsExecuteCmd.Flags().BoolVar(&scriptsIgnoreErrors, "coe", false, "Alias for --continue-on-error")
err = scriptsExecuteCmd.MarkFlagRequired("dir")
if err != nil {
+334
View File
@@ -0,0 +1,334 @@
# Project audit 1
Date: 2026-10-12
Revision reviewed: `bfe98528265b45be7bfdcdf025710981bae06a7b`
Perspectives: software engineering, security, maintenance, and databases.
## Repair progress
Implementation batches (2026-10-12):
- [x] **A01, PostgreSQL failure propagation:** stop on the first SQL failure by default; explicit continuation still returns a structured `ExecutionFailure`. Reports include skipped entries, and requested report write/close failures are errors. Live tests cover a middle-statement failure in both modes and verify dependent jobs do not start.
- [x] **A02, implicit replacement removed:** both full-DDL and diff type changes propagate conversion errors without renaming/replacing the column. Live populated-data tests verify original values, column count/type, unique-index identity, and constraint enforcement survive incompatible conversions.
- [x] **Related defect exposed by A01:** full-DDL column additions now use native `ADD COLUMN IF NOT EXISTS`, fixing duplicate-column errors for flattened output that previously appeared successful.
- [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.
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
RelSpec has a useful architecture, broad format support, and substantial tests. Its most serious weaknesses are in the boundary between generating a schema and safely applying that schema to a real database. A successful process exit does not reliably mean a successful migration. Some conversions silently discard database semantics, and several file/SQL generation paths do not preserve the safety guarantees implemented elsewhere.
**Recommendation: fix the high-priority findings before relying on unattended production database writes or processing externally supplied specifications.** Continue using generated output with review and disposable database validation while these issues are addressed. This is a repository audit, not a certification of production readiness.
### Existing strengths
- Readers, writers, the intermediate model, diffing, inspection, and transformations are separated into packages.
- There are many unit and regression tests, including live PostgreSQL writer tests and SQLite tests.
- Job files have semantic validation, dependency-cycle detection, environment-based connection references, path containment checks, and symlink checks.
- Single-file job conversions already use temporary output followed by rename.
- The asset manifest executor binds file bytes as database parameters.
- The repository includes a private vulnerability reporting policy, vendored dependencies, and release-time vet/static analysis/vulnerability-check steps.
These controls should be extended and made consistent, rather than replaced wholesale.
## Scope and verification
Reviewed the CLI, job handling, database readers/writers, PostgreSQL templates and migration generation, asset embedding, generic templates, connection builder, updater, build/release workflows, documentation, and the VS Code extension. Review depth was greatest for database mutation and input/output trust boundaries; every parser and format combination was not exhaustively audited.
Checks performed:
| Check | Result |
| --- | --- |
| `GOCACHE=/tmp/relspec-audit-gocache go test -mod=vendor ./...` | Initial sandbox run failed because HTTP tests could not open loopback sockets. Approved rerun outside the sandbox passed. |
| `GOCACHE=/tmp/relspec-audit-gocache go vet -mod=vendor ./...` | Passed. |
| `make fmt-check GOLANGCI_LINT=false` | Incorrectly passed, confirming that a failed formatter can be hidden. |
| Temporary Go reproduction using exported package APIs | Confirmed template path escape, embed path escape, embed dollar corruption, invalid PostgreSQL identifier/literal rendering, omitted MySQL FK/index output, and placeholder matching inside SQL strings. |
The local compiler reported `go1.27.1-X:nodwarf5 linux/amd64`; `go.mod` requires `1.26.9`. This was not a verification under the declared minimum toolchain. No live PostgreSQL, MySQL, or SQL Server connection was configured for the audit. Tests requiring those connections skipped; a passing package does not demonstrate live database correctness. Race testing, fuzz campaigns, benchmarks, `staticcheck`, and a fresh `govulncheck` scan were not performed. No claims about specific dependency CVEs are made.
Reproduction files and logs were placed in `/tmp`; the only intended repository change is this document. No real credentials or user data were used in reproductions. Database injection and migration effects were assessed from code/generated SQL, not executed against a database.
### Priority and evidence notation
- **P1 / High:** address before unattended database use or handling less-trusted inputs; credible data-integrity, confidentiality, or integrity impact.
- **P2 / Medium:** schedule in the next maintenance cycle; operational reliability, reproducibility, or defense-in-depth issue.
- **P3 / Low:** documentation or developer-experience issue.
- **Reproduced:** a local executable check demonstrated the stated behavior.
- **Code-confirmed:** the implementation establishes the behavior, but the complete external-system scenario was not executed.
## Findings
### A01 — PostgreSQL writes report success after statement failures
**Repair status: addressed.** The shared executor now enforces stop/continue behavior and nonzero failure across PostgreSQL, SQLite, SQL Server, and raw scripts, including requested report errors. Reports include individual outcomes, skipped operations, confirmed rollbacks, and uncertain finalization. Atomic generated-DDL runs are optional; raw scripts explicitly report script-managed execution. A09 remains a separate migration-management/concurrency contract.
**P1 / High · Engineering, database · Code-confirmed**
Evidence: `pkg/writers/pgsql/writer.go:2200–2322`, especially `executeStatements` and `finishReport`; `pkg/writers/pgsql/live_execute_test.go:144` explicitly tests continuing after failure.
Every failed statement is recorded and execution continues. `finishReport` prints an error count but returns `nil`. Failure to write the requested report is also only a warning. This behavior is unconditional in this execution path. A deployment or dependent job can proceed after an incomplete schema change because the writer returned success.
**Fix:** return a structured error when any statement fails; stop on the first error by default. A continue-on-error option should control continuation, separately from the final exit status. Aggregate results consistently across PostgreSQL, SQLite, SQL Server, and script execution. Treat failure to produce an explicitly requested execution report as an error.
**Acceptance:** force a middle statement to fail; the CLI exits nonzero, dependent jobs do not run, and the report distinguishes applied, failed, skipped, and rolled-back operations. Test both stop and continue modes.
### A02 — Failed type conversion silently substitutes an empty column
**Repair status: addressed.** The shared type-change template propagates conversion errors. Configurable `pgsql_using` expressions and `pgsql_preflight` predicates are supported, with automatic whole-column cast evaluation before schema mutations. Full-DDL and diff live tests verify preservation of populated columns and unique dependencies, custom conversions, and rejected predicates. Preflight is advisory under concurrent writes; atomic generated-DDL execution is available for transactionally supported changes, and separately reviewed rename/replacement remains required.
**P1 / High · Database · Code-confirmed**
Evidence: `pkg/writers/pgsql/templates/alter_column_type_with_check.tmpl:18–27`; `pkg/writers/pgsql/migration_writer.go:491`; `pkg/writers/pgsql/writer.go:514`.
The type-change block catches `WHEN OTHERS`, renames the existing column, and adds a fresh column with the original name. For example, converting populated text values containing non-numeric data to an integer can fail the cast and activate this fallback. The application then sees an empty replacement column. Existing indexes, constraints, and dependencies may remain attached to the renamed column. Original values are not necessarily deleted, but the application's expected column no longer contains them.
The broad exception handler also treats unrelated failures as permission to attempt this schema rewrite.
**Fix:** fail on conversion errors. Require a separately reviewed migration for any rename-and-replace operation, including backfill, validation, dependency handling, and recovery. Support an explicit conversion expression and preflight checks where appropriate.
**Acceptance:** migrating populated incompatible data leaves the original column, values, and dependencies intact and returns an actionable failure. No rename/replacement occurs implicitly.
### A03 — Generated template filenames escape the output directory
**P1 / High · Security · Reproduced**
Evidence: `pkg/writers/template/writer.go:249–308`.
The default filename pattern uses `{{.Name}}.txt`. `generateFilename` joins the rendered name with the destination without checking containment; `writeOutput` creates directories and writes the resulting path. A schema named `../escaped` wrote `escaped.txt` beside the selected output directory in the reproduction.
An imported model can therefore cause writes outside the intended destination, including overwriting files writable by the current user. Job-level checks validate the declared output path, not every subsequently generated filename. Exploitation requires the user to process the supplied model in a multi-file template mode.
**Fix:** validate every rendered filename against a clearly defined output root; reject traversal, unsafe absolute paths, and symlink escapes. Detect filename collisions before writing. Prefer root-scoped filesystem operations so validation and opening cannot be separated by a symlink replacement.
**Acceptance:** malicious names, filename patterns, symlink parents, duplicate names, and platform-specific paths cannot write outside the root or silently overwrite another generated artifact.
### A04 — SQL embed directives can read outside the script directory
**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`.
Embed paths are cleaned and joined to the script directory, then passed directly to `os.ReadFile`. A directive using `path=../synthetic-secret.txt` successfully embedded a sibling file. There is no equivalent root or symlink policy for this path. Script metadata also supplies the source path used for resolution.
A supplied script can cause local readable files to be copied into generated SQL or sent to the destination database. Permission to execute SQL against a database should not implicitly grant unrestricted local-file access. This is a local input trust-boundary issue, not an unauthenticated network endpoint.
**Fix:** define an explicit asset root, contain all embed reads within it, check symlink resolution, limit file size, and document any opt-in allowance for external files. Reuse the policy across manifest assets and embed directives.
**Acceptance:** traversal and symlink escape fail before reading; valid in-root text and binary assets still work; diagnostics do not print file contents.
### A05 — PostgreSQL SQL quoting is incomplete across syntactic contexts
**P1 / High · Security, database · Reproduced rendering defects**
Evidence: `pkg/writers/pgsql/template_functions.go:134`; `pkg/writers/pgsql/templates/add_column_with_check.tmpl:5–7`; `create_foreign_key_with_check.tmpl`; `alter_column_type_with_check.tmpl`.
Two separate defects were confirmed:
1. `quoteIdent` does not actually check reserved keywords, despite its comment. The template executor generated `CREATE TABLE IF NOT EXISTS public.order (select text);` for legal names requiring quoting.
2. Catalog lookups insert names directly into single-quoted SQL strings. A table named `O'Reilly` generated `table_name = 'O'Reilly'`. Quoting a name for an identifier elsewhere does not escape it as a string literal here.
Fixed `DO $$ ... $$` delimiters create another context that must be protected from embedded delimiter text. Besides breaking valid database names, insufficient context-aware quoting is an injection risk when less-trusted metadata is rendered and executed using privileged database credentials. A complete injection exploit was not executed during this audit. Raw SQL expressions are a separate, intentional executable-input boundary and should be documented as such.
**Fix:** centralize identifier, literal, qualified-name, and block-delimiter handling. Always quote identifiers where practical; parameterize values where supported. Generate collision-safe dollar delimiters or avoid interpolating arbitrary content into fixed blocks. Apply this across all templates, not only the examples above.
**Acceptance:** generated SQL executes correctly for reserved words, apostrophes, double quotes, Unicode, newlines, and dollar delimiters, and cannot introduce additional SQL statements through a name.
PostgreSQL documents the distinct identifier, string, and dollar-quote rules in [Lexical Structure](https://www.postgresql.org/docs/current/sql-syntax-lexical.html).
### A06 — MySQL constraint reading scans nullable metadata into strings
**P1 / High · Database · Code-confirmed; live MySQL not exercised**
Evidence: `pkg/readers/mysql/reader.go:181–220`, specifically the `rs`, `rt`, and `rc` string variables passed to `rows.Scan`.
The constraint query reads `REFERENCED_TABLE_SCHEMA`, `REFERENCED_TABLE_NAME`, and `REFERENCED_COLUMN_NAME` for primary and unique constraints as well as foreign keys. Non-foreign-key rows have no referenced object and can contain SQL NULL. Scanning these into Go strings fails, aborting inspection of ordinary tables with a primary key.
**Fix:** use nullable destinations such as `sql.NullString`, or explicit `COALESCE` where an empty string has the intended meaning. Review other nullable metadata, including expression-index column names, in the same reader.
**Acceptance:** live MySQL tests read tables with primary, unique, composite foreign-key, and expression-index definitions without scan failures and preserve their metadata. The current reader tests only cover type mapping and a missing connection string.
Reference: [MySQL KEY_COLUMN_USAGE documentation](https://dev.mysql.com/doc/refman/8.0/en/information-schema-key-column-usage-table.html) and [MySQL's documented non-FK NULL behavior](https://bugs.mysql.com/bug.php?id=9587).
### A07 — MySQL conversion silently loses constraints, indexes, and type detail
**P1 / High · Database · Reproduced omissions; additional losses code-confirmed**
Evidence: `pkg/writers/mysql/writer.go:96–163`; `pkg/readers/mysql/reader.go:153–178`; `pkg/mariadb/datatypes.go`.
The writer emits columns, primary keys, and unique constraints, but does not emit foreign keys, checks, or ordinary indexes from the model. A reproduction containing both a foreign key and an index returned success while emitting neither. The reader maps `COLUMN_TYPE` to a canonical base type without populating the separate length/precision/scale fields; its tests explicitly map `bigint unsigned` to `int64`.
Consequences include lost referential integrity, missing performance-critical indexes, changed ranges, and changed string/decimal definitions. Advertising a format as supported does not communicate these losses.
**Fix:** preserve and emit supported semantics; return explicit diagnostics for every unsupported object or attribute. Add a strict conversion mode that refuses lossy output and a per-format capability matrix. Preserve unsignedness, dimensions, generated expressions, and FK actions through the intermediate representation.
**Acceptance:** inspect → model → DDL → disposable database → inspect tests compare semantic equivalence for representative MySQL schemas. Unsupported features produce visible, testable failures or warnings, never silent success.
### A08 — Embed substitution corrupts dollar characters; placeholder parsing ignores SQL syntax
**P2 / Medium · Engineering, database · Reproduced**
Evidence: `pkg/assetloader/embed.go:159–160`; `pkg/assetloader/executor.go:32–87`.
`replaceEmbedPlaceholder` passes file content into the replacement argument of `regexp.ReplaceAllString`. Dollar characters are interpreted as capture references. Embedding `price $1 ${2} $HOME $$` produced `price ; $` in the local reproduction, rather than preserving the input.
Separately, `BuildQuery` matches placeholders inside quoted strings and comments. `SELECT ':literal', :bytes` fails with an unknown-placeholder error even though `:literal` is SQL string content. Similar text-based processing makes the MySQL executor's `strings.Split(..., ";\n")` fragile for multiline defaults containing that sequence.
**Fix:** use literal replacement callbacks or explicit span assembly for embed content. Parse SQL lexical contexts for named placeholders. Execute a structured list of generated statements instead of splitting SQL text on delimiters.
**Acceptance:** dollar text, repeated placeholders, quoted strings, comments, casts, dollar-quoted bodies, and embedded statement delimiters preserve exact intended semantics.
### A09 — Migration execution lacks a coherent atomicity and concurrency contract
**P1 / High · Database, engineering · Code-confirmed**
Evidence: `pkg/writers/pgsql/writer.go:2200–2300`; `pkg/writers/sqlexec/writer.go:107–222`; `pkg/writers/sqlite/writer.go:118–164`; `pkg/writers/mysql/writer.go:165–188`.
Executors issue independent statements/scripts without a transaction spanning the operation, migration-wide lock, or applied-script ledger in these paths. A failure can leave earlier work committed. Re-running script execution repeats supplied scripts; concurrent invocations can interleave. A script may contain its own transaction, but that does not establish a consistent executor-level guarantee.
The necessary behavior differs by database and statement type: wrapping all MySQL DDL in a transaction would not provide the same rollback guarantee as transactional PostgreSQL DDL.
**Fix:** define explicit execution modes with transaction boundaries, concurrency control, retry/recovery semantics, and accurate partial-application reports. If `scripts execute` remains a raw runner, document that clearly. If it is intended as a migration manager, add migration identity, checksums, applied history, and duplicate/concurrent-run protection.
**Acceptance:** failure injection and simultaneous-run tests establish predictable behavior; supported transactional runs roll back; nontransactional runs report exactly what committed and how to resume safely.
### A10 — Database operations cannot consistently inherit cancellation or deadlines
**P2 / Medium · Engineering, operations · Code-confirmed**
Evidence: `pkg/readers/reader.go`; `pkg/writers/writer.go`; `pkg/writers/pgsql/writer.go:2205`; `pkg/writers/sqlexec/writer.go`; `pkg/readers/mysql/reader.go:25`; `pkg/writers/mysql/writer.go:184`.
Reader/writer interfaces do not accept a caller context, and several implementations start with `context.Background()`. This disconnects CLI/job cancellation from connection establishment, catalog reads, and SQL execution. A blocked statement or lock wait has no consistent application-level deadline.
**Fix:** carry caller contexts through reader/writer operations; configure connection, statement, and lock-wait limits separately. Use a signal-aware CLI root context and preserve cancellation errors in job results.
**Acceptance:** a blocked database query terminates promptly on cancellation or timeout, cleanup is bounded, and the job exits nonzero without starting dependents.
### A11 — Windows updater executes downloads without artifact authentication
**P2 / Medium · Security, supply chain · Code-confirmed**
Evidence: `cmd/relspec/update.go:115–152`; `.gitea/workflows/release.yml`.
After confirmation or `--yes`, the updater downloads the asset URL from release metadata and starts the resulting executable. There is no signature verification, trusted-publisher check, content-size limit, or asset-origin/redirect restriction in this path. The default release API uses HTTPS, which is a useful transport control, but it is not independent artifact authentication. Compromised release metadata or publication infrastructure could substitute an installer.
**Fix:** sign installers or release manifests and verify against an independently trusted identity before launch. Require HTTPS throughout redirects, apply a deliberate origin policy, cap downloads, and clean up failed downloads. A checksum fetched from the same compromised endpoint alone is insufficient authentication.
**Acceptance:** tampered, unsigned, wrong-publisher, oversized, and disallowed-origin assets are rejected before execution.
### A12 — Connection and development-service defaults permit avoidable exposure
**P2 / Medium · Security · Code-confirmed**
Evidence: `pkg/ui/connstring.go:42–49`; `docker-compose.yml`; `Makefile`'s `docker-up` target.
The UI starts PostgreSQL and SQL Server connection fields with encryption disabled; changing the host does not change that setting. Compose publishes database ports without a loopback host restriction and supplies fixed development passwords. These are development credentials, not evidence of leaked production secrets, but the services can become reachable on host network interfaces.
**Fix:** default remote connections to authenticated encrypted transport and make local exceptions explicit. Bind development database ports to `127.0.0.1` by default; document how to deliberately expose a test service. Prefer a limited inspection role over administrative defaults.
**Acceptance:** changing a connection to a remote host does not silently retain an insecure default; standard local test startup only exposes loopback ports.
### A13 — CI does not consistently test the declared toolchain or live writer paths
**P1 / High · Maintenance, database assurance · Code-confirmed configuration defects**
Evidence: `.github/workflows/ci.yml:15,58,79`; `.github/workflows/integration-tests.yml:59–76`; `.gitea/workflows/release.yml:48–49`; `Makefile:92–108`; `tests/dbtest/dbs/mysql.sh`.
- GitHub CI specifies Go 1.24/1.25 while `go.mod` requires 1.26.9. Automatic toolchain switching can make that matrix misleading; with switching disabled those jobs cannot build the module as declared.
- The GitHub integration job runs directly on the host but connects to `postgres`, the service-container name, rather than the published loopback port. GitHub documents `localhost` for this job layout. This networking conclusion applies to GitHub-hosted execution; the Gitea runner configuration was not inspected.
- `make test-integration` runs the PostgreSQL reader and `tests/integration`, but omits live tests under `pkg/writers/pgsql` and `pkg/writers/sqlexec`. Release `go test ./...` has no configured database service/connection, so those tests can skip there too.
- MySQL has container scaffolding but no Go tests consuming `RELSPEC_TEST_MYSQL_CONN`; this leaves A06 undetected.
- `make test-unit` excludes the entire PostgreSQL reader package, including `queries_pure_test.go`.
**Fix:** use `go-version-file: go.mod`, explicitly test supported toolchains, correct GitHub service networking, and run all live readers and writers against disposable databases in required CI jobs. Make unexpected integration-test skips fail the integration job. Verify which workflows the actual Gitea deployment discovers; CI badges alone do not establish execution.
**Acceptance:** CI artifacts show real reader/writer execution, not just green packages with skipped live tests, and demonstrate MySQL constraint inspection and failing-migration behavior.
Reference: [GitHub PostgreSQL service-container networking](https://docs.github.com/en/actions/tutorials/use-containerized-services/create-postgresql-service-containers).
### A14 — Formatting gate ignores formatter failure
**P2 / Medium · Maintenance · Reproduced**
Evidence: `Makefile:62–66`.
The recipe captures formatter stdout and checks whether it is empty, without preserving the command's failure status. `make fmt-check GOLANGCI_LINT=false` prints `Checking formatting...` and exits successfully. A missing/broken formatter or dependency/toolchain failure can therefore pass this gate without checking formatting.
**Fix:** explicitly fail when the formatter command fails before inspecting its diff output. Pin the formatter version so the gate is reproducible.
**Acceptance:** a formatter that exits nonzero with empty stdout fails the target; clean formatting passes; a real diff fails.
### A15 — Release artifacts can disagree with the requested release tag
**P2 / Medium · Maintenance, supply chain · Code-confirmed**
Evidence: `.gitea/workflows/release.yml:55–94,241–269,297–314`; upload steps around lines 113–130, 278–289, and 360–371; `Makefile:84,143,256,273`.
For manual dispatch with a tag input, checkout does not explicitly select that tag. Binaries and packages are built from the dispatched checkout, while the source archives use the supplied tag and version strings claim that version. These can represent different commits. Uploads use `curl -s` without failing on HTTP error status, so publication failures can also look successful.
Additional reproducibility gaps: ordinary `make build` runs `go mod tidy` through `deps`; analysis tools use `@latest`; `release-version` lacks test/build prerequisites; `rerelease` force-moves an existing release tag.
**Fix:** resolve and validate the release tag once, check out its commit in every artifact job, and record that commit in a manifest. Fail on HTTP errors and verify the uploaded inventory. Keep normal builds read-only with respect to dependency manifests, pin build tools, and publish immutable versions with consistent quality gates.
**Acceptance:** dispatching a historical tag from a newer branch produces binaries and source from the same historical commit; failed uploads fail the workflow; normal builds leave the working tree unchanged.
### A16 — Output replacement guarantees differ across commands and writers
**P2 / Medium · Engineering, maintenance · Code-confirmed**
Evidence: `cmd/relspec/job.go:859–886`; `pkg/writers/template/writer.go:288–308`; `pkg/writers/mysql/writer.go:54–68`; `pkg/writers/json/writer.go:56–58`.
Job single-file conversion has an atomic-write wrapper, but direct writers and template output write or truncate final paths in place. Some writer cleanup functions ignore close errors. A disk error, failed generation, or interrupted multi-file run can replace a previously valid artifact with incomplete output or leave a mixed generation.
**Fix:** centralize output staging and finalization for direct CLI use and jobs. Handle close/flush errors, use exclusive unpredictable temporary files, preflight filename collisions, and define a staging/manifest strategy for multi-file generations. Preserve existing files when generation fails.
**Acceptance:** injected generation/write/finalization failures leave the previous valid output intact and return an error; no command claims success with a partial replacement.
## Perspective-specific maintenance needs
These are follow-up investments and scope gaps, not additional proven vulnerabilities.
### Software engineering
- **A common capability registry:** format construction is repeated across `convert.go`, `diff.go`, `merge.go`, `inspect.go`, and `edit.go`. Job format validation is another list. Centralize reader/writer creation and capabilities so adding a format does not require synchronized switches throughout the CLI.
- **Typed execution options and results:** connection settings, execution counters, and flags are carried through string-keyed metadata maps. Introduce typed configuration/result structures for live execution while retaining metadata for extensions.
- **Model validation before generation:** JSON/YAML readers primarily unmarshal and return models. Establish a shared normalization/validation boundary for nil entries, duplicate objects, unresolved references, incompatible attributes, and absent required fields. The inspector is useful but should not be the only optional path to structural safety.
- **Smaller database writer components:** `pkg/writers/pgsql/writer.go` is 2,643 lines and combines generation, execution, reporting, type/index policy, and statement parsing. Separate these responsibilities to make security and migration changes easier to review.
- **Adversarial parser tests:** no `Fuzz...` tests were found in `pkg` or `cmd`. Add fuzz targets for DBML/directives, ORM readers, embed handling, SQL lexical processing, and invalid intermediate models. Add large-schema benchmarks before introducing concurrency or caching.
### Security
- Document the trust model: SQL scripts, expressions/defaults, asset calls, schema metadata, templates, and release assets have different execution and local-file capabilities.
- Test redaction across every CLI/library logging path, not just the job logger and masked UI preview. Cover URL-encoded passwords, DSN parse errors, and database error messages without printing real credentials in tests.
- Run pinned vulnerability checks in required change-validation workflows and record the tool/database versions. Produce release checksums and an SBOM; signing and verification are covered by A11.
- Use scoped database roles: inspection should not require DDL rights, and schema application should not normally use a server administrator account.
### Maintenance
- Update contradictory documentation. README advertises Go 1.24.0; CONTRIBUTING says 1.25+; `go.mod` requires 1.26.9. TODO says MySQL, watch mode, type mappings, Windows installer, and installer CI are missing even though implementations exist. `tests/dbtest/README.md` still says there is no MySQL driver/reader.
- Document actual feature completeness, not just format names. Distinguish implemented, partial, experimental, and unsupported behavior with links to tests.
- The VS Code extension's `renderTemplate` is a placeholder and syntax validation mainly counts delimiters per line (`vscode-extension/relspec-template-editor/src/extension.ts`). Label it experimental or connect it to the actual Go parser/renderer. Reuse and dispose a single diagnostics collection rather than creating one on each validation.
- Align GitHub and Gitea workflow ownership, tool versions, required checks, release policies, and developer setup. Add Windows execution tests for filesystem behavior and installer/updater code, rather than relying solely on cross-compilation.
### Database specialization
- Publish an object/attribute fidelity matrix covering every supported source/destination, including views, sequences, enums, generated/identity columns, FK actions/deferral, expression/partial indexes, precision/scale, unsigned types, and permissions.
- PostgreSQL generation needs explicit fidelity review beyond tables: the model includes views, enums, permissions, and standalone sequences, but the main PostgreSQL writer does not provide corresponding complete emission paths. Its sequence generation focuses on table primary keys; scanning views to infer extensions does not emit the views. Unsupported objects need warnings or rejection.
- Establish migration safety policy: destructive-change approval in the product, reviewed plans, populated-data preflight, lock budgets, backfills, recovery, and post-apply verification. A dry run that only validates options is not proof that a migration can execute safely.
- Test semantic convergence: after applying a model, re-inspect and diff; a second application should generate no unintended change. Include populated tables, cross-schema dependencies, generated columns, reserved names, and failure recovery.
- Keep database-specific transaction and DDL behavior explicit. Do not promise uniform rollback or schema support across PostgreSQL, SQLite, SQL Server, and MySQL.
## Prioritized repair plan
| Order | Work | Completion evidence |
| --- | --- | --- |
| 1 | Fix misleading success and unsafe type fallback: A01–A02 | Failure-injection tests prove nonzero exit and preservation of existing data/schema. |
| 2 | Close file and SQL rendering boundaries: A03–A05, A08 | Adversarial filename, embed, identifier, literal, and delimiter cases pass. |
| 3 | Repair MySQL reading and fidelity: A06–A07 | Live round trips preserve constraints, indexes, and type dimensions; unsupported features fail explicitly. |
| 4 | Restore trustworthy CI coverage: A13–A14 | All relevant live writers run, toolchain matches policy, and tool failures fail gates. |
| 5 | Define execution safety: A09–A10 | Documented transaction/concurrency modes, cancellation, recovery, and precise results. |
| 6 | Harden distribution and output: A11–A12, A15–A16 | Verified installer artifacts, consistent release commit, safer defaults, and staged file replacement. |
| 7 | Reduce maintenance debt | Capability registry, typed options, accurate docs, supported-feature matrix, fuzzing, and measured performance baseline. |
Avoid expanding the format list or adding a web/API layer until the existing execution and fidelity contracts are dependable. The current code already offers enough functionality that correctness, safety, and maintainability will yield more value than additional breadth.
+9
View File
@@ -285,6 +285,7 @@ type Options struct {
Types string `yaml:"types"`
ArrayNullable string `yaml:"array_nullable"`
ContinueOnError bool `yaml:"continue_on_error"`
AtomicExecution bool `yaml:"atomic_execution"`
// FullDDL makes direct pgsql output execute the full idempotent DDL instead of
// diffing against the live database first.
FullDDL bool `yaml:"full_ddl"`
@@ -479,6 +480,14 @@ func (s *Set) Validate() error {
func (j *Job) validate() []string {
var e []string
if j.Options.AtomicExecution {
if j.Options.ContinueOnError {
e = append(e, "atomic_execution cannot be combined with continue_on_error")
}
if (j.Command != CommandConvert && j.Command != CommandMerge) || j.Output == nil || j.Output.ConnEnv == "" || j.Output.Format != "pgsql" {
e = append(e, "atomic_execution requires convert/merge with pgsql database output")
}
}
switch j.Command {
case CommandConvert, CommandMerge, CommandScriptsList, CommandScriptsExec,
+239
View File
@@ -0,0 +1,239 @@
package writers
import (
"context"
"encoding/json"
"errors"
"fmt"
"os"
"strings"
"time"
)
// ExecutionReport describes an entire invocation, including operations never attempted.
// ExecutedStatements counts operations still applied, excluding confirmed rollbacks.
type ExecutionReport struct {
Dialect string `json:"dialect,omitempty"`
TotalStatements int `json:"total_statements"`
ExecutedStatements int `json:"executed_statements"`
FailedStatements int `json:"failed_statements"`
SkippedStatements int `json:"skipped_statements"`
RolledBackStatements int `json:"rolled_back_statements"`
Outcome string `json:"outcome"`
Operations []ExecutionOperation `json:"operations"`
Schemas []SchemaReport `json:"schemas"`
Errors []ExecutionError `json:"errors,omitempty"`
StartTime string `json:"start_time"`
EndTime string `json:"end_time"`
}
type SchemaReport struct {
Name string `json:"name"`
Tables []TableReport `json:"tables"`
}
type TableReport struct {
Name string `json:"name"`
Created bool `json:"created"`
Error string `json:"error,omitempty"`
}
type ExecutionError struct {
StatementNumber int `json:"statement_number"`
Statement string `json:"statement"`
Error string `json:"error"`
}
// ExecutionOperation is one generated statement or supplied script, in execution order.
// A failed raw script may have committed some of its own statements; see Outcome.
type ExecutionOperation struct {
StatementNumber int `json:"statement_number"`
Name string `json:"name,omitempty"`
Statement string `json:"statement"`
Status string `json:"status"` // applied, executed (raw script), failed, skipped, rolled_back, unknown
Error string `json:"error,omitempty"`
Skip bool `json:"-"`
StopOnFailure bool `json:"-"`
}
// ExecutionFailure is returned even when continuation was explicitly requested.
type ExecutionFailure struct{ Report *ExecutionReport }
func (e *ExecutionFailure) Error() string {
message := fmt.Sprintf("%s execution failed: %d failed, %d executed, %d skipped, %d rolled back",
e.Report.Dialect, e.Report.FailedStatements, e.Report.ExecutedStatements,
e.Report.SkippedStatements, e.Report.RolledBackStatements)
if len(e.Report.Errors) > 0 {
first := e.Report.Errors[0]
message += fmt.Sprintf("; statement %d: %s", first.StatementNumber, first.Error)
}
return message
}
// ExecutionBackend operates on one connection. Begin switches Exec to a transaction.
// Atomic execution stops at the first failure and rolls back the whole invocation.
type ExecutionBackend struct {
Exec func(context.Context, int, string) error
Begin func(context.Context) error
Commit func(context.Context) error
Rollback func(context.Context) error
// ScriptManaged means arbitrary scripts can commit/rollback internally.
ScriptManaged bool
}
func (o *WriterOptions) ShouldContinue() bool {
legacy, _ := o.Metadata["ignore_errors"].(bool)
return o.ContinueOnError || legacy
}
// RunExecution applies a plan, records every outcome, and publishes metadata and reports.
func RunExecution(ctx context.Context, opts *WriterOptions, report *ExecutionReport, operations []ExecutionOperation, backend ExecutionBackend) error {
report.TotalStatements = len(operations)
report.Operations = append([]ExecutionOperation(nil), operations...)
report.ExecutedStatements, report.FailedStatements, report.SkippedStatements, report.RolledBackStatements = 0, 0, 0, 0
report.Errors = nil
report.StartTime = time.Now().Format(time.RFC3339Nano)
report.Outcome = "not_started"
for i := range report.Operations {
report.Operations[i].StatementNumber = i + 1
report.Operations[i].Status = "skipped"
}
var runErr error
atomic := opts.AtomicExecution
if atomic && opts.ShouldContinue() {
runErr = errors.New("atomic execution cannot be combined with continue-on-error")
} else if atomic && (backend.Begin == nil || backend.Commit == nil || backend.Rollback == nil) {
runErr = errors.New("atomic execution is not supported by this executor")
} else if atomic {
runErr = backend.Begin(ctx)
}
if runErr == nil {
report.Outcome = "nontransactional"
if backend.ScriptManaged {
report.Outcome = "script_managed"
}
stopped := false
for i := range report.Operations {
op := &report.Operations[i]
if stopped || op.Skip || strings.TrimSpace(op.Statement) == "" {
continue
}
if err := backend.Exec(ctx, i, op.Statement); err != nil {
runErr = errors.Join(runErr, err)
op.Status, op.Error = "failed", err.Error()
report.FailedStatements++
report.Errors = append(report.Errors, ExecutionError{i + 1, op.Statement, err.Error()})
stopped = op.StopOnFailure || !opts.ShouldContinue()
} else {
op.Status = "applied"
if backend.ScriptManaged {
op.Status = "executed"
}
report.ExecutedStatements++
}
}
if atomic {
if report.FailedStatements > 0 {
// Cleanup uses a bounded context independent of caller cancellation.
cleanup, cancel := context.WithTimeout(context.Background(), 5*time.Second)
rollbackErr := backend.Rollback(cleanup)
cancel()
if rollbackErr == nil {
report.Outcome = "rolled_back"
for i := range report.Operations {
if report.Operations[i].Status == "applied" {
report.Operations[i].Status = "rolled_back"
report.RolledBackStatements++
}
}
report.ExecutedStatements = 0
} else {
report.Outcome = "unknown"
runErr = errors.Join(runErr, fmt.Errorf("rollback failed: %w", rollbackErr))
}
} else {
runErr = backend.Commit(ctx)
if runErr == nil {
report.Outcome = "committed"
} else {
report.Outcome = "unknown"
}
}
if report.Outcome == "rolled_back" || report.Outcome == "unknown" {
for i := range report.Schemas {
for j := range report.Schemas[i].Tables {
report.Schemas[i].Tables[j].Created = false
}
}
}
if report.Outcome == "unknown" {
for i := range report.Operations {
if report.Operations[i].Status == "applied" {
report.Operations[i].Status = "unknown"
}
}
report.ExecutedStatements = 0
}
}
}
for _, op := range report.Operations {
if op.Status == "skipped" {
report.SkippedStatements++
}
}
report.EndTime = time.Now().Format(time.RFC3339Nano)
if report.FailedStatements > 0 {
runErr = errors.Join(&ExecutionFailure{report}, runErr)
}
return FinishExecution(opts, report, runErr)
}
// FinishExecution publishes legacy counters and a typed report even on failure.
func FinishExecution(opts *WriterOptions, report *ExecutionReport, runErr error) error {
if opts.Metadata == nil {
opts.Metadata = map[string]interface{}{}
}
opts.Metadata["execution_report"] = report
opts.Metadata["execution_total"] = report.TotalStatements
opts.Metadata["execution_success"] = report.ExecutedStatements
opts.Metadata["execution_failed"] = report.FailedStatements
opts.Metadata["execution_skipped"] = report.SkippedStatements
opts.Metadata["execution_rolled_back"] = report.RolledBackStatements
if path, _ := opts.Metadata["report_path"].(string); path != "" {
if err := WriteExecutionReport(path, report); err != nil {
runErr = errors.Join(runErr, fmt.Errorf("failed to write requested execution report: %w", err))
}
}
return runErr
}
func WriteExecutionReport(path string, report *ExecutionReport) error {
file, err := os.Create(path)
if err != nil {
return fmt.Errorf("failed to create report file: %w", err)
}
encoder := json.NewEncoder(file)
encoder.SetIndent("", " ")
return errors.Join(encoder.Encode(report), file.Close())
}
// StatementOperations creates a plan from the writers' structured statement lists.
func StatementOperations(statements []string) []ExecutionOperation {
ops := make([]ExecutionOperation, len(statements))
for i, stmt := range statements {
ops[i] = ExecutionOperation{Statement: stmt, StopOnFailure: strings.HasPrefix(strings.TrimSpace(stmt), "DO $relspec_preflight"), Skip: strings.TrimSpace(stmt) == "" || strings.HasPrefix(strings.TrimSpace(stmt), "--")}
}
return ops
}
// FailExecution records an invocation that failed before issuing any SQL.
func FailExecution(opts *WriterOptions, dialect string, operations []ExecutionOperation, cause error) error {
report := &ExecutionReport{Dialect: dialect, TotalStatements: len(operations), SkippedStatements: len(operations), Outcome: "not_started", Operations: append([]ExecutionOperation(nil), operations...)}
report.StartTime = time.Now().Format(time.RFC3339Nano)
report.EndTime = report.StartTime
for i := range report.Operations {
report.Operations[i].StatementNumber = i + 1
report.Operations[i].Status = "skipped"
}
return FinishExecution(opts, report, cause)
}
+96
View File
@@ -0,0 +1,96 @@
package writers
import (
"context"
"errors"
"path/filepath"
"strings"
"testing"
)
func TestExecutionOutcomes(t *testing.T) {
for _, atomic := range []bool{false, true} {
for _, continuation := range []bool{false, true} {
t.Run(strings.Join([]string{map[bool]string{false: "independent", true: "atomic"}[atomic], map[bool]string{false: "stop", true: "continue"}[continuation]}, "/"), func(t *testing.T) {
attempted, rollbacks := 0, 0
cause := errors.New("injected failure")
backend := ExecutionBackend{
Exec: func(_ context.Context, i int, _ string) error {
attempted++
if i == 1 {
return cause
}
return nil
},
Begin: func(context.Context) error { return nil }, Commit: func(context.Context) error { t.Fatal("must not commit"); return nil },
Rollback: func(context.Context) error { rollbacks++; return nil },
}
opts := &WriterOptions{ContinueOnError: continuation, AtomicExecution: atomic}
report := &ExecutionReport{Dialect: "test"}
err := RunExecution(context.Background(), opts, report, StatementOperations([]string{"one", "bad", "three"}), backend)
if atomic && continuation {
if err == nil || attempted != 0 || report.SkippedStatements != 3 {
t.Fatalf("invalid options must not execute: %+v %v", report, err)
}
return
}
var failure *ExecutionFailure
if !errors.As(err, &failure) || !errors.Is(err, cause) {
t.Fatalf("must preserve typed report and cause: %v", err)
}
if report.FailedStatements != 1 || report.TotalStatements != 3 || report.Errors[0].StatementNumber != 2 {
t.Fatalf("failure report: %+v", report)
}
if atomic {
if rollbacks != 1 || report.ExecutedStatements != 0 || report.RolledBackStatements != 1 || report.Operations[0].Status != "rolled_back" || report.Outcome != "rolled_back" {
t.Fatalf("rollback report: %+v", report)
}
} else if continuation {
if attempted != 3 || report.ExecutedStatements != 2 || report.SkippedStatements != 0 {
t.Fatalf("continuation report: %+v", report)
}
} else if attempted != 2 || report.ExecutedStatements != 1 || report.SkippedStatements != 1 {
t.Fatalf("stop report: %+v", report)
}
if opts.Metadata["execution_report"] != report {
t.Fatal("typed results missing")
}
})
}
}
}
func TestExecutionUncertainFinalization(t *testing.T) {
for _, failStatement := range []bool{false, true} {
t.Run(map[bool]string{false: "commit", true: "rollback"}[failStatement], func(t *testing.T) {
finalErr := errors.New("connection lost during finalization")
backend := ExecutionBackend{
Begin: func(context.Context) error { return nil },
Exec: func(_ context.Context, i int, _ string) error {
if failStatement && i == 1 {
return errors.New("statement failed")
}
return nil
},
Commit: func(context.Context) error { return finalErr }, Rollback: func(context.Context) error { return finalErr },
}
report := &ExecutionReport{}
err := RunExecution(context.Background(), &WriterOptions{AtomicExecution: true}, report, StatementOperations([]string{"one", "two"}), backend)
if !errors.Is(err, finalErr) || report.Outcome != "unknown" || report.Operations[0].Status != "unknown" || report.RolledBackStatements != 0 || report.ExecutedStatements != 0 {
t.Fatalf("uncertain outcome misreported: %+v %v", report, err)
}
})
}
}
func TestExecutionReportFailureAndFatalPreflight(t *testing.T) {
opts := &WriterOptions{ContinueOnError: true, Metadata: map[string]interface{}{"report_path": filepath.Join(t.TempDir(), "missing", "report.json")}}
report := &ExecutionReport{}
calls := 0
ops := []ExecutionOperation{{Statement: "preflight", StopOnFailure: true}, {Statement: "must not run"}}
err := RunExecution(context.Background(), opts, report, ops, ExecutionBackend{Exec: func(context.Context, int, string) error { calls++; return errors.New("invalid data") }})
var failure *ExecutionFailure
if !errors.As(err, &failure) || !strings.Contains(err.Error(), "requested execution report") || calls != 1 || report.SkippedStatements != 1 {
t.Fatalf("must retain both failures and stop: %+v %v", report, err)
}
}
+16 -16
View File
@@ -502,30 +502,30 @@ func (w *Writer) executeDatabaseSQL(db *models.Database, connString string) erro
// Connect to database
dbConn, err := sql.Open("mssql", connString)
if err != nil {
return fmt.Errorf("failed to connect to database: %w", err)
return writers.FailExecution(w.options, "SQL Server", writers.StatementOperations(statements), fmt.Errorf("failed to connect to database: %w", err))
}
defer dbConn.Close()
ctx := context.Background()
if err = dbConn.PingContext(ctx); err != nil {
return fmt.Errorf("failed to ping database: %w", err)
return writers.FailExecution(w.options, "SQL Server", writers.StatementOperations(statements), fmt.Errorf("failed to ping database: %w", err))
}
// Execute statements
executedCount := 0
for i, stmt := range statements {
fmt.Fprintf(os.Stderr, "Executing statement %d/%d...\n", i+1, len(statements))
if _, execErr := dbConn.ExecContext(ctx, stmt); execErr != nil {
fmt.Fprintf(os.Stderr, "⚠ Warning: Statement failed: %v\n", execErr)
continue
}
executedCount++
var tx *sql.Tx
backend := writers.ExecutionBackend{
Exec: func(ctx context.Context, _ int, stmt string) error {
if tx != nil {
_, err := tx.ExecContext(ctx, stmt)
return err
}
_, err := dbConn.ExecContext(ctx, stmt)
return err
},
Begin: func(ctx context.Context) error { var err error; tx, err = dbConn.BeginTx(ctx, nil); return err },
Commit: func(context.Context) error { return tx.Commit() },
Rollback: func(context.Context) error { return tx.Rollback() },
}
fmt.Fprintf(os.Stderr, "✓ Successfully executed %d statements\n", executedCount)
return nil
return writers.RunExecution(ctx, w.options, &writers.ExecutionReport{Dialect: "SQL Server"}, writers.StatementOperations(statements), backend)
}
// generateStatements renders the same script WriteDatabase would produce and
+69
View File
@@ -0,0 +1,69 @@
package mssql
import (
"database/sql"
"errors"
"fmt"
"os"
"testing"
"time"
"git.warky.dev/wdevs/relspecgo/pkg/models"
"git.warky.dev/wdevs/relspecgo/pkg/writers"
)
func TestLive_ExecutionFailureModes(t *testing.T) {
connection := os.Getenv("RELSPEC_TEST_MSSQL_CONN")
if connection == "" {
t.Skip("RELSPEC_TEST_MSSQL_CONN not set")
}
conn, err := sql.Open("mssql", connection)
if err != nil {
t.Fatal(err)
}
defer conn.Close()
for _, mode := range []string{"stop", "continue", "atomic"} {
t.Run(mode, func(t *testing.T) {
prefix := fmt.Sprintf("audit_%d_", time.Now().UnixNano())
db := models.InitDatabase("audit")
schema := models.InitSchema("dbo")
for _, name := range []string{"first", "bad", "last"} {
table := models.InitTable(prefix+name, "dbo")
col := models.InitColumn("id", table.Name, "dbo")
col.Type = "integer"
if name == "bad" {
duplicate := models.InitColumn("id", table.Name, "dbo")
duplicate.Type = "integer"
table.Columns["duplicate"] = duplicate
}
table.Columns[col.Name] = col
schema.Tables = append(schema.Tables, table)
}
db.Schemas = append(db.Schemas, schema)
t.Cleanup(func() {
for _, name := range []string{"first", "bad", "last"} {
_, _ = conn.Exec("DROP TABLE IF EXISTS dbo." + prefix + name)
}
})
opts := &writers.WriterOptions{ContinueOnError: mode == "continue", AtomicExecution: mode == "atomic", Metadata: map[string]interface{}{"connection_string": connection}}
err := NewWriter(opts).WriteDatabase(db)
var failure *writers.ExecutionFailure
if !errors.As(err, &failure) || failure.Report.FailedStatements == 0 {
t.Fatalf("expected structured error: %v", err)
}
for _, name := range []string{"first", "last"} {
var count int
if err := conn.QueryRow("SELECT count(*) FROM sys.tables WHERE name=?", prefix+name).Scan(&count); err != nil {
t.Fatal(err)
}
want := mode != "atomic" && (name == "first" || mode == "continue")
if (count == 1) != want {
t.Fatalf("%s exists=%v want=%v", name, count == 1, want)
}
}
if mode == "atomic" && (failure.Report.Outcome != "rolled_back" || failure.Report.RolledBackStatements == 0 || failure.Report.ExecutedStatements != 0) {
t.Fatalf("incorrect rollback report: %+v", failure.Report)
}
})
}
}
+64
View File
@@ -346,3 +346,67 @@ metadata:
- Default values are properly quoted
- Constraint names follow PostgreSQL conventions
- Compatible with PostgreSQL 12+
## Live execution failures
Live PostgreSQL output stops after the first failed statement by default and returns
`*writers.ExecutionFailure` (also available as `*pgsql.ExecutionFailure`; inspect it
with `errors.As`). `WriterOptions.ContinueOnError` or job
`options.continue_on_error: true` attempts later statements but still returns an error
if any fail, preventing dependent jobs from running. A failed type preflight always
stops live execution, including when continuation is enabled.
PostgreSQL, SQLite, SQL Server, and the PostgreSQL script runner share execution
reports through `WriterOptions.Metadata["execution_report"]`. Optional
`Metadata["report_path"]` writes the report as JSON; write and close failures are
errors. Reports include each operation's status and original number, plus applied,
failed, skipped, and rolled-back counts. Blank and comment entries count as skipped.
Unknown commit/rollback outcomes are explicitly marked `unknown` and are never
reported as confirmed rollback. Connection failures record all planned operations as
skipped. Raw scripts have different reporting limits; see the sqlexec README.
Generated DDL uses independent statements by default: earlier changes can remain
committed after failure. Set `WriterOptions.AtomicExecution = true` to use one
transaction with PostgreSQL, SQLite, or SQL Server generated DDL. For database output
jobs, use:
```yaml
options:
atomic_execution: true
```
Atomic execution and continuation are mutually exclusive. Rollback covers database
transactional effects; sequence advancement and external effects in SQL expressions
are outside that guarantee. Concurrency locks, migration history, and resumable
migration management remain separate work (audit A09).
## Column type conversions
Type changes propagate errors and never implicitly rename or replace a column.
Before schema changes, full-DDL and diff plans evaluate every existing value through
the conversion expression and target cast. Diff preflights run before dropping
constraints/indexes or creating sequences; database-wide preflights run before
changing any schema. Matching types, missing tables/columns, generated columns, and
identity columns need no conversion check.
Column metadata in JSON/YAML models supports two optional SQL expressions:
```yaml
metadata:
pgsql_using: "split_part(name, ' ', 1)::integer"
pgsql_preflight: "name ~ '^[0-9]+ units$'"
```
`pgsql_using` replaces the default `column::target_type` expression in `USING`.
`pgsql_preflight` is a row predicate; false or NULL rejects the conversion. These
fields must contain nonempty strings. Both are trusted executable SQL, like defaults
and check expressions; they are not values or a sandbox. Preflights evaluate the
conversion expression, so use expressions without side effects. New types/extensions
needed by an existing-column conversion must already be available for preflight.
Preflight checks can race concurrent writes and do not prove that locks, defaults,
or every dependency will permit the ALTER. The ALTER still validates the conversion;
use atomic execution when preceding operations also need rollback. Generated SQL
clients must stop on preflight failure; live RelSpec execution enforces this even
with continuation enabled. Rename/replacement/backfill operations still require a
separately reviewed migration.
+178 -4
View File
@@ -3,9 +3,11 @@ package pgsql
import (
"context"
"encoding/json"
"errors"
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"time"
@@ -141,15 +143,16 @@ func TestLive_WriteDatabaseEmptyThenIdenticalThenDrifted(t *testing.T) {
}
}
func TestLive_WriteDatabaseFailedStatementContinues(t *testing.T) {
func TestLive_WriteDatabaseFailedStatementReturnsError(t *testing.T) {
connString := liveWriterConn(t)
schema, _ := liveWriterSchema(t, connString)
reportPath := filepath.Join(t.TempDir(), "report.json")
db := liveModel(schema, map[string]string{"bad": "no_such_type_xyz"})
rep, err := runLiveWrite(t, connString, db, map[string]interface{}{"full_ddl": true, "report_path": reportPath})
if err != nil {
t.Fatalf("failed statements must not abort the run: %v", err)
var failure *ExecutionFailure
if !errors.As(err, &failure) {
t.Fatalf("expected structured failure, got: %v", err)
}
if rep.FailedStatements == 0 || len(rep.Errors) != rep.FailedStatements {
t.Fatalf("expected recorded failures: %+v", rep)
@@ -230,7 +233,7 @@ func TestLive_ExecuteStatementsSkipsCommentsAndBlank(t *testing.T) {
t.Fatal(err)
}
r := w.executionReport
if r.ExecutedStatements != 2 || r.FailedStatements != 0 || r.TotalStatements != 5 {
if r.ExecutedStatements != 2 || r.FailedStatements != 0 || r.TotalStatements != 5 || r.SkippedStatements != 3 {
t.Errorf("counts: %+v", r)
}
if len(r.Schemas) != 1 || r.Schemas[0].Name != schema || len(r.Schemas[0].Tables) != 1 || !r.Schemas[0].Tables[0].Created {
@@ -241,3 +244,174 @@ func TestLive_ExecuteStatementsSkipsCommentsAndBlank(t *testing.T) {
t.Errorf("table not created: %v", err)
}
}
func TestLive_ExecuteStatementsFailureModes(t *testing.T) {
connString := liveWriterConn(t)
for _, continueOnError := range []bool{false, true} {
t.Run(fmt.Sprintf("continue=%v", continueOnError), func(t *testing.T) {
schema, conn := liveWriterSchema(t, connString)
reportPath := filepath.Join(t.TempDir(), "report.json")
w := NewWriter(&writers.WriterOptions{
ContinueOnError: continueOnError,
Metadata: map[string]interface{}{"report_path": reportPath},
})
w.executionReport = &ExecutionReport{}
err := w.executeStatements([]string{
"CREATE SCHEMA " + schema,
"SELECT 1 / 0",
"CREATE TABLE " + schema + ".after_failure (id int)",
}, connString)
var failure *ExecutionFailure
if !errors.As(err, &failure) || failure.Report != w.executionReport {
t.Fatalf("expected structured execution failure, got %v", err)
}
var report ExecutionReport
data, err := os.ReadFile(reportPath)
if err != nil {
t.Fatal(err)
}
if err := json.Unmarshal(data, &report); err != nil {
t.Fatal(err)
}
executed, skipped := 1, 1
if continueOnError {
executed, skipped = 2, 0
}
if report.TotalStatements != 3 || report.ExecutedStatements != executed || report.FailedStatements != 1 || report.SkippedStatements != skipped || report.EndTime == "" {
t.Fatalf("incorrect partial application report: %+v", report)
}
if len(report.Errors) != 1 || report.Errors[0].StatementNumber != 2 {
t.Fatalf("incorrect failure details: %+v", report.Errors)
}
if exists := columnExists(t, conn, schema, "after_failure", "id"); exists != continueOnError {
t.Fatalf("later statement executed=%v, continue=%v", exists, continueOnError)
}
})
}
}
func TestLive_IncompatibleTypeChangePreservesColumnAndData(t *testing.T) {
connString := liveWriterConn(t)
schema, conn := liveWriterSchema(t, connString)
ctx := context.Background()
_, err := conn.Exec(ctx, "CREATE SCHEMA "+schema+"; CREATE TABLE "+schema+".accounts (id integer PRIMARY KEY, name text UNIQUE); INSERT INTO "+schema+".accounts VALUES (1, 'not-an-integer')")
if err != nil {
t.Fatal(err)
}
var originalIndex uint32
if err := conn.QueryRow(ctx, "SELECT to_regclass($1)::oid", schema+".accounts_name_key").Scan(&originalIndex); err != nil {
t.Fatal(err)
}
// Exercise both full-DDL and diff generation against populated data.
for _, fullDDL := range []bool{false, true} {
t.Run(fmt.Sprintf("full_ddl=%v", fullDDL), func(t *testing.T) {
model := liveModel(schema, map[string]string{"name": "integer"})
unique := models.InitConstraint("accounts_name_key", models.UniqueConstraint)
unique.Columns = []string{"name"}
model.Schemas[0].Tables[0].Constraints[unique.Name] = unique
report, err := runLiveWrite(t, connString, model, map[string]interface{}{"full_ddl": fullDDL})
var failure *ExecutionFailure
if !errors.As(err, &failure) || report.FailedStatements != 1 {
t.Fatalf("expected failed conversion, got report=%+v err=%v", report, err)
}
var value, typ string
if err := conn.QueryRow(ctx, "SELECT name, pg_typeof(name)::text FROM "+schema+".accounts WHERE id=1").Scan(&value, &typ); err != nil {
t.Fatal(err)
}
if value != "not-an-integer" || typ != "text" {
t.Fatalf("original column changed: value=%q type=%q", value, typ)
}
var columns int
if err := conn.QueryRow(ctx, "SELECT count(*) FROM information_schema.columns WHERE table_schema=$1 AND table_name='accounts'", schema).Scan(&columns); err != nil || columns != 2 {
t.Fatalf("unexpected replacement column: columns=%d err=%v", columns, err)
}
var index uint32
if err := conn.QueryRow(ctx, "SELECT to_regclass($1)::oid", schema+".accounts_name_key").Scan(&index); err != nil || index != originalIndex {
t.Fatalf("unique index changed: index=%d original=%d err=%v", index, originalIndex, err)
}
if _, err := conn.Exec(ctx, "INSERT INTO "+schema+".accounts VALUES (2, 'not-an-integer')"); err == nil {
t.Fatal("unique constraint no longer enforced")
}
})
}
}
func TestLive_AtomicExecutionRollback(t *testing.T) {
connString := liveWriterConn(t)
schema, conn := liveWriterSchema(t, connString)
w := NewWriter(&writers.WriterOptions{AtomicExecution: true})
w.executionReport = &ExecutionReport{}
err := w.executeStatements([]string{
"-- Schema: " + schema,
"CREATE SCHEMA " + schema,
"CREATE TABLE " + schema + ".created (id int)",
"SELECT 1 / 0",
"SELECT 1",
}, connString)
var failure *ExecutionFailure
if !errors.As(err, &failure) {
t.Fatalf("expected execution failure: %v", err)
}
report := failure.Report
if report.Outcome != "rolled_back" || report.RolledBackStatements != 2 || report.ExecutedStatements != 0 || report.FailedStatements != 1 || report.SkippedStatements != 2 {
t.Fatalf("rollback report: %+v", report)
}
if report.Schemas[0].Tables[0].Created {
t.Fatal("rolled-back table must not be reported created")
}
if columnExists(t, conn, schema, "created", "id") {
t.Fatal("table survived transaction rollback")
}
}
func TestLive_TypePreflightPrecedesDropsEvenWithContinuation(t *testing.T) {
connString := liveWriterConn(t)
schema, conn := liveWriterSchema(t, connString)
_, err := conn.Exec(context.Background(), "CREATE SCHEMA "+schema+"; CREATE TABLE "+schema+".accounts (id integer PRIMARY KEY, name text UNIQUE); INSERT INTO "+schema+".accounts VALUES (1, '123'), (2, 'bad')")
if err != nil {
t.Fatal(err)
}
// The desired model would drop the unique constraint. Incompatible data must
// be caught first, including bad data after the first row.
model := liveModel(schema, map[string]string{"name": "integer"})
w := NewWriter(&writers.WriterOptions{ContinueOnError: true, Metadata: map[string]interface{}{"connection_string": connString}})
err = w.WriteDatabase(model)
var failure *ExecutionFailure
if !errors.As(err, &failure) || failure.Report.FailedStatements != 1 || failure.Report.ExecutedStatements != 0 {
t.Fatalf("preflight must stop mutations: %+v %v", w.executionReport, err)
}
var exists bool
if err := conn.QueryRow(context.Background(), "SELECT to_regclass($1) IS NOT NULL", schema+".accounts_name_key").Scan(&exists); err != nil || !exists {
t.Fatalf("constraint was dropped before preflight: %v", err)
}
}
func TestLive_CustomTypeConversionAndPredicate(t *testing.T) {
connString := liveWriterConn(t)
for _, fullDDL := range []bool{false, true} {
t.Run(fmt.Sprintf("full_ddl=%v", fullDDL), func(t *testing.T) {
schema, conn := liveWriterSchema(t, connString)
_, err := conn.Exec(context.Background(), "CREATE SCHEMA "+schema+"; CREATE TABLE "+schema+".accounts (id integer PRIMARY KEY, name text); INSERT INTO "+schema+".accounts VALUES (1,'42 units'),(2,'7 units')")
if err != nil {
t.Fatal(err)
}
model := liveModel(schema, map[string]string{"name": "integer"})
col := model.Schemas[0].Tables[0].Columns["name"]
col.Metadata["pgsql_using"] = "split_part(name, ' ', 1)::integer"
col.Metadata["pgsql_preflight"] = "name ~ '^[0-9]+ units$' AND name NOT LIKE '7 %'"
meta := map[string]interface{}{"full_ddl": fullDDL}
if _, err := runLiveWrite(t, connString, model, meta); err == nil || !strings.Contains(err.Error(), "preflight rejected rows") {
t.Fatalf("predicate must reject: %v", err)
}
col.Metadata["pgsql_preflight"] = "name ~ '^[0-9]+ units$'"
if _, err := runLiveWrite(t, connString, model, meta); err != nil {
t.Fatal(err)
}
var value int
if err := conn.QueryRow(context.Background(), "SELECT sum(name) FROM "+schema+".accounts").Scan(&value); err != nil || value != 49 {
t.Fatalf("custom conversion result %d: %v", value, err)
}
})
}
}
+9 -8
View File
@@ -487,15 +487,16 @@ func (w *MigrationWriter) generateAlterTableScripts(schema *models.Schema, model
} else if !columnsEqual(modelCol, currentCol) {
// Column exists but properties changed
if !columnTypesEqual(modelCol, currentCol) {
newType := effectiveAlterColumnSQLType(modelCol)
sql, err := w.executor.ExecuteAlterColumnTypeWithCheck(AlterColumnTypeWithCheckData{
SchemaName: schema.Name,
TableName: modelTable.Name,
ColumnName: modelCol.Name,
NewType: newType,
EquivalentTypes: equivalentTypeListSQL(newType),
UsingExpr: buildAlterColumnUsingExpression(modelCol.Name, newType),
data, err := conversionData(schema.Name, modelTable.Name, modelCol)
if err != nil {
return nil, err
}
scripts = append(scripts, MigrationScript{
ObjectName: fmt.Sprintf("%s.%s.%s", schema.Name, modelTable.Name, modelCol.Name),
ObjectType: "preflight column type", Schema: schema.Name, Priority: -10000,
Sequence: len(scripts), Body: preflightColumnType(data, w.options.FlattenSchema),
})
sql, err := w.executor.ExecuteAlterColumnTypeWithCheck(data)
if err != nil {
return nil, err
}
+7 -11
View File
@@ -136,7 +136,7 @@ func TestWriteMigration_AltersColumnTypeWhenActualTypeDiffers(t *testing.T) {
}
}
func TestWriteMigration_AltersColumnTypeFallsBackToRenameAndAddOnConversionFailure(t *testing.T) {
func TestWriteMigration_AltersColumnTypePreservesConversionFailure(t *testing.T) {
current := models.InitDatabase("testdb")
currentSchema := models.InitSchema("public")
currentTable := models.InitTable("learnings", "public")
@@ -167,17 +167,13 @@ func TestWriteMigration_AltersColumnTypeFallsBackToRenameAndAddOnConversionFailu
}
output := buf.String()
if !strings.Contains(output, "EXCEPTION WHEN OTHERS THEN") {
t.Fatalf("expected migration to guard the type conversion with an exception handler, got:\n%s", output)
if !strings.Contains(output, "ALTER COLUMN details TYPE integer USING details::integer;") {
t.Fatalf("expected explicit type conversion, got:\n%s", output)
}
if !strings.Contains(output, "RENAME COLUMN details TO %I") {
t.Fatalf("expected migration to rename the old column (derived from the live type) on conversion failure, got:\n%s", output)
}
if !strings.Contains(output, "renamed_column := 'details_' || trim(both '_' from regexp_replace(lower(current_type)") {
t.Fatalf("expected migration to derive the renamed column name from the live type, got:\n%s", output)
}
if !strings.Contains(output, "ADD COLUMN details integer") {
t.Fatalf("expected migration to add a fresh column with the new type on conversion failure, got:\n%s", output)
for _, unsafe := range []string{"EXCEPTION WHEN OTHERS", "renamed_column", "RENAME COLUMN"} {
if strings.Contains(output, unsafe) {
t.Fatalf("type conversion must propagate errors without replacing the column (%s):\n%s", unsafe, output)
}
}
}
+12 -5
View File
@@ -2,6 +2,7 @@ package pgsql
import (
"encoding/json"
"errors"
"os"
"path/filepath"
"strings"
@@ -237,8 +238,9 @@ func TestWriteAndFinishReport(t *testing.T) {
options: &writers.WriterOptions{Metadata: map[string]interface{}{"report_path": path}},
executionReport: report,
}
if err := w.finishReport(); err != nil {
t.Fatalf("finishReport: %v", err)
var failure *ExecutionFailure
if err := w.finishReport(); !errors.As(err, &failure) || failure.Report != report {
t.Fatalf("expected structured execution failure, got: %v", err)
}
data, err := os.ReadFile(path)
@@ -270,10 +272,15 @@ func TestWriteReportBadPath(t *testing.T) {
if err := w.writeReport(filepath.Join(t.TempDir(), "missing", "r.json")); err == nil {
t.Error("expected error for unwritable path")
}
// finishReport must swallow the report error.
// Explicitly requested reports are part of successful execution.
w.options.Metadata = map[string]interface{}{"report_path": filepath.Join(t.TempDir(), "missing", "r.json")}
if err := w.finishReport(); err != nil {
t.Errorf("finishReport must not fail on report write error: %v", err)
if err := w.finishReport(); err == nil {
t.Error("finishReport must fail on report write error")
}
w.executionReport.FailedStatements = 1
var failure *ExecutionFailure
if err := w.finishReport(); !errors.As(err, &failure) || !strings.Contains(err.Error(), "requested execution report") {
t.Fatalf("must preserve both execution and report errors: %v", err)
}
}
+12 -9
View File
@@ -99,15 +99,15 @@ type AddColumnData struct {
// AlterColumnTypeWithCheckData contains data for the guarded alter column
// type template, which only alters existing columns whose live type
// differs from the desired one, and falls back to renaming the old column
// and adding a fresh one when the in-place conversion is not possible.
// differs from the desired one. Conversion errors propagate unchanged.
type AlterColumnTypeWithCheckData struct {
SchemaName string
TableName string
ColumnName string
NewType string
EquivalentTypes string
UsingExpr string
SchemaName string
TableName string
ColumnName string
NewType string
EquivalentTypes string
UsingExpr string
PreflightPredicate string
}
// AlterColumnDefaultWithCheckData contains data for the guarded alter
@@ -332,7 +332,10 @@ func (te *TemplateExecutor) ExecuteAlterColumnTypeWithCheck(data AlterColumnType
if err != nil {
return "", fmt.Errorf("failed to execute alter_column_type_with_check template: %w", err)
}
return buf.String(), nil
body := strings.TrimSpace(buf.String())
body = strings.TrimPrefix(body, "DO $$")
body = strings.TrimSuffix(body, "$$;")
return dollarBlock(strings.TrimSpace(body), "relspec_alter_type"), nil
}
// ExecuteAlterColumnDefaultWithCheck executes the guarded alter column
@@ -1,12 +1 @@
DO $$
BEGIN
IF NOT EXISTS (
SELECT 1 FROM information_schema.columns
WHERE table_schema = '{{.SchemaName}}'
AND table_name = '{{.TableName}}'
AND column_name = '{{.ColumnName}}'
) THEN
ALTER TABLE {{qual_table .SchemaName .TableName}} ADD COLUMN {{.ColumnDefinition}};
END IF;
END;
$$;
ALTER TABLE {{qual_table .SchemaName .TableName}} ADD COLUMN IF NOT EXISTS {{.ColumnDefinition}};
@@ -1,30 +1,22 @@
DO $$
DECLARE
current_type text;
renamed_column text;
BEGIN
SELECT pg_catalog.format_type(a.atttypid, a.atttypmod)
INTO current_type
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;
IF current_type IS NOT NULL
AND current_type <> ALL(ARRAY[{{.EquivalentTypes}}]) THEN
BEGIN
ALTER TABLE {{qual_table .SchemaName .TableName}}
ALTER COLUMN {{quote_ident .ColumnName}} TYPE {{.NewType}}{{if .UsingExpr}} USING {{.UsingExpr}}{{end}};
EXCEPTION WHEN OTHERS THEN
renamed_column := '{{.ColumnName}}_' || trim(both '_' from regexp_replace(lower(current_type), '[^a-z0-9]+', '_', 'g'));
EXECUTE format('ALTER TABLE {{qual_table .SchemaName .TableName}} RENAME COLUMN {{quote_ident .ColumnName}} TO %I', renamed_column);
ALTER TABLE {{qual_table .SchemaName .TableName}}
ADD COLUMN {{quote_ident .ColumnName}} {{.NewType}};
END;
ALTER TABLE {{qual_table .SchemaName .TableName}}
ALTER COLUMN {{quote_ident .ColumnName}} TYPE {{.NewType}}{{if .UsingExpr}} USING {{.UsingExpr}}{{end}};
END IF;
END;
$$;
+91
View File
@@ -0,0 +1,91 @@
package pgsql
import (
"fmt"
"strings"
"github.com/jackc/pgx/v5"
"git.warky.dev/wdevs/relspecgo/pkg/models"
)
// conversionData accepts executable SQL only through explicitly named column metadata.
func conversionData(schema, table string, col *models.Column) (AlterColumnTypeWithCheckData, error) {
target := effectiveAlterColumnSQLType(col)
data := AlterColumnTypeWithCheckData{
SchemaName: schema, TableName: table, ColumnName: col.Name,
NewType: target, EquivalentTypes: equivalentTypeListSQL(target),
UsingExpr: buildAlterColumnUsingExpression(col.Name, target),
}
for key, destination := range map[string]*string{"pgsql_using": &data.UsingExpr, "pgsql_preflight": &data.PreflightPredicate} {
if value, exists := col.Metadata[key]; exists {
expression, ok := value.(string)
if !ok || strings.TrimSpace(expression) == "" {
return data, fmt.Errorf("%s.%s.%s metadata %s must be a nonempty SQL expression", schema, table, col.Name, key)
}
*destination = expression
}
}
return data, nil
}
// sqlLiteral is independent of the connection's standard_conforming_strings setting.
func sqlLiteral(value string) string {
return "E'" + strings.NewReplacer("\\", "\\\\", "'", "''").Replace(value) + "'"
}
func dollarBlock(body, prefix string) string {
for i := 0; ; i++ {
delimiter := fmt.Sprintf("$%s_%d$", prefix, i)
if !strings.Contains(body, delimiter) {
return "DO " + delimiter + "\n" + body + "\n" + delimiter + ";"
}
}
}
// preflightColumnType checks every value before any schema changes. It is advisory:
// writes can race the check, so the ALTER itself still performs the real conversion.
func preflightColumnType(data AlterColumnTypeWithCheckData, flatten bool) string {
ident := pgx.Identifier{data.TableName}
if data.SchemaName != "" {
if flatten {
ident = pgx.Identifier{data.SchemaName + "_" + data.TableName}
} else {
ident = pgx.Identifier{data.SchemaName, data.TableName}
}
}
table := ident.Sanitize()
body := fmt.Sprintf(`DECLARE current_type text;
BEGIN
SELECT pg_catalog.format_type(atttypid, atttypmod) INTO current_type
FROM pg_catalog.pg_attribute
WHERE attrelid = to_regclass(%s) AND attname = %s AND attnum > 0 AND NOT attisdropped;
IF current_type IS NOT NULL AND current_type <> ALL(ARRAY[%s]) THEN
`, sqlLiteral(table), sqlLiteral(data.ColumnName), data.EquivalentTypes)
if data.PreflightPredicate != "" {
body += fmt.Sprintf(` IF EXISTS (SELECT 1 FROM %s WHERE NOT COALESCE((%s), false)) THEN
RAISE EXCEPTION USING MESSAGE = %s;
END IF;
`, table, data.PreflightPredicate, sqlLiteral("type conversion preflight rejected rows for "+data.SchemaName+"."+data.TableName+"."+data.ColumnName))
}
// Aggregate evaluation forces the conversion for all rows, including non-first rows.
body += fmt.Sprintf(" PERFORM count((%s)::%s) FROM %s;\n END IF;\nEND;", data.UsingExpr, data.NewType, table)
return dollarBlock(body, "relspec_preflight")
}
func (w *Writer) generateTypePreflights(schema *models.Schema) ([]string, error) {
var statements []string
for _, table := range schema.Tables {
for _, col := range getSortedColumns(table.Columns) {
if col.Generated || col.Identity {
continue
}
data, err := conversionData(schema.Name, table.Name, col)
if err != nil {
return nil, err
}
statements = append(statements, preflightColumnType(data, w.options.FlattenSchema))
}
}
return statements, nil
}
+72
View File
@@ -0,0 +1,72 @@
package pgsql
import (
"strings"
"testing"
"git.warky.dev/wdevs/relspecgo/pkg/models"
"git.warky.dev/wdevs/relspecgo/pkg/writers"
)
func TestConversionMetadataValidationAndBlockDelimiters(t *testing.T) {
col := models.InitColumn("name", "accounts", "public")
col.Type = "integer"
for _, key := range []string{"pgsql_using", "pgsql_preflight"} {
for _, value := range []interface{}{true, " "} {
col.Metadata[key] = value
if _, err := conversionData("public", "accounts", col); err == nil || !strings.Contains(err.Error(), key) {
t.Fatalf("expected invalid %s metadata error: %v", key, err)
}
delete(col.Metadata, key)
}
}
col.Metadata["pgsql_using"] = "length('$relspec_alter_type_0$ $relspec_preflight_0$')"
data, err := conversionData("public", "accounts", col)
if err != nil {
t.Fatal(err)
}
executor, err := NewTemplateExecutor(false)
if err != nil {
t.Fatal(err)
}
alter, err := executor.ExecuteAlterColumnTypeWithCheck(data)
if err != nil {
t.Fatal(err)
}
if !strings.HasPrefix(alter, "DO $relspec_alter_type_1$") {
t.Fatalf("unsafe alter delimiter: %s", alter)
}
preflight := preflightColumnType(data, false)
if !strings.HasPrefix(preflight, "DO $relspec_preflight_1$") {
t.Fatalf("unsafe preflight delimiter: %s", preflight)
}
}
func TestPreflightsPrecedeAllSchemaMutations(t *testing.T) {
db := models.InitDatabase("preflight")
for _, name := range []string{"one", "two"} {
schema := models.InitSchema(name)
table := models.InitTable("accounts", name)
col := models.InitColumn("name", "accounts", name)
col.Type = "integer"
table.Columns[col.Name] = col
schema.Tables = append(schema.Tables, table)
db.Schemas = append(db.Schemas, schema)
}
stmts, err := NewWriter(&writers.WriterOptions{}).GenerateDatabaseStatements(db)
if err != nil {
t.Fatal(err)
}
lastPreflight, firstDDL := -1, len(stmts)
for i, stmt := range stmts {
if strings.HasPrefix(stmt, "DO $relspec_preflight") {
lastPreflight = i
}
if strings.HasPrefix(stmt, "CREATE SCHEMA") && i < firstDDL {
firstDDL = i
}
}
if lastPreflight < 0 || lastPreflight >= firstDDL {
t.Fatal("preflights must precede all schema changes")
}
}
+121 -164
View File
@@ -2,7 +2,6 @@ package pgsql
import (
"context"
"encoding/json"
"fmt"
"io"
"os"
@@ -12,6 +11,8 @@ import (
"sync"
"time"
"github.com/jackc/pgx/v5"
"git.warky.dev/wdevs/relspecgo/pkg/models"
"git.warky.dev/wdevs/relspecgo/pkg/pgsql"
"git.warky.dev/wdevs/relspecgo/pkg/readers"
@@ -27,36 +28,12 @@ type Writer struct {
executor *TemplateExecutor
}
// ExecutionReport tracks the execution status of SQL statements
type ExecutionReport struct {
TotalStatements int `json:"total_statements"`
ExecutedStatements int `json:"executed_statements"`
FailedStatements int `json:"failed_statements"`
Schemas []SchemaReport `json:"schemas"`
Errors []ExecutionError `json:"errors,omitempty"`
StartTime string `json:"start_time"`
EndTime string `json:"end_time"`
}
// SchemaReport tracks execution per schema
type SchemaReport struct {
Name string `json:"name"`
Tables []TableReport `json:"tables"`
}
// TableReport tracks execution per table
type TableReport struct {
Name string `json:"name"`
Created bool `json:"created"`
Error string `json:"error,omitempty"`
}
// ExecutionError represents a failed statement
type ExecutionError struct {
StatementNumber int `json:"statement_number"`
Statement string `json:"statement"`
Error string `json:"error"`
}
// Shared execution results retain the PostgreSQL package's public type names.
type ExecutionReport = writers.ExecutionReport
type ExecutionFailure = writers.ExecutionFailure
type SchemaReport = writers.SchemaReport
type TableReport = writers.TableReport
type ExecutionError = writers.ExecutionError
// NewWriter creates a new PostgreSQL SQL writer
func NewWriter(options *writers.WriterOptions) *Writer {
@@ -109,9 +86,20 @@ func (w *Writer) WriteDatabase(db *models.Database) error {
}
fmt.Fprintf(w.writer, "\n")
for _, schema := range db.Schemas {
preflights, err := w.generateTypePreflights(schema)
if err != nil {
return err
}
for _, stmt := range preflights {
if _, err := fmt.Fprintln(w.writer, stmt); err != nil {
return err
}
}
}
// Process each schema in the database
for _, schema := range db.Schemas {
if err := w.WriteSchema(schema); err != nil {
if err := w.writeSchema(schema); err != nil {
return fmt.Errorf("failed to write schema %s: %w", schema.Name, err)
}
}
@@ -129,9 +117,16 @@ func (w *Writer) GenerateDatabaseStatements(db *models.Database) ([]string, erro
statements = append(statements, fmt.Sprintf("-- Database: %s", db.Name))
statements = append(statements, "-- Generated by RelSpec")
for _, schema := range db.Schemas {
preflights, err := w.generateTypePreflights(schema)
if err != nil {
return nil, err
}
statements = append(statements, preflights...)
}
// Process each schema in the database
for _, schema := range db.Schemas {
schemaStatements, err := w.GenerateSchemaStatements(schema)
schemaStatements, err := w.generateSchemaStatements(schema)
if err != nil {
return nil, fmt.Errorf("failed to generate statements for schema %s: %w", schema.Name, err)
}
@@ -187,6 +182,15 @@ 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) {
preflights, err := w.generateTypePreflights(schema)
if err != nil {
return nil, err
}
statements, err := w.generateSchemaStatements(schema)
return append(preflights, statements...), err
}
func (w *Writer) generateSchemaStatements(schema *models.Schema) ([]string, error) {
statements := []string{}
if err := w.checkDirectives(schema); err != nil {
@@ -510,15 +514,11 @@ func (w *Writer) GenerateAlterColumnTypeStatements(schema *models.Schema) ([]str
// rather than risk an ALTER COLUMN TYPE Postgres won't accept cleanly.
continue
}
targetType := effectiveAlterColumnSQLType(col)
stmt, err := w.executor.ExecuteAlterColumnTypeWithCheck(AlterColumnTypeWithCheckData{
SchemaName: schema.Name,
TableName: table.Name,
ColumnName: col.Name,
NewType: targetType,
EquivalentTypes: equivalentTypeListSQL(targetType),
UsingExpr: buildAlterColumnUsingExpression(col.Name, targetType),
})
data, err := conversionData(schema.Name, table.Name, col)
if err != nil {
return nil, err
}
stmt, err := w.executor.ExecuteAlterColumnTypeWithCheck(data)
if err != nil {
return nil, fmt.Errorf("failed to generate alter column type for %s.%s.%s: %w", schema.Name, table.Name, col.Name, err)
}
@@ -774,6 +774,22 @@ func (w *Writer) WriteSchema(schema *models.Schema) error {
if w.writer == nil {
w.writer = os.Stdout
}
preflights, err := w.generateTypePreflights(schema)
if err != nil {
return err
}
for _, stmt := range preflights {
if _, err := fmt.Fprintln(w.writer, stmt); err != nil {
return err
}
}
return w.writeSchema(schema)
}
func (w *Writer) writeSchema(schema *models.Schema) error {
if w.writer == nil {
w.writer = os.Stdout
}
if err := w.checkDirectives(schema); err != nil {
return err
@@ -2054,7 +2070,8 @@ func extractSequenceName(defaultExpr string) string {
func (w *Writer) executeDatabaseSQL(db *models.Database, connString string) error {
// Initialize execution report
w.executionReport = &ExecutionReport{
StartTime: getCurrentTimestamp(),
Dialect: "PostgreSQL",
StartTime: time.Now().Format(time.RFC3339Nano),
Schemas: make([]SchemaReport, 0),
Errors: make([]ExecutionError, 0),
}
@@ -2067,7 +2084,8 @@ func (w *Writer) executeDatabaseSQL(db *models.Database, connString string) erro
} else {
statements = diffStatements
if len(statements) == 0 {
w.executionReport.EndTime = getCurrentTimestamp()
w.executionReport.EndTime = time.Now().Format(time.RFC3339Nano)
w.executionReport.Outcome = "no_changes"
fmt.Fprintln(os.Stderr, "✓ Database is already up to date; nothing to execute")
return w.finishReport()
}
@@ -2179,12 +2197,17 @@ func (w *Writer) diffStatements(model, current *models.Database) ([]string, erro
return nil, fmt.Errorf("failed to generate migration scripts: %w", err)
}
var preflights []string
lastSchema := ""
for _, script := range scripts {
body := strings.TrimSpace(script.Body)
if body == "" {
continue
}
if script.ObjectType == "preflight column type" {
preflights = append(preflights, body)
continue
}
if script.Schema != "" && script.Schema != lastSchema {
statements = append(statements, fmt.Sprintf("-- Schema: %s", script.Schema))
lastSchema = script.Schema
@@ -2192,150 +2215,84 @@ func (w *Writer) diffStatements(model, current *models.Database) ([]string, erro
statements = append(statements, body)
}
statements = append(statements, setvalStatements...)
statements = append(preflights, statements...)
return statements, nil
}
// executeStatements runs statements one by one against the database and writes the report.
func (w *Writer) executeStatements(statements []string, connString string) error {
w.executionReport.TotalStatements = len(statements)
// Connect to database
fmt.Fprintln(os.Stderr, " → Connecting to PostgreSQL output database...")
ctx := context.Background()
conn, err := pgsql.Connect(ctx, connString, "writer-pgsql")
if err != nil {
return fmt.Errorf("failed to connect to database: %w", err)
err = writers.FailExecution(w.options, "PostgreSQL", writers.StatementOperations(statements), fmt.Errorf("failed to connect to database: %w", err))
w.executionReport = w.options.Metadata["execution_report"].(*ExecutionReport)
return err
}
defer conn.Close(ctx)
fmt.Fprintln(os.Stderr, " → Connected. Executing statements...")
// Track schemas and tables
schemaMap := make(map[string]*SchemaReport)
w.executionReport.Dialect = "PostgreSQL"
var tx pgx.Tx
exec := func(ctx context.Context, i int, stmt string) error {
fmt.Fprintf(os.Stderr, "Executing statement %d/%d [%s] %s...\n", i+1, len(statements), detectStatementType(stmt), extractStatementContext(stmt))
if tx != nil {
_, err := tx.Exec(ctx, stmt)
return err
}
_, err := conn.Exec(ctx, stmt)
return err
}
backend := writers.ExecutionBackend{
Exec: exec,
Begin: func(ctx context.Context) error { var err error; tx, err = conn.Begin(ctx); return err },
Commit: func(ctx context.Context) error { return tx.Commit(ctx) },
Rollback: func(ctx context.Context) error { return tx.Rollback(ctx) },
}
schemaNames := make([]string, len(statements))
currentSchema := ""
// Execute each statement
for i, stmt := range statements {
stmtTrimmed := strings.TrimSpace(stmt)
// Skip comments
if strings.HasPrefix(stmtTrimmed, "--") {
// Check if this is a schema comment to track schema changes
if strings.Contains(stmtTrimmed, "Schema:") {
parts := strings.Split(stmtTrimmed, "Schema:")
if len(parts) > 1 {
currentSchema = strings.TrimSpace(parts[1])
if _, exists := schemaMap[currentSchema]; !exists {
schemaReport := SchemaReport{
Name: currentSchema,
Tables: make([]TableReport, 0),
}
schemaMap[currentSchema] = &schemaReport
}
}
}
continue
}
// Skip empty statements
if stmtTrimmed == "" {
continue
}
stmtType := detectStatementType(stmtTrimmed)
stmtCtx := extractStatementContext(stmtTrimmed)
if stmtCtx != "" {
fmt.Fprintf(os.Stderr, "Executing statement %d/%d [%s] %s...\n", i+1, len(statements), stmtType, stmtCtx)
} else {
fmt.Fprintf(os.Stderr, "Executing statement %d/%d [%s]...\n", i+1, len(statements), stmtType)
}
_, execErr := conn.Exec(ctx, stmt)
if execErr != nil {
w.executionReport.FailedStatements++
execError := ExecutionError{
StatementNumber: i + 1,
Statement: truncateStatement(stmt),
Error: execErr.Error(),
}
w.executionReport.Errors = append(w.executionReport.Errors, execError)
// Track table creation failure
if strings.Contains(strings.ToUpper(stmtTrimmed), "CREATE TABLE") && currentSchema != "" {
tableName := extractTableNameFromCreate(stmtTrimmed)
if tableName != "" && schemaMap[currentSchema] != nil {
schemaMap[currentSchema].Tables = append(schemaMap[currentSchema].Tables, TableReport{
Name: tableName,
Created: false,
Error: execErr.Error(),
})
}
}
// Continue with next statement instead of failing completely
fmt.Fprintf(os.Stderr, "⚠ Warning: Statement %d failed: %v\n", i+1, execErr)
continue
}
w.executionReport.ExecutedStatements++
// Track successful table creation
if strings.Contains(strings.ToUpper(stmtTrimmed), "CREATE TABLE") && currentSchema != "" {
tableName := extractTableNameFromCreate(stmtTrimmed)
if tableName != "" && schemaMap[currentSchema] != nil {
schemaMap[currentSchema].Tables = append(schemaMap[currentSchema].Tables, TableReport{
Name: tableName,
Created: true,
})
}
if strings.HasPrefix(strings.TrimSpace(stmt), "-- Schema:") {
currentSchema = strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(stmt), "-- Schema:"))
}
schemaNames[i] = currentSchema
}
// Convert schema map to slice
for _, schemaReport := range schemaMap {
w.executionReport.Schemas = append(w.executionReport.Schemas, *schemaReport)
schemaIndexes := make(map[string]int)
// Populate per-schema details before finalizing the shared report.
backend.Exec = func(ctx context.Context, i int, stmt string) error {
err := exec(ctx, i, stmt)
if strings.Contains(strings.ToUpper(stmt), "CREATE TABLE") {
schemaName := schemaNames[i]
if schemaName != "" {
idx, exists := schemaIndexes[schemaName]
if !exists {
idx = -1
}
if idx < 0 {
w.executionReport.Schemas = append(w.executionReport.Schemas, SchemaReport{Name: schemaName, Tables: []TableReport{}})
idx = len(w.executionReport.Schemas) - 1
schemaIndexes[schemaName] = idx
}
table := TableReport{Name: extractTableNameFromCreate(stmt), Created: err == nil}
if err != nil {
table.Error = err.Error()
}
w.executionReport.Schemas[idx].Tables = append(w.executionReport.Schemas[idx].Tables, table)
}
}
return err
}
w.executionReport.EndTime = getCurrentTimestamp()
return w.finishReport()
return writers.RunExecution(ctx, w.options, w.executionReport, writers.StatementOperations(statements), backend)
}
// finishReport writes the optional report file and prints the execution summary.
func (w *Writer) finishReport() error {
// Write report if path is specified
if reportPath, ok := w.options.Metadata["report_path"].(string); ok && reportPath != "" {
if err := w.writeReport(reportPath); err != nil {
fmt.Fprintf(os.Stderr, "⚠ Warning: Failed to write report: %v\n", err)
} else {
fmt.Fprintf(os.Stderr, "✓ Report written to: %s\n", reportPath)
}
}
var err error
if w.executionReport.FailedStatements > 0 {
fmt.Fprintf(os.Stderr, "⚠ Completed with %d errors out of %d statements\n",
w.executionReport.FailedStatements, w.executionReport.TotalStatements)
} else {
fmt.Fprintf(os.Stderr, "✓ Successfully executed %d statements\n", w.executionReport.ExecutedStatements)
err = &ExecutionFailure{Report: w.executionReport}
}
return nil
return writers.FinishExecution(w.options, w.executionReport, err)
}
// writeReport writes the execution report to a JSON file
func (w *Writer) writeReport(reportPath string) error {
file, err := os.Create(reportPath)
if err != nil {
return fmt.Errorf("failed to create report file: %w", err)
}
defer file.Close()
encoder := json.NewEncoder(file)
encoder.SetIndent("", " ")
if err := encoder.Encode(w.executionReport); err != nil {
return fmt.Errorf("failed to encode report: %w", err)
}
return nil
return writers.WriteExecutionReport(reportPath, w.executionReport)
}
// extractTableNameFromCreate extracts table name from CREATE TABLE statement
+14 -30
View File
@@ -1016,15 +1016,9 @@ func TestGenerateAddColumnStatements(t *testing.T) {
// Verify expected elements
expectedStrings := []string{
"ALTER TABLE public.users ADD COLUMN id integer NOT NULL",
"ALTER TABLE public.users ADD COLUMN email varchar(255) NOT NULL",
"ALTER TABLE public.users ADD COLUMN status text DEFAULT 'active'",
"information_schema.columns",
"table_schema = 'public'",
"table_name = 'users'",
"column_name = 'id'",
"column_name = 'email'",
"column_name = 'status'",
"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 {
@@ -1033,12 +1027,6 @@ func TestGenerateAddColumnStatements(t *testing.T) {
}
}
// Verify DO blocks are present for conditional adds
doBlockCount := strings.Count(output, "DO $$")
if doBlockCount < 3 {
t.Errorf("Expected at least 3 DO blocks (one per column), got %d", doBlockCount)
}
// Verify IF NOT EXISTS logic
ifNotExistsCount := strings.Count(output, "IF NOT EXISTS")
if ifNotExistsCount < 3 {
@@ -1083,14 +1071,14 @@ 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 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 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, "DO $$") {
t.Errorf("Output missing DO block\nFull output:\n%s", output)
if strings.Contains(output, "information_schema.columns") {
t.Errorf("column additions should use native IF NOT EXISTS\nFull output:\n%s", output)
}
}
@@ -1206,7 +1194,7 @@ func TestWriteSchema_AlterColumnDefaultStripsBackticksFromFunctionExpression(t *
}
}
func TestWriteSchema_GuardedAlterColumnTypeFallsBackOnConversionFailure(t *testing.T) {
func TestWriteSchema_GuardedAlterColumnTypePreservesConversionFailure(t *testing.T) {
db := models.InitDatabase("testdb")
schema := models.InitSchema("public")
@@ -1228,17 +1216,13 @@ func TestWriteSchema_GuardedAlterColumnTypeFallsBackOnConversionFailure(t *testi
}
output := buf.String()
if !strings.Contains(output, "EXCEPTION WHEN OTHERS THEN") {
t.Fatalf("expected guarded alter to fall back on conversion failure, got:\n%s", output)
if !strings.Contains(output, "ALTER COLUMN name TYPE integer USING name::integer;") {
t.Fatalf("expected explicit type conversion, got:\n%s", output)
}
if !strings.Contains(output, "renamed_column := 'name_' || trim(both '_' from regexp_replace(lower(current_type)") {
t.Fatalf("expected fallback to derive a renamed column name from the live type, got:\n%s", output)
}
if !strings.Contains(output, "RENAME COLUMN name TO %I") {
t.Fatalf("expected fallback to rename the existing column, got:\n%s", output)
}
if !strings.Contains(output, "ADD COLUMN name integer") {
t.Fatalf("expected fallback to add a fresh column with the new type, got:\n%s", output)
for _, unsafe := range []string{"EXCEPTION WHEN OTHERS", "renamed_column", "RENAME COLUMN"} {
if strings.Contains(output, unsafe) {
t.Fatalf("type conversion must propagate errors without replacing the column (%s):\n%s", unsafe, output)
}
}
}
+24 -5
View File
@@ -186,11 +186,30 @@ Future support planned for:
- SQLite
- Generic SQL via database/sql
## Transaction Behavior
## Execution and transaction behavior
**Current**: Each script executes in its own implicit transaction (PostgreSQL default behavior)
This is a raw PostgreSQL script runner, not a migration manager. It has no applied
history, checksums, migration lock, or duplicate-run protection. Re-running repeats
the supplied scripts. Scripts may contain transaction control, so RelSpec does not
wrap them in a transaction and rejects `WriterOptions.AtomicExecution`.
**Future Enhancement**: Option to wrap all scripts in a single transaction for atomic execution with rollback on error.
By default execution stops on the first SQL or embed-preparation failure. Set
`WriterOptions.ContinueOnError`, legacy `Metadata["ignore_errors"]`, or CLI
`--ignore-errors` / `--continue-on-error` / `--coe` to attempt later scripts. Any
failure still returns a nonzero result and prevents dependent jobs from running.
The shared `writers.ExecutionReport` is available at `Metadata["execution_report"]`;
`Metadata["report_path"]` requests a JSON report and report-write failures are errors.
Counters and operations cover every schema in the invocation, including scripts
skipped after stopping or because they are empty. Legacy `execution_total`,
`execution_success`, `execution_failed`, `execution_skipped`, and
`execution_rolled_back` metadata counters are also populated on failure.
Reports use `outcome: script_managed` and successful script status `executed`, rather
than claiming committed database changes. A script may commit, roll back, or leave a
transaction open, and a failed script may already have committed work. The report
records which scripts succeeded/failed/skipped; it cannot infer statement-level
commit or rollback inside arbitrary scripts. The connection closes after execution.
## Performance Considerations
@@ -226,7 +245,7 @@ docker run -d --name postgres-test \
postgres:16
# Run your integration tests
go test -tags=integration ./pkg/writers/sqlexec/
RELSPEC_TEST_PG_CONN='postgres://postgres:test@127.0.0.1:5432/testdb?sslmode=disable' go test -count=1 ./pkg/writers/sqlexec/
# Cleanup
docker stop postgres-test
@@ -237,7 +256,7 @@ docker rm postgres-test
- `WriteTable()` is not supported (returns error)
- Requires PostgreSQL connection (no offline mode)
- No built-in transaction wrapping (yet)
- No executor-level transaction wrapping for raw scripts
- No rollback script support (yet, though `models.Script.Rollback` field exists)
## Related
+44 -105
View File
@@ -42,22 +42,25 @@ func (w *Writer) WriteDatabase(db *models.Database) error {
return fmt.Errorf("connection_string is required in writer metadata")
}
// Preserve schema order and script ordering within each schema, but aggregate once.
var scripts []*models.Script
for _, schema := range db.Schemas {
for _, script := range sortedScripts(schema.Scripts) {
copy := *script
copy.Name = schema.Name + "." + script.Name
scripts = append(scripts, &copy)
}
}
// Connect to database
ctx := context.Background()
conn, err := pgsql.Connect(ctx, connString, "writer-sqlexec")
if err != nil {
return fmt.Errorf("failed to connect to database: %w", err)
return writers.FailExecution(w.options, "PostgreSQL scripts", scriptOperations(scripts), fmt.Errorf("failed to connect to database: %w", err))
}
defer conn.Close(ctx)
// Execute scripts from all schemas
for _, schema := range db.Schemas {
if err := w.executeScripts(ctx, conn, schema.Scripts); err != nil {
return fmt.Errorf("failed to execute scripts from schema %s: %w", schema.Name, err)
}
}
return nil
return w.runScripts(ctx, conn, scripts)
}
// WriteSchema executes all scripts from a single schema
@@ -76,7 +79,7 @@ func (w *Writer) WriteSchema(schema *models.Schema) error {
ctx := context.Background()
conn, err := pgsql.Connect(ctx, connString, "writer-sqlexec")
if err != nil {
return fmt.Errorf("failed to connect to database: %w", err)
return writers.FailExecution(w.options, "PostgreSQL scripts", scriptOperations(sortedScripts(schema.Scripts)), fmt.Errorf("failed to connect to database: %w", err))
}
defer conn.Close(ctx)
@@ -93,113 +96,49 @@ func (w *Writer) WriteTable(table *models.Table) error {
return fmt.Errorf("WriteTable is not supported for SQL script execution")
}
// executeScripts executes scripts in Priority, Sequence, then Name order
// executeScripts executes scripts in Priority, Sequence, then Name order.
func (w *Writer) executeScripts(ctx context.Context, conn *pgx.Conn, scripts []*models.Script) error {
if len(scripts) == 0 {
return nil
}
return w.runScripts(ctx, conn, sortedScripts(scripts))
}
// Check if we should ignore errors
ignoreErrors := false
if val, ok := w.options.Metadata["ignore_errors"].(bool); ok {
ignoreErrors = val
}
// Track failed scripts and execution counts
var failedScripts []struct {
name string
priority int
sequence uint
err error
}
successCount := 0
totalCount := 0
// Sort scripts by Priority (ascending), Sequence (ascending), then Name (ascending)
sortedScripts := make([]*models.Script, len(scripts))
copy(sortedScripts, scripts)
sort.Slice(sortedScripts, func(i, j int) bool {
if sortedScripts[i].Priority != sortedScripts[j].Priority {
return sortedScripts[i].Priority < sortedScripts[j].Priority
func sortedScripts(scripts []*models.Script) []*models.Script {
ordered := append([]*models.Script(nil), scripts...)
sort.SliceStable(ordered, func(i, j int) bool {
if ordered[i].Priority != ordered[j].Priority {
return ordered[i].Priority < ordered[j].Priority
}
if sortedScripts[i].Sequence != sortedScripts[j].Sequence {
return sortedScripts[i].Sequence < sortedScripts[j].Sequence
if ordered[i].Sequence != ordered[j].Sequence {
return ordered[i].Sequence < ordered[j].Sequence
}
return sortedScripts[i].Name < sortedScripts[j].Name
return ordered[i].Name < ordered[j].Name
})
return ordered
}
// Execute each script in order
for _, script := range sortedScripts {
if script.SQL == "" {
continue
}
totalCount++
fmt.Printf("Executing script: %s (Priority=%d, Sequence=%d)\n",
script.Name, script.Priority, script.Sequence)
// Execute the SQL script
func (w *Writer) runScripts(ctx context.Context, conn *pgx.Conn, scripts []*models.Script) error {
operations := scriptOperations(scripts)
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)
if err != nil {
if ignoreErrors {
fmt.Printf("⚠ Error preparing %s: %v (continuing due to --ignore-errors)\n", script.Name, err)
failedScripts = append(failedScripts, struct {
name string
priority int
sequence uint
err error
}{
name: script.Name,
priority: script.Priority,
sequence: script.Sequence,
err: err,
})
continue
}
return fmt.Errorf("script %s (Priority=%d, Sequence=%d): %w",
script.Name, script.Priority, script.Sequence, err)
return fmt.Errorf("preparing script %s: %w", script.Name, err)
}
_, err = conn.Exec(ctx, sql)
if err != nil {
if ignoreErrors {
fmt.Printf("⚠ Error executing %s: %v (continuing due to --ignore-errors)\n", script.Name, err)
failedScripts = append(failedScripts, struct {
name string
priority int
sequence uint
err error
}{
name: script.Name,
priority: script.Priority,
sequence: script.Sequence,
err: err,
})
continue
}
return fmt.Errorf("script %s (Priority=%d, Sequence=%d): %w",
script.Name, script.Priority, script.Sequence, err)
if _, err := conn.Exec(ctx, sql); err != nil {
return fmt.Errorf("script %s: %w", script.Name, err)
}
return nil
}}
// Supplied scripts can manage transactions themselves. Do not promise whole-script rollback.
return writers.RunExecution(ctx, w.options, &writers.ExecutionReport{Dialect: "PostgreSQL scripts"}, operations, backend)
}
successCount++
fmt.Printf("✓ Successfully executed: %s\n", script.Name)
func scriptOperations(scripts []*models.Script) []writers.ExecutionOperation {
operations := make([]writers.ExecutionOperation, len(scripts))
for i, script := range scripts {
operations[i] = writers.ExecutionOperation{Name: script.Name, Statement: script.SQL}
}
// Store execution results in metadata for caller
w.options.Metadata["execution_total"] = totalCount
w.options.Metadata["execution_success"] = successCount
w.options.Metadata["execution_failed"] = len(failedScripts)
// Print summary of failed scripts if any
if len(failedScripts) > 0 {
fmt.Printf("\n⚠ Failed Scripts Summary (%d failed):\n", len(failedScripts))
for i, failed := range failedScripts {
fmt.Printf(" %d. %s (Priority=%d, Sequence=%d)\n Error: %v\n",
i+1, failed.name, failed.priority, failed.sequence, failed.err)
}
}
return nil
return operations
}
func processEmbedDirectives(script *models.Script) (string, error) {
+29 -6
View File
@@ -2,6 +2,7 @@ package sqlexec
import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
@@ -121,7 +122,7 @@ func TestLive_ExecuteScriptsOrder(t *testing.T) {
if strings.Join(got, ",") != strings.Join(want, ",") {
t.Errorf("execution order = %v, want %v", got, want)
}
if opts.Metadata["execution_total"] != 6 || opts.Metadata["execution_success"] != 6 || opts.Metadata["execution_failed"] != 0 {
if opts.Metadata["execution_total"] != 7 || opts.Metadata["execution_success"] != 6 || opts.Metadata["execution_failed"] != 0 {
t.Errorf("counts: %v", opts.Metadata)
}
}
@@ -160,8 +161,8 @@ func TestLive_IgnoreErrorsContinues(t *testing.T) {
{Name: "02_ok", Priority: 2, SQL: fmt.Sprintf("CREATE TABLE %s.after(id int);", schema)},
}
opts := liveOptions(connString, map[string]interface{}{"ignore_errors": true})
if err := NewWriter(opts).WriteSchema(&models.Schema{Name: schema, Scripts: scripts}); err != nil {
t.Fatalf("ignore_errors must not fail: %v", err)
if err := NewWriter(opts).WriteSchema(&models.Schema{Name: schema, Scripts: scripts}); err == nil {
t.Fatal("continuation must still return failure")
}
if opts.Metadata["execution_total"] != 2 || opts.Metadata["execution_success"] != 1 || opts.Metadata["execution_failed"] != 1 {
t.Errorf("counts: %v", opts.Metadata)
@@ -186,8 +187,8 @@ func TestLive_EmbedDirectiveErrorHandling(t *testing.T) {
}
opts := liveOptions(connString, map[string]interface{}{"ignore_errors": true})
if err := NewWriter(opts).WriteSchema(&models.Schema{Name: schema, Scripts: []*models.Script{bad}}); err != nil {
t.Errorf("ignore_errors: %v", err)
if err := NewWriter(opts).WriteSchema(&models.Schema{Name: schema, Scripts: []*models.Script{bad}}); err == nil {
t.Error("embed preparation failures must propagate even when continuing")
}
if opts.Metadata["execution_failed"] != 1 {
t.Errorf("counts: %v", opts.Metadata)
@@ -217,7 +218,29 @@ func TestLive_WriteDatabaseMultiSchema(t *testing.T) {
// A failure in one schema aborts and names that schema.
db.Schemas[1].Scripts[0].SQL = "SELECT * FROM definitely_missing_table;"
err := NewWriter(liveOptions(connString, nil)).WriteDatabase(db)
if err == nil || !strings.Contains(err.Error(), "schema "+s2) {
if err == nil || !strings.Contains(err.Error(), s2+".b") {
t.Errorf("expected error naming schema %s, got %v", s2, err)
}
}
func TestLive_MultiSchemaFailuresAggregate(t *testing.T) {
connection := liveConn(t)
opts := liveOptions(connection, nil)
opts.ContinueOnError = true
db := &models.Database{Schemas: []*models.Schema{
{Name: "one", Scripts: []*models.Script{{Name: "bad", SQL: "SELECT 1 / 0"}, {Name: "empty"}}},
{Name: "two", Scripts: []*models.Script{{Name: "ok", SQL: "SELECT 1"}}},
}}
err := NewWriter(opts).WriteDatabase(db)
var failure *writers.ExecutionFailure
if !errors.As(err, &failure) {
t.Fatalf("expected structured failure: %v", err)
}
report := failure.Report
if report.TotalStatements != 3 || report.FailedStatements != 1 || report.ExecutedStatements != 1 || report.SkippedStatements != 1 || report.Outcome != "script_managed" {
t.Fatalf("incorrect aggregate: %+v", report)
}
if report.Operations[0].Name != "one.bad" || report.Operations[2].Name != "two.ok" || report.Operations[2].Status != "executed" {
t.Fatalf("script details: %+v", report.Operations)
}
}
+29 -30
View File
@@ -117,50 +117,49 @@ func (c *statementCollector) Write(p []byte) (int, error) {
// against the SQLite database file at dbPath.
func (w *Writer) executeDatabaseSQL(db *models.Database, dbPath string) error {
collector := &statementCollector{}
saved := w.writer
w.writer = collector
defer func() { w.writer = saved }()
if err := w.writeContent(db); err != nil {
return fmt.Errorf("failed to generate SQL statements: %w", err)
}
conn, err := sql.Open("sqlite", dbPath)
if err != nil {
return fmt.Errorf("failed to open sqlite database %q: %w", dbPath, err)
return writers.FailExecution(w.options, "SQLite", writers.StatementOperations(collector.statements), fmt.Errorf("failed to open sqlite database %q: %w", dbPath, err))
}
defer conn.Close()
// Pin one connection so connection-local settings (including foreign_keys) persist.
ctx := context.Background()
ignoreErrors := false
if val, ok := w.options.Metadata["ignore_errors"].(bool); ok {
ignoreErrors = val
session, err := conn.Conn(ctx)
if err != nil {
return writers.FailExecution(w.options, "SQLite", writers.StatementOperations(collector.statements), err)
}
total, executed := 0, 0
var execErrors []string
for _, stmt := range collector.statements {
if strings.HasPrefix(stmt, "--") {
continue
}
total++
if _, err := conn.ExecContext(ctx, stmt); err != nil {
execErrors = append(execErrors, fmt.Sprintf("statement %d (%s): %v", total, truncateStatement(stmt), err))
if !ignoreErrors {
break
defer session.Close()
var tx *sql.Tx
backend := writers.ExecutionBackend{
Exec: func(ctx context.Context, _ int, stmt string) error {
if tx != nil {
_, err := tx.ExecContext(ctx, stmt)
return err
}
continue
}
executed++
_, err := session.ExecContext(ctx, stmt)
return err
},
Begin: func(ctx context.Context) error {
// SQLite ignores foreign_keys changes inside transactions.
if _, err := session.ExecContext(ctx, "PRAGMA foreign_keys=ON"); err != nil {
return err
}
var err error
tx, err = session.BeginTx(ctx, nil)
return err
},
Commit: func(context.Context) error { return tx.Commit() },
Rollback: func(context.Context) error { return tx.Rollback() },
}
w.options.Metadata["execution_total"] = total
w.options.Metadata["execution_success"] = executed
w.options.Metadata["execution_failed"] = len(execErrors)
if len(execErrors) > 0 {
return fmt.Errorf("failed to execute %d/%d statement(s) against %q:\n%s", len(execErrors), total, dbPath, strings.Join(execErrors, "\n"))
}
return nil
return writers.RunExecution(ctx, w.options, &writers.ExecutionReport{Dialect: "SQLite"}, writers.StatementOperations(collector.statements), backend)
}
// truncateStatement shortens a SQL statement for error messages.
+35 -1
View File
@@ -2,6 +2,7 @@ package sqlite
import (
"database/sql"
"errors"
"os"
"path/filepath"
"strings"
@@ -182,7 +183,7 @@ func TestExecute_StopsOnErrorUnlessIgnored(t *testing.T) {
path := prepare(t)
opts := &writers.WriterOptions{Metadata: map[string]any{"connection_string": path}}
err := NewWriter(opts).WriteDatabase(shopDB())
if err == nil || !strings.Contains(err.Error(), "failed to execute") {
if err == nil || !strings.Contains(err.Error(), "execution failed") {
t.Fatalf("expected failure, got %v", err)
}
if opts.Metadata["execution_failed"] != 1 {
@@ -248,3 +249,36 @@ func TestDBMLFixtureExecutes(t *testing.T) {
t.Errorf("nothing executed: %+v", opts.Metadata)
}
}
func TestExecuteAtomicRollback(t *testing.T) {
path := filepath.Join(t.TempDir(), "atomic.db")
opts := &writers.WriterOptions{AtomicExecution: true, Metadata: map[string]any{"connection_string": path}}
db := models.InitDatabase("atomic")
schema := models.InitSchema("main")
for _, name := range []string{"first", "bad", "last"} {
table := models.InitTable(name, "main")
col := models.InitColumn("id", name, "main")
col.Type = "integer"
table.Columns[col.Name] = col
schema.Tables = append(schema.Tables, table)
}
// A duplicate column creates an invalid table definition in the middle.
bad := schema.Tables[1]
bad.Columns["duplicate"] = models.InitColumn("id", "bad", "main")
bad.Columns["duplicate"].Type = "integer"
db.Schemas = append(db.Schemas, schema)
err := NewWriter(opts).WriteDatabase(db)
var failure *writers.ExecutionFailure
if !errors.As(err, &failure) || failure.Report.Outcome != "rolled_back" || failure.Report.RolledBackStatements == 0 || failure.Report.ExecutedStatements != 0 {
t.Fatalf("rollback report: %v", err)
}
conn, err := sql.Open("sqlite", path)
if err != nil {
t.Fatal(err)
}
defer conn.Close()
var count int
if err := conn.QueryRow("SELECT count(*) FROM sqlite_master WHERE type='table'").Scan(&count); err != nil || count != 0 {
t.Fatalf("tables survived rollback: %d %v", count, err)
}
}
+6
View File
@@ -99,8 +99,14 @@ type WriterOptions struct {
// ContinueOnError instructs SQL writers to prepend `\set ON_ERROR_STOP off`
// to their output so that psql continues past errors instead of stopping.
// Live SQL execution also continues when set, but still returns an error
// if any statement fails.
ContinueOnError bool
// AtomicExecution applies generated DDL in one transaction. It cannot be
// combined with ContinueOnError; raw script execution rejects it.
AtomicExecution bool
// Additional options can be added here as needed
Metadata map[string]interface{}
}