Skip to content

Add 'migrate adopt' bundle: live-DB diff, convention renames, baseline, drop guard - #323

Closed
DJGosnell wants to merge 18 commits into
masterfrom
migrate-adopt-existing-db
Closed

Add 'migrate adopt' bundle: live-DB diff, convention renames, baseline, drop guard#323
DJGosnell wants to merge 18 commits into
masterfrom
migrate-adopt-existing-db

Conversation

@DJGosnell

Copy link
Copy Markdown
Member

Summary

Adds the full "adopt an existing database" bundle so a legacy DB (commonly snake_case) can be brought under Quarry migrations without the multi-step manual dance, and without silently degrading systematic renames into data-losing DROP COLUMN + ADD COLUMN.

Six features, delivered together:

  1. --from-database <connstr> on migrate add / migrate diff — sources the comparison ("from") snapshot from a live database via introspection instead of the last project snapshot.
  2. Always-on convention-aware rename matching — a deterministic canonical-equality pre-pass in the core differ emits RENAME COLUMN/RENAME TABLE for case/separator-only renames (snake↔Pascal↔camel↔lower), so they can never silently become drop+add.
  3. --rename-map — explicit rename overrides (inline table.col=New,bare=New or @file), trusted verbatim, now validated against both the live DB and the project schema before anything is written.
  4. migrate baseline <name> — records a snapshot (from project schemas or a live DB) as status='applied' in __quarry_migrations without executing DDL.
  5. Data-loss drop guard — a DROP of a populated column/table on a DB-sourced diff aborts unless --allow-data-loss is passed.
  6. migrate adopt --from-database — one command wrapping 1–5: baseline the live state as an applied InitialCreate, then generate a single pending alignment migration to match the project schemas.

Shared refactor: introspection (connection-string build + introspector factory + per-table loop + metadata→SchemaSnapshot adapter) extracted into Schema/DatabaseSchemaReader.cs, used by scaffold, add, baseline, and adopt.

Source: current discussion (no tracking issue).

Reason for Change

Adopting an existing database was painful: migrate add only diffs schema-vs-last-snapshot (never schema-vs-live-DB), so onboarding required scaffolding to legacy names, snapshotting, hand-inserting a __quarry_migrations history row, swapping in the real schemas, and hand-authoring a rename migration. Worse, automatic rename detection used Levenshtein scoring where systematic snake_case→PascalCase renames landed in the borderline zone and could silently degrade to DROP + ADD (data loss). This bundle makes adoption a single command and makes convention renames deterministic and safe.

Impact

  • New CLI verbs migrate baseline, migrate adopt and flags --from-database / -d / --rename-map / --allow-data-loss.
  • Behavior change for all migrate add users — see Breaking Changes.
  • New tool source: Schema/DatabaseSchemaReader.cs, Schema/RenameMap.cs, Schema/MigrationHistoryWriter.cs, Schema/DropGuard.cs.
  • Full suite green: 3714 → 3796 after final rebase (0 failed / 0 skipped); +~70 new tests across the feature and the review-remediation pass.

Plan items implemented as specified

  • 1a Extract shared DatabaseSchemaReader from ScaffoldCommand (no behavior change).
  • 1b Metadata → SchemaSnapshot adapter (CLR types via ReverseTypeMapper, kind inferred from PK/FK lists, PK-backing indexes skipped, composite keys).
  • 2 NamingConventions.Canonicalize + always-on canonical rename pre-pass (not subject to the acceptRename reject callback).
  • 3 --rename-map parse (inline + @file) + forced-rename pre-transform for pairs below the Hungarian floor.
  • 4 MigrationHistoryWriter (ensure-table + mark-applied); squash refactored to use it.
  • 6 --from-database on add/diff via a shared ResolveFromSnapshotAsync (default path unchanged).
  • 7 --allow-data-loss drop guard on DB-sourced diffs.
  • 9 Docs: README.md, llm.md, llm-migrate.md (the manual "A4 dance" replaced by adopt).

Deviations from plan implemented

  • baseline / adopt implemented as methods on MigrateCommands (not separate *Command.cs classes) to reuse its private file-generation / dialect / connection helpers. Consequence: the command orchestration is covered by-composition (adapter, differ, guard, history writer tested in isolation) rather than by direct CLI unit tests.
  • Step 8 uncovered a snapshot field-asymmetry bug and added DatabaseSchemaReader.NormalizeForDiff: ProjectSchemaReader leaves identity/length/default/FK/index at defaults while the introspection adapter fills them richly, so a raw DB-vs-schema diff over-reports AlterColumn for nearly every column. Both sides are normalized to the reliably-shared subset (name/type/nullable/kind) for the diff; files are still generated from the rich snapshots. FK/index changes are intentionally out of scope for the alignment diff (see Migration note below).
  • baseline/adopt write a checksum sentinel "baseline" (mirrors squash's "squashed"); StrictChecksums=true will warn on baselined migrations (accepted, squash-consistent).

Gaps in original plan implemented (review remediation)

A structured review (0 H / 5 M / 10 L) was run on the integrated diff; all 5 M and the actionable L findings were fixed (5A + 6B; 4 dismissed as accepted/informational):

  • Schema-aware drop guard (F5, M) — a normalized diff strips the schema qualifier, so the guard now re-qualifies each drop with the live table's real schema (DropGuard.BuildTableSchemaMap/ResolveSchema). Previously a drop against a table in a non-default PostgreSQL/SqlServer schema could mis-count and bypass the guard. Verified end-to-end on real PostgreSQL (AdoptGuardPostgresTests).
  • Rename-map validation (F6/F7, M/L)RenameMap.Validate rejects a target absent from the project schema, a duplicate/colliding target, and warns on entries matching nothing. adopt now parses + validates the map before writing the baseline, so an invalid map can no longer leave a half-adopted database (baseline marked applied, then a crash).
  • Canonical table-rename schema transfer (F8, L) — a canonical table rename that also moves schema now carries oldSchemaName, instead of silently dropping the move.
  • Non-fresh adopt warning (F2, L)adopt warns when the project already has migrations (baseline is recorded at latest+1 and earlier versions are not reconciled).
  • Missing safety test (F9, M) — added AdoptGuardScenarioTests: the introspect→normalize→diff→guard pipeline flags a populated unmapped column (adopt aborts) and the --allow-data-loss branch is asserted.
  • Test coverage (F10, L) — added real-PostgreSQL multi-schema guard tests + DropGuard schema-qualification unit tests; history-table DDL parity test (F11) comparing the writer's DDL against the runtime's.
  • Dead-code cleanup (F12, L) — removed a duplicated AddParameter helper left in MigrateCommands after the squash refactor.

Migration Steps

  • Recommended path: quarry migrate adopt <Name> -c "<connstr>" -d <dialect> [--rename-map …], then await db.MigrateAsync(connection)InitialCreate is skipped and the alignment migration renames columns in place, preserving data.
  • The alignment diff focuses on columns (renames + type/nullability). It does not reconcile foreign keys or indexes captured only in the baseline — add genuinely new ones with a follow-up quarry migrate add.

Security Considerations

  • Row-count queries quote all interpolated identifiers via SqlFormatting.QuoteIdentifier; history writes are fully parameterized (positional order preserved for MySQL). Connection strings are never echoed to stdout/stderr. The review's Security pass returned no concerns.

Breaking Changes

  • Internal / behavioral (all migrate add users): canonical name matching is now always on in the core differ, not just in adopt. An add+drop pair whose names are equal under canonicalization (case/separator-only, e.g. user_nameUserName) is always emitted as a RENAME COLUMN/RENAME TABLE and is not offered to the interactive / acceptRename confirmation. This strictly prevents data loss, but a user who genuinely intends to drop a column and add a canonically-equal one (discarding the old data) must split it across two migrations or use --allow-data-loss on a DB-sourced diff. Interactive users no longer see a "Is this a rename?" prompt for convention-only renames.
  • Consumer-facing API: none. New verbs/flags are additive; MigrateAdd/MigrateDiff gained optional parameters with defaults (source-compatible).

DJGosnell added 18 commits July 13, 2026 17:17
Move CreateIntrospectorAsync + BuildConnectionString (and the per-table
introspection loop as ReadTablesAsync) out of ScaffoldCommand into a shared
Schema/DatabaseSchemaReader.cs, and promote TableIntrospectionData to a
top-level type. Refactor ScaffoldCommand to use the helper. Behavior-preserving
groundwork for migrate add --from-database / baseline / adopt.

Add DatabaseSchemaReader.cs to Quarry.Tests compile set and characterization
tests for BuildConnectionString across all four dialects.
DatabaseSchemaReader.ToSnapshot converts live-DB TableIntrospectionData into a
migration SchemaSnapshot for SchemaDiffer: recovers CLR types via
ReverseTypeMapper, infers column Kind from PK/FK column lists, converts FK action
strings to ForeignKeyAction, skips PK-backing indexes, and derives composite keys.

Adds SQLite-backed adapter tests covering identity PK, nullable text columns,
default expressions, FK cascade, PK-index skipping, and composite keys.
Before heuristic Levenshtein scoring, SchemaDiffer now runs a deterministic
pre-pass that matches added/dropped tables and columns whose names are equal
under canonical normalization (lowercase + strip _/-/space). These convention-
only renames (e.g. user_name -> UserName) are emitted as RenameColumn/RenameTable
directly and are never subject to the acceptRename reject callback, so a
snake_case->PascalCase migration can no longer silently degrade to drop+add.

Ambiguous canonical collisions (same canonical form on either side) fall back to
the existing scoring path. Adds NamingConventions.Canonicalize and 6 differ tests
(deterministic under default/reject callbacks, collision fallback, no false
positives on genuinely different names).
RenameMap parses inline (table.col=New,bare=New) and @file (CSV table,from,to or
from,to) specs, with qualified entries taking precedence over bare ones and
case-insensitive name matching. ApplyForcedRenames returns a copy of the 'from'
snapshot with mapped columns renamed (updating FK/index/composite-key references)
plus the list of applied renames, so a caller can emit an explicit RenameColumn
even for pairs the heuristic differ would score below its floor.

Adds RenameMap.cs to the test compile set and 9 tests covering parse variants,
precedence, file/header/comment handling, and the sub-threshold forced-rename diff.
Extract the __quarry_migrations write logic into a reusable
MigrationHistoryWriter: EnsureHistoryTableAsync (CREATE TABLE IF NOT EXISTS,
mirroring the runtime DDL, since the tool never creates the table itself) and
MarkAppliedAsync (status='applied' INSERT, with optional squash_from). Refactor
MigrateSquash to call MarkAppliedAsync instead of its inline INSERT.

This lets 'migrate baseline'/'adopt' record a migration as already-applied without
running its DDL. Quarry->Quarry.Tool/Tests IVT already exists, so the runtime's
FNV-1a ComputeChecksum is reachable for baseline checksums. Adds 4 SQLite tests
(table ensure+idempotency, applied-row insert, checksum parity, squash baseline).
MigrateBaseline generates a migration representing the current schema (or a live
database's schema via --from-database) and records it as already-applied in
__quarry_migrations without executing its DDL — the core of adopting an existing
database. Wires the 'migrate baseline' verb in Program.cs + usage.

Implemented as a MigrateCommands method to reuse its private file-gen/dialect/
connection helpers; uses a 'baseline' checksum sentinel (mirrors squash). Adds an
end-to-end test proving a mark-applied row makes MigrationRunner skip the
migration (no DDL executed). Full suite green (3662).
migrate add and migrate diff accept --from-database <connstr> (+ -d/--dialect):
when set, the 'from'/comparison snapshot is introspected from the live database
(via DatabaseSchemaReader) instead of the last persisted project snapshot, so the
diff runs against database reality. Shared ResolveFromSnapshotAsync helper; the
default (no --from-database) path is unchanged. Program.cs wired for both verbs.

Adds a data-path test (introspect real SQLite -> adapter -> differ) proving a
legacy snake_case DB diffed against a PascalCase schema emits RenameColumn, not
drop+add.
DropGuard.FindViolationsAsync counts rows for each DropColumn/DropTable step
against the live database (DropColumn uses WHERE col IS NOT NULL so all-null
columns don't block). migrate add --from-database refuses to emit a migration
that would drop a populated column/table unless --allow-data-loss is passed -
the safety net for a rename the differ missed and degraded to drop+add.

Wired into migrate add (and reused by adopt); not diff, which is preview-only.
Adds 5 DropGuard tests.
migrate adopt orchestrates the full adoption of an existing database: introspect
the live DB, record its state as an applied baseline (InitialCreate, no DDL run),
then generate a pending alignment migration transforming the DB to the project
schemas — canonical + --rename-map renames, guarded against data loss.

Critical fix: ProjectSchemaReader populates only name/clrType/nullable/kind on
ColumnDef (identity/length/default left default; identity inferred from PK kind),
while the DB adapter fills them richly — so diffing the two raw snapshots emitted
a spurious AlterColumn per column. DatabaseSchemaReader.NormalizeForDiff projects
both sides onto the shared comparable subset (and clears FK/index/naming noise);
the --from-database add/diff paths and adopt now diff normalized snapshots while
generating files from the rich originals.

Wires 'migrate adopt' in Program.cs + usage. Adds NormalizeForDiff tests (raw
diff over-reports, normalized diff yields only renames, real type change still
detected) and an end-to-end adopt scenario (baseline skipped, rename applied,
data preserved). Full suite green (3671).
…ames (step 9)

Update llm.md (migrations section), llm-migrate.md (Phase 6: adopt workflow
replaces the manual scaffold-then-rename dance), and Quarry.Tool/README.md
(migrate baseline/adopt command references, --from-database/--allow-data-loss
flags on migrate add, and a Rename Detection section documenting the always-on
deterministic canonical matching).
REMEDIATE pass over the adopt bundle review (5A/6B fixed, 4D dismissed):

- F5: DropGuard is now schema-aware. A normalized diff strips the schema
  qualifier, so BuildTableSchemaMap/ResolveSchema re-qualify each drop from
  the live snapshot. Verified on a real PostgreSQL non-default schema.
- F6/F7: RenameMap.Validate rejects invalid maps (target absent from project,
  duplicate/colliding target) and warns on unmatched entries. MigrateAdopt
  parses + validates the rename-map BEFORE writing the baseline, so an invalid
  map never leaves a half-adopted DB.
- F8: canonical table renames now carry the schema transfer (oldSchemaName)
  when the table also moves schema.
- F2: adopt warns when the project already has migrations (non-fresh adopt).
- F12: removed the dead AddParameter helper from MigrateCommands.
- F1/F13: docs steer to adopt/--from-database and flag the always-on canonical
  rename as a behavior change for all `migrate add` users.

Tests (+18): PostgreSQL multi-schema guard (AdoptGuardPostgresTests), adopt
abort-on-drop composition (AdoptGuardScenarioTests), DropGuard schema
qualification, RenameMap.Validate, canonical table-rename schema transfer, and
history-table DDL parity. Full suite green (3714).
Step 9 updated only llm-migrate.md and the tool README; the docs/ articles
were missing the new commands. Add to docs/articles/migrations.md a full
'Adopting an Existing Database' section (adopt, baseline, --from-database,
--rename-map, the data-loss guard, and the always-on convention-aware rename
behavior + breaking-change note), plus the new flags on migrate add/diff and an
updated rename-detection description. Point docs/articles/migrating-to-quarry.md
at 'migrate adopt' for adopting a live, populated database.
@DJGosnell

Copy link
Copy Markdown
Member Author

Closing without merging — the rename-based approach here is the wrong model for the problem.

Why: This PR adopts an existing database by generating a migration that renames the physical columns/tables to match the C# property names. But Quarry is a map, don't rename ORM: the runtime already resolves C# properties to physical DB names via the NamingStyle override and per-column MapTo(...). So the correct, low-risk way to work with a legacy database whose columns don't match your schema names is to map them (scaffold with a naming style / MapTo) and baselinezero DDL, no risk to other consumers. Physically renaming a live database is the more dangerous choice (MySQL folds identifier case depending on lower_case_table_names; PostgreSQL quoted "UserName" ≠ unquoted username; and renaming a shared schema silently breaks views/reports/other apps).

Root cause this surfaced: migrate adopt only needed to rename because the migration tool doesn't currently honor NamingStyle or MapToProjectSchemaReader reads a property named Naming (the real API is NamingStyle) and ignores MapTo entirely, so its snapshots use PascalCase property names while the runtime uses the physical names. That divergence (and the fact that adding/removing a mapping produces no migration at all) is a pre-existing correctness bug, filed as #324.

Follow-up: Fix #324 so NamingStyle/MapTo are honored in the migration path. Once mapping works end-to-end, adopting an existing database is just "scaffold with mapping + baseline" — a dedicated rename-based migrate adopt command is unnecessary, so this branch is being dropped rather than reworked.

The full investigation (agent findings, the Roslyn probe results, and the end-to-end AccountSchema/credit_limit divergence) is captured in #324.

@DJGosnell DJGosnell closed this Jul 14, 2026
@DJGosnell
DJGosnell deleted the migrate-adopt-existing-db branch July 14, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant