Sync to EF Core 9.0.0-rc.2.24460.3 (npgsql#3275)

Closes npgsql#3223

Directory.Packages.props

@@ -1,7 +1,7 @@
   <Project>
   <PropertyGroup>
-    <EFCoreVersion>[9.0.0-rc.1.24451.1]</EFCoreVersion>
-    <MicrosoftExtensionsVersion>9.0.0-rc.1.24431.7</MicrosoftExtensionsVersion>
+    <EFCoreVersion>[9.0.0-rc.2.24460.3]</EFCoreVersion>
+    <MicrosoftExtensionsVersion>9.0.0-rc.2.24456.9</MicrosoftExtensionsVersion>
     <NpgsqlVersion>8.0.4</NpgsqlVersion>
   </PropertyGroup>
 

src/EFCore.PG/Migrations/Internal/NpgsqlHistoryRepository.cs

@@ -6,7 +6,7 @@
 ///     any release. You should only use it directly in your code with extreme caution and knowing that
 ///     doing so can result in application failures when updating to a new Entity Framework Core release.
 /// </summary>
-public class NpgsqlHistoryRepository : HistoryRepository
+public class NpgsqlHistoryRepository : HistoryRepository, IHistoryRepository
 {
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -19,60 +19,29 @@ public NpgsqlHistoryRepository(HistoryRepositoryDependencies dependencies)
     {
     }
 
-    // TODO: We override Exists() as a workaround for https://github.com/dotnet/efcore/issues/34569; this should be fixed on the EF side
-    // before EF 9.0 is released
-
-    /// <summary>
-    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
-    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
-    ///     any release. You should only use it directly in your code with extreme caution and knowing that
-    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
-    /// </summary>
-    public override bool Exists()
-        => Dependencies.DatabaseCreator.Exists()
-            && InterpretExistsResult(
-                Dependencies.RawSqlCommandBuilder.Build(ExistsSql).ExecuteScalar(
-                    new RelationalCommandParameterObject(
-                        Dependencies.Connection,
-                        null,
-                        null,
-                        Dependencies.CurrentContext.Context,
-                        Dependencies.CommandLogger, CommandSource.Migrations)));
-
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public override async Task<bool> ExistsAsync(CancellationToken cancellationToken = default)
-        => await Dependencies.DatabaseCreator.ExistsAsync(cancellationToken).ConfigureAwait(false)
-            && InterpretExistsResult(
-                await Dependencies.RawSqlCommandBuilder.Build(ExistsSql).ExecuteScalarAsync(
-                    new RelationalCommandParameterObject(
-                        Dependencies.Connection,
-                        null,
-                        null,
-                        Dependencies.CurrentContext.Context,
-                        Dependencies.CommandLogger, CommandSource.Migrations),
-                    cancellationToken).ConfigureAwait(false));
+    public override LockReleaseBehavior LockReleaseBehavior => LockReleaseBehavior.Transaction;
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public override IDisposable GetDatabaseLock()
+    public override IMigrationsDatabaseLock AcquireDatabaseLock()
     {
-        // TODO: There are issues with the current lock implementation in EF - most importantly, the lock isn't acquired within a
-        // transaction so we can't use e.g. LOCK TABLE. This should be fixed for rc.1, see #34439.
+        Dependencies.MigrationsLogger.AcquiringMigrationLock();
 
-        // Dependencies.RawSqlCommandBuilder
-        //     .Build($"LOCK TABLE {Dependencies.SqlGenerationHelper.DelimitIdentifier(TableName, TableSchema)} IN ACCESS EXCLUSIVE MODE")
-        //     .ExecuteNonQuery(CreateRelationalCommandParameters());
+        Dependencies.RawSqlCommandBuilder
+            .Build($"LOCK TABLE {Dependencies.SqlGenerationHelper.DelimitIdentifier(TableName, TableSchema)} IN ACCESS EXCLUSIVE MODE")
+            .ExecuteNonQuery(CreateRelationalCommandParameters());
 
-        return new DummyDisposable();
+        return new NpgsqlMigrationDatabaseLock(this);
     }
 
     /// <summary>
@@ -81,19 +50,24 @@ public override IDisposable GetDatabaseLock()
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    public override Task<IAsyncDisposable> GetDatabaseLockAsync(CancellationToken cancellationToken = default)
+    public override async Task<IMigrationsDatabaseLock> AcquireDatabaseLockAsync(CancellationToken cancellationToken = default)
     {
-        // TODO: There are issues with the current lock implementation in EF - most importantly, the lock isn't acquired within a
-        // transaction so we can't use e.g. LOCK TABLE. This should be fixed for rc.1, see #34439.
-
-        // await Dependencies.RawSqlCommandBuilder
-        //     .Build($"LOCK TABLE {Dependencies.SqlGenerationHelper.DelimitIdentifier(TableName, TableSchema)} IN ACCESS EXCLUSIVE MODE")
-        //     .ExecuteNonQueryAsync(CreateRelationalCommandParameters(), cancellationToken)
-        //     .ConfigureAwait(false);
+        await Dependencies.RawSqlCommandBuilder
+            .Build($"LOCK TABLE {Dependencies.SqlGenerationHelper.DelimitIdentifier(TableName, TableSchema)} IN ACCESS EXCLUSIVE MODE")
+            .ExecuteNonQueryAsync(CreateRelationalCommandParameters(), cancellationToken)
+            .ConfigureAwait(false);
 
-        return Task.FromResult<IAsyncDisposable>(new DummyDisposable());
+        return new NpgsqlMigrationDatabaseLock(this);
     }
 
+    private RelationalCommandParameterObject CreateRelationalCommandParameters()
+        => new(
+            Dependencies.Connection,
+            null,
+            null,
+            Dependencies.CurrentContext.Context,
+            Dependencies.CommandLogger, CommandSource.Migrations);
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -127,6 +101,48 @@ SELECT 1 FROM pg_catalog.pg_class c
     protected override bool InterpretExistsResult(object? value)
         => (bool?)value == true;
 
+    bool IHistoryRepository.CreateIfNotExists()
+    {
+        // In PG, doing CREATE TABLE IF NOT EXISTS concurrently can result in a unique constraint violation
+        // (duplicate key value violates unique constraint "pg_type_typname_nsp_index"). We catch this and report that the table wasn't
+        // created.
+        try
+        {
+            return Dependencies.MigrationCommandExecutor.ExecuteNonQuery(
+                    GetCreateIfNotExistsCommands(), Dependencies.Connection, new MigrationExecutionState(), commitTransaction: true)
+                != 0;
+        }
+        catch (PostgresException e) when (e.SqlState == "23505")
+        {
+            return false;
+        }
+    }
+
+    async Task<bool> IHistoryRepository.CreateIfNotExistsAsync(CancellationToken cancellationToken)
+    {
+        // In PG, doing CREATE TABLE IF NOT EXISTS concurrently can result in a unique constraint violation
+        // (duplicate key value violates unique constraint "pg_type_typname_nsp_index"). We catch this and report that the table wasn't
+        // created.
+        try
+        {
+            return (await Dependencies.MigrationCommandExecutor.ExecuteNonQueryAsync(
+                    GetCreateIfNotExistsCommands(), Dependencies.Connection, new MigrationExecutionState(), commitTransaction: true,
+                    cancellationToken: cancellationToken).ConfigureAwait(false))
+                != 0;
+        }
+        catch (PostgresException e) when (e.SqlState == "23505")
+        {
+            return false;
+        }
+    }
+
+    private IReadOnlyList<MigrationCommand> GetCreateIfNotExistsCommands()
+        => Dependencies.MigrationsSqlGenerator.Generate([new SqlOperation
+        {
+            Sql = GetCreateIfNotExistsScript(),
+            SuppressTransaction = true
+        }]);
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -136,7 +152,7 @@ protected override bool InterpretExistsResult(object? value)
     public override string GetCreateIfNotExistsScript()
     {
         var script = GetCreateScript();
-        return script.Insert(script.IndexOf("CREATE TABLE", StringComparison.Ordinal) + 12, " IF NOT EXISTS");
+        return script.Replace("CREATE TABLE", "CREATE TABLE IF NOT EXISTS");
     }
 
     /// <summary>
@@ -178,8 +194,16 @@ public override string GetEndIfScript()
 END $EF$;
 """;
 
-    private sealed class DummyDisposable : IDisposable, IAsyncDisposable
+    private sealed class NpgsqlMigrationDatabaseLock(IHistoryRepository historyRepository) : IMigrationsDatabaseLock
     {
+        /// <summary>
+        ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+        ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+        ///     any release. You should only use it directly in your code with extreme caution and knowing that
+        ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+        /// </summary>
+        public IHistoryRepository HistoryRepository => historyRepository;
+
         public void Dispose()
         {
         }

src/EFCore.PG/Migrations/Internal/NpgsqlMigrator.cs

@@ -39,10 +39,11 @@ public NpgsqlMigrator(
         IDatabaseProvider databaseProvider,
         IMigrationsModelDiffer migrationsModelDiffer,
         IDesignTimeModel designTimeModel,
-        IDbContextOptions contextOptions)
+        IDbContextOptions contextOptions,
+        IExecutionStrategy executionStrategy)
         : base(migrationsAssembly, historyRepository, databaseCreator, migrationsSqlGenerator, rawSqlCommandBuilder,
             migrationCommandExecutor, connection, sqlGenerationHelper, currentContext, modelRuntimeInitializer, logger,
-            commandLogger, databaseProvider, migrationsModelDiffer, designTimeModel, contextOptions)
+            commandLogger, databaseProvider, migrationsModelDiffer, designTimeModel, contextOptions, executionStrategy)
     {
         _historyRepository = historyRepository;
         _connection = connection;

test/EFCore.PG.FunctionalTests/ComputedColumnTest.cs

@@ -139,9 +139,6 @@ public void Can_use_computed_columns_with_nullable_enum()
     public async Task InitializeAsync()
         => TestStore = await NpgsqlTestStore.CreateInitializedAsync("ComputedColumnTest");
 
-    public Task DisposeAsync()
-    {
-        TestStore.Dispose();
-        return Task.CompletedTask;
-    }
+    public async Task DisposeAsync()
+        => await TestStore.DisposeAsync();
 }

test/EFCore.PG.FunctionalTests/Migrations/MigrationsInfrastructureNpgsqlTest.cs

@@ -7,25 +7,6 @@ namespace Npgsql.EntityFrameworkCore.PostgreSQL.Migrations
     public class MigrationsInfrastructureNpgsqlTest(MigrationsInfrastructureNpgsqlTest.MigrationsInfrastructureNpgsqlFixture fixture)
         : MigrationsInfrastructureTestBase<MigrationsInfrastructureNpgsqlTest.MigrationsInfrastructureNpgsqlFixture>(fixture)
     {
-        // TODO: The following test the migration lock, which isn't yet implemented - waiting for EF-side fixes in rc.2
-        #region Unskip for 9.0.0-rc.2
-
-        public override void Can_apply_one_migration_in_parallel()
-        {
-        }
-
-        public override Task Can_apply_one_migration_in_parallel_async()
-            => Task.CompletedTask;
-
-        public override void Can_apply_second_migration_in_parallel()
-        {
-        }
-
-        public override Task Can_apply_second_migration_in_parallel_async()
-            => Task.CompletedTask;
-
-        #endregion Unskip for 9.0.0-rc.2
-
         public override void Can_get_active_provider()
         {
             base.Can_get_active_provider();

test/EFCore.PG.FunctionalTests/NpgsqlValueGenerationScenariosTest.cs

@@ -10,7 +10,7 @@ public class NpgsqlValueGenerationScenariosTest
     [Fact]
     public async Task Insert_with_sequence_id()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
 
         using (var context = new BlogContextSequence(testStore.Name))
         {
@@ -35,7 +35,7 @@ public class BlogContextSequence(string databaseName) : ContextBase(databaseName
     [Fact]
     public async Task Insert_with_sequence_HiLo()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
 
         using (var context = new BlogContextHiLo(testStore.Name))
         {
@@ -68,7 +68,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
     [Fact]
     public async Task Insert_with_default_value_from_sequence()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
 
         using (var context = new BlogContextDefaultValue(testStore.Name))
         {
@@ -146,7 +146,7 @@ public class BlogWithStringKey
     [Fact]
     public async Task Insert_with_key_default_value_from_sequence()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
 
         using (var context = new BlogContextKeyColumnWithDefaultValue(testStore.Name))
         {
@@ -187,7 +187,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
     [ConditionalFact]
     public async Task Insert_uint_to_Identity_column_using_value_converter()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
         using (var context = new BlogContextUIntToIdentityUsingValueConverter(testStore.Name))
         {
             context.Database.EnsureCreatedResiliently();
@@ -231,7 +231,7 @@ public class BlogWithUIntKey
     [ConditionalFact]
     public async Task Insert_string_to_Identity_column_using_value_converter()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
         using (var context = new BlogContextStringToIdentityUsingValueConverter(testStore.Name))
         {
             context.Database.EnsureCreatedResiliently();
@@ -276,7 +276,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
     [Fact]
     public async Task  Insert_with_explicit_non_default_keys()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
 
         using (var context = new BlogContextNoKeyGeneration(testStore.Name))
         {
@@ -312,7 +312,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
     [Fact]
     public async Task  Insert_with_explicit_with_default_keys()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
 
         using (var context = new BlogContextNoKeyGenerationNullableKey(testStore.Name))
         {
@@ -348,7 +348,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
     [Fact]
     public async Task  Insert_with_non_key_default_value()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
 
         using (var context = new BlogContextNonKeyDefaultValue(testStore.Name))
         {
@@ -401,7 +401,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
     [Fact]
     public async Task  Insert_with_non_key_default_value_readonly()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
 
         using (var context = new BlogContextNonKeyReadOnlyDefaultValue(testStore.Name))
         {
@@ -455,7 +455,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
     [Fact]
     public async Task  Insert_with_serial_non_id()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
 
         int afterSave;
 
@@ -493,7 +493,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
     [Fact]
     public async Task  Insert_with_client_generated_GUID_key()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
 
         Guid afterSave;
         using (var context = new BlogContext(testStore.Name))
@@ -518,7 +518,7 @@ public class BlogContext(string databaseName) : ContextBase(databaseName);
     [Fact]
     public async Task  Insert_with_server_generated_GUID_key()
     {
-        using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
+        await using var testStore = await NpgsqlTestStore.CreateInitializedAsync(DatabaseName);
 
         Guid afterSave;
         using (var context = new BlogContextServerGuidKey(testStore.Name))

test/EFCore.PG.FunctionalTests/Query/CompatibilityQueryNpgsqlTest.cs

@@ -84,8 +84,8 @@ public virtual void Dispose()
         {
         }
 
-        public virtual Task DisposeAsync()
-            => _testStore.DisposeAsync();
+        public virtual async Task DisposeAsync()
+            => await _testStore.DisposeAsync();
     }
 
     public class CompatibilityTestEntity