Skip to content

Fix redundant (string)null schema argument in ToTable snapshot generation#38424

Open
Copilot wants to merge 4 commits into
mainfrom
copilot/removing-a-migration-bug-fix
Open

Fix redundant (string)null schema argument in ToTable snapshot generation#38424
Copilot wants to merge 4 commits into
mainfrom
copilot/removing-a-migration-bug-fix

Conversation

Copilot AI commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Fixes #26067

Removing a migration rewrites the model snapshot, appending a redundant (string)null schema argument to every ToTable call, leaving the snapshot with spurious uncommitted changes.

-    b.ToTable("MyEntity");
+    b.ToTable("MyEntity", (string)null);

Root cause

ToTable("X") always records a null Schema annotation. When the snapshot is round-tripped — which is what migrations remove does, rebuilding the model from the snapshot — CSharpSnapshotGenerator.GenerateTableMapping re-emitted that null as , (string)null, even when the entity's default schema was already null (making the argument redundant). The migrations add path uses the live model, where a table-name-by-convention carries no schema annotation, which is why only remove reproduced it.

Changes

  • CSharpSnapshotGenerator.GenerateTableMapping: emit the explicit null schema only when entityType.GetDefaultSchema() != null, so the argument is kept only when meaningful (overriding an inherited non-null schema, e.g. an owned entity under a schema-qualified owner) and dropped when redundant. Snapshot generation becomes idempotent.
  • Tests:
    • ToTable_with_default_null_schema_is_not_changed_on_snapshot_round_trip (CSharpMigrationsGeneratorTest) — asserts the snapshot is stable across generate → rebuild → regenerate.
    • Convert_owned_entity_with_no_schema_to_regular_entity (MigrationsTestBase) — covers the owned-entity-under-schema case where (string)null is required; asserter is order-independent and guards schema checks behind AssertSchemaNames (SQLite has no schema support and orders tables differently). SQLite and SQL Server overrides included.

Copilot AI and others added 2 commits June 13, 2026 00:28
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix migration removal causing changes in snapshot Fix redundant (string)null schema argument in ToTable snapshot generation Jun 13, 2026
Copilot AI requested a review from AndriySvyryd June 13, 2026 00:42
@AndriySvyryd AndriySvyryd requested a review from Copilot June 13, 2026 01:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes snapshot code generation so that migrations remove (which round-trips through the model snapshot) no longer introduces redundant , (string)null schema arguments on ToTable calls when the effective default schema is already null, improving snapshot idempotence.

Changes:

  • Update CSharpSnapshotGenerator.GenerateTableMapping to only emit an explicit null schema argument when the entity type’s default schema is non-null (i.e., when null is an intentional override).
  • Add a design-time snapshot round-trip test to ensure ToTable("X") stays stable and doesn’t become ToTable("X", (string)null) after round-tripping.
  • Add/extend migrations functional coverage for converting an owned type (under a schema-qualified owner) into a regular entity while preserving the intended schema behavior across providers.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs Avoids emitting redundant explicit-null schema arguments during snapshot generation when the default schema is already null.
test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs Adds a regression test ensuring snapshots remain identical after generate → rebuild → regenerate when default schema is null.
test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs Adds a provider-agnostic migration scenario covering owned-to-regular conversion with schema interplay and snapshot round-tripping.
test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsSqliteTest.cs Adds provider-specific SQL assertions for the new owned-to-regular conversion scenario on SQLite.
test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs Adds an override for the new scenario, but currently lacks SQL assertions (see comment).

Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
@AndriySvyryd AndriySvyryd marked this pull request as ready for review June 16, 2026 00:58
@AndriySvyryd AndriySvyryd requested a review from a team as a code owner June 16, 2026 00:58
Copilot AI review requested due to automatic review settings June 16, 2026 00:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@AndriySvyryd AndriySvyryd requested a review from cincuranet June 16, 2026 01:24
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.

Removing a migration adds a (string)null argument to all ToTable calls in the snapshot

3 participants