dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.8k stars 3.2k forks source link

efcore9: Migration with .Sql that suppresses a transaction logs an error even though it works #35133

Open ChristophHornung opened 3 days ago

ChristophHornung commented 3 days ago

When switching to efcore9 existing migrations that include a .Sql(..., suppressTransaction: true) will generate an error in the log even though they work. The what is new documentation states this should only issues a warning.

Include your code

I uploaded a minimal reproduction here.

Include verbose output

The log outputs the fail message but also shows that the SQL is executed correctly.

info: Microsoft.EntityFrameworkCore.Migrations[20402]
      Applying migration '20241118110903_NonTransactable'.
fail: Microsoft.EntityFrameworkCore.Migrations[20410]
      The migration operation 'UPDATE Posts SET NewPostId = PostId+1
      ' from migration 'NonTransactable' cannot be executed in a transaction. If the app is terminated or an unrecoverable error occurs while this operation is being executed then the migration will be left in a partially applied state and would need to be reverted manually before it can be applied again. Create a separate migration that contains just this operation.
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (0ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      ALTER TABLE "Posts" ADD "NewPostId" INTEGER NOT NULL DEFAULT 0;
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (0ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      UPDATE Posts SET NewPostId = PostId+1
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (2ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      INSERT INTO "__EFMigrationsHistory" ("MigrationId", "ProductVersion")
      VALUES ('20241118110903_NonTransactable', '9.0.0');

Include provider and version information

EF Core version: no error log on 8, but error logged on 9 Database provider: tested with SQL and SQLite Target framework: .NET8 and .NET9

cincuranet commented 3 days ago

Similar to #35096.

AndriySvyryd commented 3 days ago

This is by design. As the message says you should create a separate migration that contains just the operation that suppressed transactions.

ChristophHornung commented 3 days ago

This is by design. As the message says you should create a separate migration that contains just the operation that suppressed transactions.

@AndriySvyryd Those are all existing migrations with multiple databases in the field that already ran at least parts of the migration chain, so nothing that I can easily change. Now every time we pull up new databases our dashboard lights up with lots of error level logs.

Why would an operation that does not fail not log a warning instead of an error, the what-is-new documentation explicitly mentions that only a warning will be given? And what is the proposed solution to remedy that in old migrations?

AndriySvyryd commented 2 days ago

Why would an operation that does not fail not log a warning instead of an error, the what-is-new documentation explicitly mentions that only a warning will be given? And what is the proposed solution to remedy that in old migrations?

Perhaps you have the default behavior for warnings set to Throw. Try adding this using the DbContext options builder.

.ConfigureWarnings(e => e.Log(RelationalEventId.NonTransactionalMigrationOperationWarning));
ChristophHornung commented 2 days ago

@AndriySvyryd Maybe there is some confusion here. There is no exception being thrown, as you can see in the minimal reproduction. Only a an error is logged.

The issue I have is with that log being of the log-level "Error" instead of the (imho) more appropriate level "Warning" (since everything works just fine and the event id is even called 'NonTransactionalMigrationOperationWarning').

ChristophHornung commented 2 days ago

To be more precise, this is about the log level defined here:

https://github.com/dotnet/efcore/blob/cda78166c99b55b61cf0e24ce76c3f9980a79f63/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs#L3629

AndriySvyryd commented 2 days ago

I see. Yes, we can change that.