Skip to content

Reexamine migration table detection/creation #3407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
roji opened this issue Dec 15, 2024 · 4 comments
Open

Reexamine migration table detection/creation #3407

roji opened this issue Dec 15, 2024 · 4 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@roji
Copy link
Member

roji commented Dec 15, 2024

In order to fix #2878 for 9.0, #3294 changed our migration history table creation strategy - we try to select from it without pre-checking whether it exists, and catch "table doesn't exist" exceptions, at which point we create it.

This unfortunately doesn't work if the migration process is wrapped in a user transaction - the error causes the transaction to go into a failed state, and everything breaks down. In addition, this (currently) causes an error to appear in the logs, confusing users and making them think the migration failed. Reexamine our strategy here.

The relevant tasks are MigrationsInfrastructureNpgsqlTest.{Can_apply_two_migrations_in_transaction,Can_apply_two_migrations_in_transaction_async}.

Note: the fix may need to be backported.

@roji roji added the bug Something isn't working label Dec 15, 2024
@roji roji added this to the 10.0.0 milestone Dec 15, 2024
@roji roji self-assigned this Dec 15, 2024
@roji
Copy link
Member Author

roji commented Jan 21, 2025

Also take a look at #3436, #3496 when doing this.

@pantonis
Copy link

pantonis commented Feb 8, 2025

I ge this when I use Update-Database command.

@rudism
Copy link

rudism commented Feb 27, 2025

Is there any workaround for this? My migration process includes executing some pre- and post-migration scripts that tear down and recreate various things in the database to support a legacy system which need to run in the same transaction as the migrations (so that everything gets rolled back in case of an error). I'm unable to upgrade from Npgsql 8 -> 9 currently as the SuppressTransaction = true when creating the history repository prevent the ability to run migrations in a transaction.

@rudism
Copy link

rudism commented Feb 27, 2025

For now I'm working around this with my own modified implementation of the IHistoryRepository interface that inherits from NpgsqlHistoryRepository but overrides the CreateIfNotExists and CreateIfNotExistsAsync methods to omit the SuppressTransaction thing:

Code
using Microsoft.EntityFrameworkCore.Migrations;
using Microsoft.EntityFrameworkCore.Migrations.Operations;
using Npgsql;
using Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.Internal;

namespace Cosmic.Data.Mapping;

#pragma warning disable EF1001
public class HackyHistoryRepository(HistoryRepositoryDependencies dependencies)
: NpgsqlHistoryRepository(dependencies), IHistoryRepository {

  bool IHistoryRepository.CreateIfNotExists() {
    try {
      return Dependencies.MigrationCommandExecutor.ExecuteNonQuery(
        GetCreateIfNotExistsCommands(),
        Dependencies.Connection,
        new MigrationExecutionState(),
        commitTransaction: true) != 0;
    } catch (PostgresException e) when (
        e.SqlState is PostgresErrorCodes.UniqueViolation
          or PostgresErrorCodes.DuplicateTable
          or PostgresErrorCodes.DuplicateObject) {
      return false;
    }
  }

  async Task<bool> IHistoryRepository.CreateIfNotExistsAsync(
      CancellationToken cancellationToken) {
    try {
      return (await Dependencies.MigrationCommandExecutor.ExecuteNonQueryAsync(
        GetCreateIfNotExistsCommands(),
        Dependencies.Connection,
        new MigrationExecutionState(),
        commitTransaction: true,
        cancellationToken: cancellationToken).ConfigureAwait(false)) != 0;
    } catch (PostgresException e) when (
        e.SqlState is PostgresErrorCodes.UniqueViolation
          or PostgresErrorCodes.DuplicateTable
          or PostgresErrorCodes.DuplicateObject) {
      return false;
    }
  }

  private IReadOnlyList<MigrationCommand> GetCreateIfNotExistsCommands() =>
    Dependencies.MigrationsSqlGenerator.Generate([new SqlOperation {
      Sql = GetCreateIfNotExistsScript(),
    }]);
}
#pragma warning restore EF1001

Then I inject it when configuring the database connection:

services.AddDbContext<MyDbContext>(static opts => opts
  .UseNpgsql(connectionString, static builder => { ... })
  .ReplaceService<IHistoryRepository, HackyHistoryRepository>();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants