Skip to content
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

Sync to EF Core 9.0.0-rc.2.24460.3 #3275

Merged
merged 1 commit into from
Sep 13, 2024
Merged

Sync to EF Core 9.0.0-rc.2.24460.3 #3275

merged 1 commit into from
Sep 13, 2024

Conversation

roji
Copy link
Member

@roji roji commented Sep 13, 2024

Closes #3223


/// <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()
Copy link
Member Author

Choose a reason for hiding this comment

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

@AndriySvyryd here's the PG implementation of the final implementation of the migration lock... Everything seems OK (at least the MigrationsInfrastructureTestBase.Can_apply_*_in_parallel are passing).

Note that NpgsqlMigrationDatabaseLock is a no-op, as it generally will always be when LockReleaseBehavior is Transaction (since the lock is implicitly released when the transaction terminates, and not before). So it effectively functions only as a wrapper for MigrationHistory - feels a bit weird, but fine.

GetCreateIfNotExistsCommands(), Dependencies.Connection, new MigrationExecutionState(), commitTransaction: true)
!= 0;
}
catch (PostgresException e) when (e.SqlState == "23505")
Copy link
Member Author

Choose a reason for hiding this comment

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

@AndriySvyryd note this annoying thing to deal with the PG problem of doing CREATE TABLE IF NOT EXISTS in parallel :/

Choose a reason for hiding this comment

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

It's only a real issue when running test code that exercises these convoluted scenarios. I'd say that the correct thing here is to let it throw and handle it in the execution strategy used in tests

@roji roji enabled auto-merge (squash) September 13, 2024 19:55
@roji roji merged commit b11d279 into npgsql:main Sep 13, 2024
16 checks passed
@roji roji deleted the Sync branch September 13, 2024 20:04
@@ -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");

Choose a reason for hiding this comment

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

Nothing like some string-based code reuse 😆

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.

Implement migration locking
2 participants