Open williamdenton opened 5 years ago
@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 MigrationId
?
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 __EFMigrationsHistory
table, after adding the package, the ef query that inspects this table for migrations required snake cased columns
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. The problem I can forsee though is:
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 __EFMigrationsHistory
table.
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 try it
services.AddIdentityServer()
.AddConfigurationStore(options =>
{
options.ConfigureDbContext = builder =>
{
builder.UseSnakeCaseNamingConvention();
builder.UseMySql(connectionString,
sql => sql.MigrationsAssembly(migrationsAssembly));
};
})
.AddOperationalStore(options =>
{
options.ConfigureDbContext = builder =>
{
builder.UseSnakeCaseNamingConvention();
builder.UseMySql(connectionString,
sql => sql.MigrationsAssembly(migrationsAssembly));
};
})
TEMP WORKAROUND:
dotnet ef migrations add RenameToSnakeCase
(opt => opt.UseNpgsql(connectionStr).
dotnet ef database update
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 dotnet ef
will build your project, and won't be trying to access a database with the name "migration_id" this time.
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 But I don't know if this will cause other issues ¯_(ツ)_/¯
When I first apply the naming conventions extension, I am seeing:
SELECT migration_id, product_version FROM "__EFMigrationsHistory" ORDER BY migration_id;
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?
https://github.com/calcasa/EFCore.NamingConventions/commit/55948474e6d9f3811d54cc9cf3f95c9070eae141 looks interesting.
what some people mentioned on this thread, the migrations table should be ignored from naming conventions.
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 __efmigrationshistory
table and assuming that the database was "fresh". The fix was renaming __efmigrationshistory
to __EFMigrationsHistory
.
The columns in __EFMigrationsHistory
did not need to be updated. Could look like efcore is case-sensitive on 6.0 and upwards because the lower-cased __EFMigrationsHistory
worked fine on 5.0.
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 NpgsqlHistoryRepository
with You DB provider implementation. Take into account that those classes are internal EF API and subject to change.
@alfeg worked to me!
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 https://github.com/aspnet/EntityFrameworkCore/issues/5159#issuecomment-522223918 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