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

Missing "public" schema fallback for migration table exists query #2787

Closed
Turnerj opened this issue Jun 5, 2023 · 3 comments · Fixed by #2788
Closed

Missing "public" schema fallback for migration table exists query #2787

Turnerj opened this issue Jun 5, 2023 · 3 comments · Fixed by #2788
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Turnerj
Copy link

Turnerj commented Jun 5, 2023

There is a subtle bug with how the migration table exists query works when dealing with a combination of defined and non-defined schemas in a database (I know that is unusual but it is what it is).

The problem is this block of code:

protected override string ExistsSql
{
get
{
var builder = new StringBuilder();
builder.Append("SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n ON n.oid=c.relnamespace WHERE ");
var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string));
if (TableSchema is not null)
{
builder
.Append("n.nspname=")
.Append(stringTypeMapping.GenerateSqlLiteral(TableSchema))
.Append(" AND ");
}
builder
.Append("c.relname=")
.Append(stringTypeMapping.GenerateSqlLiteral(TableName))
.Append(");");
return builder.ToString();
}
}

If you don't specify a schema, that query will actually match against all schemas rather than filter to the default public schema. This means if you have a migration in another schema, that will be picked up even though it isn't valid for the current migration.

As far as I can tell, you can probably just do this without the outer null-check to resolve it:

         builder 
             .Append("n.nspname=") 
             .Append(stringTypeMapping.GenerateSqlLiteral(TableSchema ?? "public")) 
             .Append(" AND "); 

This was against Npgsql.EntityFrameworkCore.PostgreSQL v6.0.8


Steps to recreate issue:

  • Have two contexts, one with a schema, one without
  • Run a migration for the context with the schema
  • Attempt to run a migration for the context without the schema
roji added a commit to roji/efcore.pg that referenced this issue Jun 5, 2023
@roji
Copy link
Member

roji commented Jun 5, 2023

Thanks, makes sense! I submitted #2788 to fix this, I'll also backport this.

@roji roji added this to the 8.0.0 milestone Jun 5, 2023
@roji roji self-assigned this Jun 5, 2023
@roji roji added the bug Something isn't working label Jun 5, 2023
@roji roji closed this as completed in #2788 Jun 5, 2023
@roji roji modified the milestones: 8.0.0, 7.0.6 Jun 5, 2023
@Turnerj
Copy link
Author

Turnerj commented Jun 5, 2023

Cheers mate, appreciate the quick response on this!

renovate bot referenced this issue in orso-co/Orso.Arpa.Api Sep 18, 2023
)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
|
[Npgsql.EntityFrameworkCore.PostgreSQL](https://github.com/npgsql/efcore.pg)
| nuget | patch | `7.0.4` -> `7.0.11` |

---

### Release Notes

<details>
<summary>npgsql/efcore.pg
(Npgsql.EntityFrameworkCore.PostgreSQL)</summary>

###
[`v7.0.11`](https://github.com/npgsql/efcore.pg/releases/tag/v7.0.11)

This patch release fixes a single bug
([https://github.com/npgsql/efcore.pg/issues/2787](https://github.com/npgsql/efcore.pg/issues/2787)),
but also takes a dependency on the latest EF Core and Npgsql patch
versions. Everyone is encouraged to upgrade.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 10pm every weekday,every
weekend,before 5am every weekday" in timezone Europe/Berlin, Automerge -
At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/orso-co/Orso.Arpa.Api).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi44My4wIiwidXBkYXRlZEluVmVyIjoiMzYuODMuMCIsInRhcmdldEJyYW5jaCI6ImRldmVsb3AifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@tnotheis
Copy link

tnotheis commented Oct 5, 2023

After upgrading to version 7.0.11, I receive the following error when running my app:

Unhandled exception. Npgsql.PostgresException (0x80004005): 42P07: relation "__EFMigrationsHistory" already exists

Here's a little bit of context about my app:
I have multiple DbContexts, where each of them has its own connection string with its own user, where each user has a separate default schema. I was creating the __EFMigrationsHistory tables for each DbContext manually as part of an database initialization script, which I had to run once after creating a new database. This was working fine.

But with your change, all DbContexts seem to try to create a __EFMigrationsHistory in the "public" schema, which, of course, isn't possible.

Tomorrow I will try to create a minimal example for you to reproduce this, and open a dedicated issue. But maybe you can already have a look into this.

Thanks.

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

Successfully merging a pull request may close this issue.

3 participants