-
Notifications
You must be signed in to change notification settings - Fork 77
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
__EFMigrationsHistory columns are snake_cased, upgrading needs manual alter script #1
Comments
@williamdenton isn't this the wrong way around? alter table "__EFMigrationsHistory"
rename column "MigrationId" to migration_id; Shouldn't you be renaming it back to You are right in raising this issue though. I am working with IdentityServer4 and that has two db contexts. One set of migrations works, but the other fails because of the column name issue. |
@JwanKhalaf in my case i already had a |
ah I see @williamdenton. Am I right in thinking that if one wishes to avoid manual intervention, one shouldn't currently use this package? |
Apologies, I was accidentally not subscribed to notifications on this repo, will try to investigate ASAP. |
@roji it might be useful to have a configuration object that controls which parts of the schema have the naming convention applied. then people can opt in to the bits they need/want, or migrate in a controlled manner |
@williamdenton allowing fine-grained configuration of which parts of the schema are affected seems a bit much to me... I have a hard time imagining a situation where people want to snake_case only columns but not tables as part of a gradual migration (but am open to hear otherwise!). For __EFMigrationHistory specifically, I have to investigate since this table has a specific meaning to EF Core, and I'm not sure how flexible things are there. It doesn't seem too terrible to leave this table outside the naming convention, as it's not exactly part of the model etc. But I'll definitely look into it. |
Any news on this? |
It's probably good to consider the default behavior with the "fix": I'll probably use the same approach as @williamdenton and rename my migration table so I can continue working with ef commands. I'd assume most ppl will do the same since it's nice to use this library and I don't want to wait for a fix on this. if I create a new database from scratch, the columns now are already created with the snake case convention. In cases where the database already exists (my case) the change suggested by @williamdenton will not be part of a migration but just a one time change, because I want to support ppl from creating a new db from scratch and since the app will be using the library all will be fine. If we say: We are not going to touch the __EFMigrationHistory table in this library, ppl that applied the fix now will again have migration problems. Hopefully I could explain the issues in a clear way. |
I think it'd be great if there's an option to avoid altering the In my case, I use IdentityServer4, running migrations for IdentityServer4 DB context will throw: 42703: column "migration_id" of relation "__EFMigrationsHistory" does not exist So I tried manually changing the columns back to how they were: ALTER TABLE "__EFMigrationsHistory" RENAME COLUMN "migration_id" TO "MigrationId";
ALTER TABLE "__EFMigrationsHistory" RENAME COLUMN product_version TO "ProductVersion"; Now running IdentityServer4 migrations will work: dotnet-ef database update -c ConfigurationDbContext
dotnet-ef database update -c PersistedGrantDbContext But this causes a new problem where if I create a new migration and remove it during development, it'll throw this error: dotnet-ef migrations remove -c ApplicationDbContext
Build started...
Build succeeded.
Npgsql.PostgresException (0x80004005): 42703: column "migration_id" does not exist I'll have to stick with using PascalCase for now |
@hantatsang
|
TEMP WORKAROUND:
This Creates your migration with the renaming, and when you actually apply it, you don't want EntityFramework to be trying to access column names that don't exist yet ("migration_id") So you apply it without using the "UseSnakeCaseNamingConvention(), because You have to do this song and dance EVERY TIME you want to perform Migrations. Might be easier to just rename your __EFMigrations columns like @williamdenton suggested |
When I first apply the naming conventions extension, I am seeing: SELECT migration_id, product_version Error: column "migration_id" does not exist |
what some people mentioned on this thread, the migrations table should be ignored from naming conventions. |
any update on this? |
Any update on this? calcasa@5594847 looks interesting. |
well I would not prefer it. I prefer that it also renames it, but a flag would be ok-ish |
I just created a new migration (the first on 6.0) and tried to apply it to a database that uses snake case naming from before. What happened is that migrations got applied without the snake casing and according to the migration history spanning longer back than when the database was renamed to snake case, this happened due to efcore not picking up on the |
I'm using the following workaround that is described in docs https://docs.microsoft.com/en-us/ef/core/managing-schemas/migrations/history-table public class CamelCaseHistoryContext : NpgsqlHistoryRepository
{
public CamelCaseHistoryContext(HistoryRepositoryDependencies dependencies) : base(dependencies)
{
}
protected override void ConfigureTable(EntityTypeBuilder<HistoryRow> history)
{
base.ConfigureTable(history);
history.Property(h => h.MigrationId).HasColumnName("MigrationId");
history.Property(h => h.ProductVersion).HasColumnName("ProductVersion");
}
}
// later in code
opts.UseNpgsql()
.ReplaceService<IHistoryRepository, CamelCaseHistoryContext>()
.UseSnakeCaseNamingConvention(); Replace |
@alfeg worked to me! |
@alfeg's solution works but let's pay attention to https://www.npgsql.org/efcore/api/Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.Internal.NpgsqlHistoryRepository.html and the warning
A cleaner solution from EFCore would be better :) |
An extra gotcha that maybe worth mentioning in the readme:
Referencing this package also snake_cases the columns in the
__EFMigrationsHistory
table (though interestingly, not the table name itself). This means migrations fail as they can not run a version check to obtain the current database version.This was solved by running this script on the DB
I have been using code very similar to dotnet/efcore#5159 (comment) to provide snake case columns in my DB, so all columns are already snake cased.
Moving over to this package did try to recreate primary keys (as warned in the readme) however in my case postgres already had the constraint name in lower case so commenting out the generated migration and letting it run though as a no-op has successfully aligned the stars for me (on a very small demo codebase).
Had the constraint name actually needed an update i would have replace the generated migration with this rather than running the generated drop/create
Console output from running
dotnet ef database update --context DemoMigratorDbContext
The text was updated successfully, but these errors were encountered: