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

[Tragetaschen]

File a bug

I have a main project referencing a shared project that contains the EF Core entities, context and migrations.

Without any changes to the entities, I add a new migration using the CLI dotnet ef migrations add Empty -p ../shared. Expectedly, this creates a migration script and a corresponding .Designer.cs, where both Up and Down methods are empty and the snapshot does not change.

I now remove that migration by running dotnet ef migrations remove -p ../shared and I expect to be in the same state where I started and both migration files are removed as expected. However, the snapshot now has open changes, because every b.ToTable("…") call has changed:

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

When I add another new (empty) migration, the additional null parameters are removed again and the snapshot has no outstanding changes.

Include verbose output

dotnet ef with --verbose and more details

PS …\main> dotnet ef migrations add Empty -p ..\shared\ --verbose
Using project '..\shared\shared.csproj'.
Using startup project '…\main\main.csproj'.
Writing '..\shared\obj\shared.csproj.EntityFrameworkCore.targets'...
dotnet msbuild /target:GetEFProjectMetadata /property:EFProjectMetadataFile=…\AppData\Local\Temp\tmp3717.tmp /verbosity:quiet /nologo ..\shared\shared.csproj
Writing '…\main\obj\main.csproj.EntityFrameworkCore.targets'...
dotnet msbuild /target:GetEFProjectMetadata /property:EFProjectMetadataFile=…\AppData\Local\Temp\tmp392C.tmp /verbosity:quiet /nologo …\main\main.csproj
Build started...
dotnet build …\main\main.csproj /verbosity:quiet /nologo

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:04.55
Build succeeded.
dotnet exec --depsfile …\main\bin\Debug\net6.0\main.deps.json --additionalprobingpath …\.nuget\packages --additionalprobingpath "C:\Program Files (x86)\Microsoft Visual Studio\Shared\NuGetPackages" --additionalprobingpath "C:\Program Files (x86)\Microsoft\Xamarin\NuGet" --runtimeconfig …\main\bin\Debug\net6.0\main.runtimeconfig.json …\.dotnet\tools\.store\dotnet-ef\6.0.0-rc.1.21452.10\dotnet-ef\6.0.0-rc.1.21452.10\tools\netcoreapp3.1\any\tools\netcoreapp2.0\any\ef.dll migrations add Empty --assembly …\main\bin\Debug\net6.0\shared.dll --project ..\shared\shared.csproj --startup-assembly …\main\bin\Debug\net6.0\main.dll --startup-project …\main\main.csproj --project-dir …\shared\ --root-namespace shared --language C# --framework net6.0 --nullable --working-dir …\main --verbose
Using assembly 'shared'.
Using startup assembly 'main'.
Using application base '…\main\bin\Debug\net6.0'.
Using working directory '…\main'.
Using root namespace 'shared'.
Using project directory '…\shared\'.
Remaining arguments: .
Finding DbContext classes...
Finding IDesignTimeDbContextFactory implementations...
Finding application service provider in assembly 'main'...
Finding Microsoft.Extensions.Hosting service provider...
Using environment 'Development'.
Using application service provider from Microsoft.Extensions.Hosting.
Found DbContext 'StateDbContext'.
Finding DbContext classes in the project...
Using context 'StateDbContext'.
info: Microsoft.EntityFrameworkCore.Infrastructure[10403]
      Entity Framework Core 6.0.0-rc.1.21452.10 initialized 'StateDbContext' using provider 'Microsoft.EntityFrameworkCore.SqlServer:6.0.0-rc.1.21452.10' with options: None
Finding design-time services referenced by assembly 'main'...
Finding design-time services referenced by assembly 'shared'...
No referenced design-time services were found.
Finding design-time services for provider 'Microsoft.EntityFrameworkCore.SqlServer'...
Using design-time services from provider 'Microsoft.EntityFrameworkCore.SqlServer'.
Finding IDesignTimeServices implementations in assembly 'main'...
No design-time services were found.
Writing migration to '…\shared\Migrations\20210916120127_Empty.cs'.
Writing model snapshot to '…\shared\Migrations\StateDbContextModelSnapshot.cs'.
Done. To undo this action, use 'ef migrations remove'
PS …\main> git status
On branch feature/fix-migrations
Untracked files:
  (use "git add <file>..." to include in what will be committed)
        ../shared/Migrations/20210916120127_Empty.Designer.cs
        ../shared/Migrations/20210916120127_Empty.cs

nothing added to commit but untracked files present (use "git add" to track)
PS …\main> dotnet ef migrations remove -p ..\shared\ --verbose
Using project '..\shared\shared.csproj'.
Using startup project '…\main\main.csproj'.
Writing '..\shared\obj\shared.csproj.EntityFrameworkCore.targets'...
dotnet msbuild /target:GetEFProjectMetadata /property:EFProjectMetadataFile=…\AppData\Local\Temp\tmpBCB2.tmp /verbosity:quiet /nologo ..\shared\shared.csproj
Writing '…\main\obj\main.csproj.EntityFrameworkCore.targets'...
dotnet msbuild /target:GetEFProjectMetadata /property:EFProjectMetadataFile=…\AppData\Local\Temp\tmpBEC7.tmp /verbosity:quiet /nologo …\main\main.csproj
Build started...
dotnet build …\main\main.csproj /verbosity:quiet /nologo

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:02.31
Build succeeded.
dotnet exec --depsfile …\main\bin\Debug\net6.0\main.deps.json --additionalprobingpath …\.nuget\packages --additionalprobingpath "C:\Program Files (x86)\Microsoft Visual Studio\Shared\NuGetPackages" --additionalprobingpath "C:\Program Files (x86)\Microsoft\Xamarin\NuGet" --runtimeconfig …\main\bin\Debug\net6.0\main.runtimeconfig.json …\.dotnet\tools\.store\dotnet-ef\6.0.0-rc.1.21452.10\dotnet-ef\6.0.0-rc.1.21452.10\tools\netcoreapp3.1\any\tools\netcoreapp2.0\any\ef.dll migrations remove --assembly …\main\bin\Debug\net6.0\shared.dll --project ..\shared\shared.csproj --startup-assembly …\main\bin\Debug\net6.0\main.dll --startup-project …\main\main.csproj --project-dir …\shared\ --root-namespace shared --language C# --framework net6.0 --nullable --working-dir …\main --verbose
Using assembly 'shared'.
Using startup assembly 'main'.
Using application base '…\main\bin\Debug\net6.0'.
Using working directory '…\main'.
Using root namespace 'shared'.
Using project directory '…\shared\'.
Remaining arguments: .
Finding DbContext classes...
Finding IDesignTimeDbContextFactory implementations...
Finding application service provider in assembly 'main'...
Finding Microsoft.Extensions.Hosting service provider...
Using environment 'Development'.
Using application service provider from Microsoft.Extensions.Hosting.
Found DbContext 'StateDbContext'.
Finding DbContext classes in the project...
Using context 'StateDbContext'.
info: Microsoft.EntityFrameworkCore.Infrastructure[10403]
      Entity Framework Core 6.0.0-rc.1.21452.10 initialized 'StateDbContext' using provider 'Microsoft.EntityFrameworkCore.SqlServer:6.0.0-rc.1.21452.10' with options: None
Finding design-time services referenced by assembly 'main'...
Finding design-time services referenced by assembly 'shared'...
No referenced design-time services were found.
Finding design-time services for provider 'Microsoft.EntityFrameworkCore.SqlServer'...
Using design-time services from provider 'Microsoft.EntityFrameworkCore.SqlServer'.
Finding IDesignTimeServices implementations in assembly 'main'...
No design-time services were found.
Removing migration '20210916120127_Empty'.
Reverting the model snapshot.
Done.
PS …\main> git status
On branch feature/fix-migrations
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   ../shared/Migrations/StateDbContextModelSnapshot.cs

no changes added to commit (use "git add" and/or "git commit -a")
PS …\main> git diff
diff --git a/shared/Migrations/StateDbContextModelSnapshot.cs b/shared/Migrations/StateDbContextModelSnapshot.cs
index 8c1403c..368ca98 100644
--- a/shared/Migrations/StateDbContextModelSnapshot.cs
+++ b/shared/Migrations/StateDbContextModelSnapshot.cs
@@ -,7 +,7 @@ namespace shared.Migrations

                     b.HasIndex("MyEntityGuid");

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

Include provider and version information

EF Core version: 6.0.0-rc.1.21452.10 (both library and global tool)
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: net6.0 RC1
Operating system: Windows 10
IDE: (not used) VS 2022 Preview 4

[ajcvickers]

@Tragetaschen Are you seeing any functional impact from this?

[Tragetaschen]

I do not see any functional difference besides the uncommited changes in Git

[ajcvickers]

@Tragetaschen We discussed this and while ideally there wouldn't be a change in this case, but we're going to close this since it's not true that in general going up then down will always result in the same snapshot, and so any process that does this must account for potential differences.

[jasekiw]

This is really frustrating considering that it is really easy to corrupt the model snapshot. I commonly review diffs to ensure that it is not corrupted but this is muddying the waters. I haven't ran into any other cases that changes my snapshot other than this one. What other cases are there @ajcvickers?

[jasekiw]

Another note:

It appears this stems from using the attribute [Table("MyTableName)]. If .ToTable is used below in OnModelCreating then the migrations always has the (string)null and yields no differences when removed.

  modelBuilder.Entity<MyEntity>(e =>
        {
            e.ToTable("MyEntity");

[derrexal]

Все еще актуально...

[jasekiw]

@ajcvickers Closing this issue is in direct conflict from the recommended practice of merging the snapshot file posted by the ef core team https://learn.microsoft.com/en-us/ef/core/managing-schemas/migrations/teams#merging

Yes a revert may not always match but I have never encountered that. This issue causes a change on every single table in the snapshot table leaving hundreds of changes making it impossible to deal with a merge due to the noise. I believe this needs to be reconsidered to be re-opened.

[pcoffer]

This needs to be reopened. It makes it very difficult to see real changes when diffing a merge request. One small real change is buried in a haystack of hundreds of fake changes in a real project.

[icottiero]

Please reopen this and fix it! My team had a lot of issues because of the noise generated by these unnecessary changes which caused us to miss some important changes that have broken the snapshot.
This isn't a functional bug, because the snapshot does the same thing with or without the (string) null, but non-functional requirements like the ease of use can also be a good motivation for fixing something 🙏

[jasekiw]

@ajcvickers I understood last comment to mean: ideally this wouldn't happen but it's not worth our time. However, would you be open to PR's?

We discussed this and while ideally there wouldn't be a change in this case, but we're going to close this since it's not true that in general going up then down will always result in the same snapshot, and so any process that does this must account for potential differences.

I was able to fix this issue with a small adjustment to the migration code that you can see in this commit: 353612c

I figured out the schemaAnnotion variable is not null during revert but has a null schemaAnnotion.Value causing it to add (string)null. It seems that during "add migration" this doesn't happen. Since .ToTable has an overload for (tableName,schemaName,tableBuilder) and (tableName, tableBuilder) I decided that we can just not include the ,(string)null as it should not cause any issues with the overloads.

Would you be open to me creating a pr with this change? Are there any tests you think I would need to add to it?