Skip to content

Migration tool ignores NamingStyle and MapTo — snapshots/DDL diverge from runtime; mapping changes silently produce no migration #324

Description

@DJGosnell

Description

The migration tooling (quarry migrate add / diff, via ProjectSchemaReader) does not honor two column-naming mechanisms that the runtime source generator does honor:

  • the NamingStyle override (e.g. NamingStyle.SnakeCase), and
  • per-column MapTo("physical_name").

As a result, migration snapshots and generated DDL use the C# property names (UserName, CreditLimit) while the runtime generates SQL using the physical names (user_name, credit_limit). A generated migration creates columns the runtime never queries (and vice-versa). Worse, adding or removing a mapping between two schema versions produces zero diff steps — the change is silently ignored, so the physical column the runtime targets is never created or renamed by any migration.

This matters because mapping (via NamingStyle / MapTo) is the intended, low-risk way to use an existing database whose columns don't match your C# property names — you map them rather than physically renaming the database. That path is currently broken in the migration tool.

Location

  • src/Quarry.Tool/Schema/ProjectSchemaReader.cs
    • ExtractTableDef reads the naming style from a property literally named "Naming" (~line 113), but the real API property — base class Schema.NamingStyle (src/Quarry/Schema/Schema.cs:35), every sample, and scaffold output — is NamingStyle. The override is therefore never detected and namingStyle stays NamingStyleKind.Exact.
    • ExtractColumnDef (~lines 231–319) recognizes only Computed / Collation in the fluent chain; there is no MapTo case and it never passes mappedName, so ColumnDef.MappedName is always null and ColumnDef.Name is the un-styled property name.
  • src/Quarry.Tool/Schema/DatabaseSchemaReader.csNormalizeForDiff (~line 218) rebuilds each column as new ColumnDef(c.Name, c.ClrType, c.IsNullable, c.Kind), dropping MappedName; the diff keys off Name only.
  • Contrast — runtime path (correct): src/Quarry.Generator/Parsing/SchemaParser.cs:252 reads GetMembers("NamingStyle"), and ~line 405 resolves columnName = modifiers.MappedName ?? NamingConventions.ToColumnName(prop.Name, namingStyle).
  • Masking tests: src/Quarry.Tests/Migration/ProjectSchemaReaderIndexTests.cs (the two naming tests, ~lines 82–99 and 368–386) declare a fictional public NamingStyle Naming => … property that matches the buggy "Naming" check, so they pass while the real API (NamingStyle) is never exercised.

Diagnostics

Observed by running ProjectSchemaReader.ExtractSchemaSnapshot against real-API schemas (temporary Roslyn probe):

  • Schema with protected override NamingStyle NamingStyle => NamingStyle.SnakeCase; + Col<string> UserName
    → extracted TableDef.NamingStyle = Exact, ColumnDef.Name = "UserName" (expected SnakeCase / user_name).
  • Schema with public Col<string> UserName => MapTo<string>("user_name");
    ColumnDef.Name = "UserName", MappedName = null (expected physical user_name).
  • Diff of v1 (with MapTo) → v2 (without MapTo)
    0 steps (expected a RenameColumn, or drop+add).

End-to-end divergence on a real committed schema:

  • src/Quarry.Tests/Samples/AccountSchema.cs:16: public Col<Money> CreditLimit => Mapped<Money, MoneyMapping>().MapTo("credit_limit");
  • Runtime (existing passing test src/Quarry.Tests/SqlOutput/CrossDialectSchemaTests.cs → Select_MapToColumn_CreditLimit, ~lines 45–63): emits SELECT "AccountId", "credit_limit" FROM "accounts".
  • Migration tool (probe): would emit CREATE TABLE accounts (… CreditLimit …) — a column named CreditLimit, not credit_limit.
  • Net: apply that migration, then run the app → every query does SELECT credit_limit against a table that only has CreditLimit → runtime failure.

What Has Been Tried

  • Wrote a temporary NUnit probe using the same Roslyn compilation harness as ProjectSchemaReaderIndexTests to extract TableDef/ColumnDef for real-API schemas and to diff a mapping removal; captured the outputs above; then deleted the probe (no committed changes).
  • Traced both pipelines (runtime SchemaParser vs. tool ProjectSchemaReader) to confirm the divergence is on the tool side only.
  • Confirmed committed sample migrations are unaffected by a fix: 1_SimpleMigration, Quarry.Sample.Aot, Quarry.Sample.WebApp use Exact naming (their PascalCase columns are correct). The snake_case samples (3_Scaffolding) and the MapTo sample (AccountSchema) have no committed migrations, so NamingStyle/MapTo + migrate add is currently untested territory — churn risk of fixing is low.

Gathered Information

  • NamingConventions.ToColumnName / Canonicalize (src/Quarry.Shared/Migration/NamingConventions.cs) already implement the property→physical transform the runtime uses.
  • SnapshotCodeGenerator already emits .MapTo("…") from ColumnDef.MappedName (asserted in src/Quarry.Tests/Migration/SnapshotCodeGeneratorTests.cs ~line 225), so the snapshot-code round-trip can represent a mapped name — the gap is purely on the ProjectSchemaReader read side.
  • Only the two ProjectSchemaReaderIndexTests naming tests would need updating (fictional Naming → real NamingStyle). SchemaTests uses the real override but tests the runtime Schema property, so it is unaffected.

Suggested Approach

  1. In ProjectSchemaReader.ExtractTableDef, detect the naming style from the real NamingStyle property (prop.Name == "NamingStyle"), keeping Exact as the default when not overridden.
  2. In ProjectSchemaReader.ExtractColumnDef, parse MapTo("…") (and Mapped<…>().MapTo("…")) from the column's expression body — mirroring the runtime SchemaParser case "MapTo" — and resolve the physical name as MapTo-arg ?? ToColumnName(prop.Name, namingStyle).
  3. Choose a representation consistent with the snapshot round-trip: either set ColumnDef.Name to the physical name (simplest — migration DDL then matches the runtime) or populate MappedName and thread it through SchemaDiffer / DdlRenderer / NormalizeForDiff. If Name becomes the physical name, still populate MappedName so SnapshotCodeGenerator re-emits .MapTo(...).
  4. Update the two ProjectSchemaReaderIndexTests naming tests to the real NamingStyle override, and add coverage for: NamingStyle honored, MapTo honored, and a mapping add/remove producing the correct rename (not a silent no-op). Add an end-to-end guard that a MapTo schema's migration DDL column name matches the runtime SQL column name (using AccountSchema / credit_limit).
  5. Changelog note: a project that already ran migrate add on a NamingStyle/MapTo schema has PascalCase snapshots; after the fix its next migrate add will show a physical-name diff. Those projects were already broken (runtime vs. migration mismatch), so this is a correction.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions