Repository navigation
Conversation
271e475 to
8201d93
Compare
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The adapter refactor is coherent, documented, and supported by comprehensive regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Moves schema-operation safety validation into dialect adapters, using live metadata before generation, planning, and execution.
Changes:
- Adds MySQL/MariaDB live-metadata validation.
- Improves squash preservation, baseline selection, and cleanup.
- Adds documentation and regression coverage.
| File | Description |
|---|---|
README.md |
Documents validation and squash guarantees. |
DESIGN.md |
Describes adapter validation architecture. |
packages/cli/src/commands/apply.ts |
Integrates preflight validation into apply and plan. |
packages/cli/src/commands/apply.test.ts |
Tests plan validation and failure handling. |
packages/cli/src/commands/generate.ts |
Validates migrations and safely replaces squash sources. |
packages/cli/src/dev/database.ts |
Creates clean squash baselines and improves cleanup. |
packages/cli/src/dev/database.test.ts |
Tests baseline selection and cleanup. |
packages/cli/src/dialect/mysql-column-metadata.ts |
Centralizes native column metadata reads. |
packages/cli/src/dialect/mysql-compatible.ts |
Reuses the metadata reader during introspection. |
packages/cli/src/dialect/mysql-schema-adapter.ts |
Implements MySQL/MariaDB validation policy. |
packages/cli/src/dialect/mysql-validation.test.ts |
Covers unsafe and stale column metadata. |
packages/cli/src/dialect/mysql.test.ts |
Updates adapter and introspection expectations. |
packages/cli/src/dialect/schema-adapter.ts |
Adds the validation adapter contract. |
packages/cli/src/dialect/sqlite.ts |
Lazily initializes SQLite connections. |
packages/cli/src/dialect/types.ts |
Removes shared validation verdict metadata. |
packages/cli/src/introspector.ts |
Stops exposing validation verdicts. |
packages/cli/src/migration.test.ts |
Tests validation ordering and planning. |
packages/cli/src/migration.ts |
Validates reconciled migration operations. |
packages/cli/src/operations/column/alterColumn.ts |
Removes construction-time dialect policy. |
packages/cli/src/operations/shared/utils.ts |
Removes the shared safety guard. |
packages/cli/tests/generate_squash.test.ts |
Covers squash preservation and ID allocation. |
packages/cli/tests/introspector_constraints.test.ts |
Tests live-schema drift rejection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
I'm concerned about During actual application, the later migration can validate against live metadata after the earlier migration has executed. Planning cannot inspect that future state, so it rejects the sequence. This treats an unsafe change and a change whose safety cannot be established by the current validation approach as equivalent failures, preventing users from previewing an otherwise applicable migration sequence. Squashing can avoid this by creating the column in its final form, but it is not a general solution: some of the migrations may already have been applied in other environments, making rewriting their history impractical. At the same time, preflight validation of the entire pending sequence during actual application would help detect problems in later migrations before earlier ones are applied. However, adopting the current planning validation unchanged would also reject legitimate create-then-modify sequences. Without prescribing a specific implementation yet, I think we should consider explicitly tracking schema state transitions in execution order and validating each operation against the state at that point, rather than rejecting it simply because an earlier operation affected the same column or table. We could then allow transitions whose relevant metadata can be established, while remaining conservative when required attributes genuinely cannot be predicted. This would help align validation between planning and actual application without requiring users to rewrite valid migration histories. |
Motivation
This PR builds on the MySQL/MariaDB support merged in #238 and now targets
main. It contains the operation-validation refactor separately from the initial dialect feature so the changes can be reviewed independently.MySQL-compatible column modifications replace a full column definition. The initial safety guard stored an
alterationBlockedReasonon common column metadata and rejected changes while constructing generic operations. That mixed dialect-specific execution policy into schema inspection and diff construction, and application depended on a previously captured verdict rather than current database facts.The refactor moves that policy into the selected schema adapter and performs preflight validation before saving or executing schema changes.
Design Decision
Adapter-owned validation against live facts
SchemaAdapter.validateOperationsreceives a database connection and the reconciled operation list. Standard adapters perform no additional validation; MySQL and MariaDB read native column facts and reject full-definition changes that would lose unsupported attributes or restore a stale default.Introspection and validation share a native metadata reader, but common schema models and operation constructors no longer carry or interpret validation verdicts. The removed field was introduced by the unreleased parent feature, so no compatibility layer is needed.
Strict, explicit validation boundaries
Actual application validates each migration before its first application-schema operation, using the state left by preceding migrations. SQL planning instead validates the ordered pending sequence against a separate real metadata connection, because the collecting driver cannot answer catalog queries.
Changes requiring unknown future column metadata are rejected rather than guessed safe. This deliberately avoids a schema simulator and does not promise atomicity across migration files or protection against concurrent DDL. Planning errors take precedence over partial collected SQL.
Preserve squash inputs until a replacement is ready
Squash retains its sources through validation and replacement writing, allocates an unused migration ID, and uses exclusive file creation. Development squash uses a fresh baseline with superseded migrations excluded, rather than reusing a database that may already contain their effects. Existing persistent development instances remain untouched.
Filesystem replacement is not atomic; removal failures after a successful write still require inspection before application.
Summary