dotnet
Adding a required field to entity with without explicitly specifying a default value produces migrations that are inconsistent with model
When adding a new required field without a default value a default value. And adding creating a migration afterwards the up migration contains:
- an "add column step"
- with the column marked as non nullable (this is correct)
- but it also sets a default value (it seems to be the default value of the type (int = 0, datetime = minvalue etc.)
- again nowhere in the model a default value was specified
After executing this migration the result is:
- There is a default value set in the db
- Without it being reflected in the code (entity model / snapshot)
From my pov there are 2 options to fix this:
- option 1
- ef migrations add fails when no default value is explicitly specified
- this does mean if the column should not have a default value beyond the initial intend of filling nulls in the existing data
- we need to remove the default value and add another migration
- option 2
- when executing ef migrations add the resulting migration should include 2 steps for each column
- add column (with default value)
- and alter column (removing the default value)
- This would only be done if the user did not explicitly specify a default value for the new column
I do prefer the second option as it creates less unnecessary migrations, that said I can easily think of situations where it will be problem that ef just decides what the default value for existing data should be. Hence I would suggest to output a warning when migrations are created similar to danger of data loss. This would give the user the ability to double check the migration and adjust the "temporary" default value if needed.
Include provider and version information
EF Core version: 6.01 Database provider: I'm assuming this will apply to all relational providers Target framework: .NET 6.0 Operating system: Windows IDE: 2022 latest
@ntziolis This is currently by design. However, the team will discuss and consider if we should make a change here.
/cc @bricelam
@ajcvickers Tbh I assumed this is by design given as it would put a lot of strain on the dev to handle this each time. So totally get why it currently behaves like this right now.
That said it did make me quite uneasy to realize that non null is not hard enforced on db level even though the ef model "implies" it would be enforced (by not explicitly defining a default value). I think at the very least the a migration should also execute an alter column that removes the default value right after adding the column. This way the margin of potential issues is reduced to the data that existed prior to the column being added.
We went through 15+ projects (we are an SI) we created ion the last 2 years and indeed found some instances of fields not being set in our code, but inserts going through. I understand those bugs should not have slipped into prod in the first place, but I'm sure we are not the only ones. :)
If we had #5642, we could just generate an operation equivalent to UPDATE A SET X = 0 WHERE X IS NULL instead of abusing the default constraint. I believe the current behavior was carried over from EF6.
@bricelam interesting, while this would mean 3 steps are necessary (create column nullable / update to remove nulls / alter column to remove default value), it would open up the possibility of executing more complex "fill-the-nulls" logic. This would come in handy for new columns whos value can be derived from existing data where a default value approach would no really work well.
Until this is finalized, is there a current possible way to just make EF not generate a default value in the migration?
So the property is nun-nullable, the database is NOT NULL and has no default value set in SQL?
@wertzui You can edit the generated migration, assuming the table doesn't have existing data.
What we do is we edit the migration as follows:
- If necessary we update the default value for create column step
- We then add an alter column step right after the create step for the same column, where we set the column to be
- non nullable
- and we remove the default value
This procedure works both if there is data as well as if there is no data. And ensure the model is in line with the actual schema in the db.
Although I would have preferred changing something in the model and not in the migration, I will go with the workaround. Thanks @ajcvickers
@ntziolis although that is not needed in my case (no row has NULL in the table that I want to migrate), I'm aware of that pattern and have used it in the past quite often.
However when looking at how EF should handle this, I'm not sure if your proposed option 2 should be the default, as it might do unintended changes.
So I'm proposing
- option 3 (as it has been on older EF core versions and without NRTs)
- ef migrations add works and does not add a default value
- The generated SQL does not contain code that will change anything in the existing data
- The execution of the generated SQL fails if the column contains NULL values
- The user is forced to actually think how he wants to handle these values by either changing the data beforehand or changing the generated migration
Notes from triage:
- Consider leaving the database in a state where the rows have been updated to the CLR default, but the column doesn't have a default constraint anymore unless one is specified in the model.
- Consider warning if the model doesn't have a default constraint.
Maybe this could be made configurable with a switch or seomthing. I'd rather have errors on columns that should be non-null than having invalid business data in there from the migration
