Repository navigation
Update pattern for scaffolding column default constraints #13613
Description
When using database first, creating a table (MSSQL) with a a bit field that has a default value produces strange results.
MSSQL Script:
CREATE TABLE [dbo].[Account] (
[Id] INT NOT NULL IDENTITY(1,1),
[EmailAddress] VARCHAR(256) NOT NULL,
[PasswordSalt] BINARY(512) NOT NULL,
[PasswordHash] BINARY(512) NOT NULL,
[Verified] BIT NOT NULL CONSTRAINT DF_Account_Verified DEFAULT (0),
[UtcDateCreated] DATETIME NOT NULL CONSTRAINT DF_Account_UtcDateCreated DEFAULT (GETUTCDATE()),
[Active] BIT NOT NULL CONSTRAINT DF_Account_Active DEFAULT (1),
CONSTRAINT PK_Account_Id PRIMARY KEY ([Id]),
CONSTRAINT UQ_Account_EmailAddress UNIQUE ([EmailAddress])
);
Scaffolding command:
Scaffold-DbContext "Server=172.16.1.140;Database=DocStudio;User Id=<uid>;Password=<pass>;" Microsoft.EntityFrameworkCore.SqlServer -OutputDir Models -f
Generated model :
using System;
using System.Collections.Generic;
namespace Namespace.Models
{
public partial class Account
{
public int Id { get; set; }
public string EmailAddress { get; set; }
public byte[] PasswordSalt { get; set; }
public byte[] PasswordHash { get; set; }
public bool Verified { get; set; }
public DateTime UtcDateCreated { get; set; }
public bool? Active { get; set; }
}
}
and finally, the fluent OnModelCreating method:
protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Account>(entity =>
{
entity.HasIndex(e => e.EmailAddress)
.HasName("UQ_Account_EmailAddress")
.IsUnique();
entity.Property(e => e.Active)
.IsRequired()
.HasDefaultValueSql("((1))");
entity.Property(e => e.EmailAddress)
.IsRequired()
.HasMaxLength(256)
.IsUnicode(false);
entity.Property(e => e.PasswordHash)
.IsRequired()
.HasMaxLength(512);
entity.Property(e => e.PasswordSalt)
.IsRequired()
.HasMaxLength(512);
entity.Property(e => e.UtcDateCreated)
.HasColumnType("datetime")
.HasDefaultValueSql("(getutcdate())");
});
}
Several issues here. Note the difference between Verified and Active. Despite being identical in their definition they produce different property types in the model.
The lack of a default value for Verified. There should be a definition like so:
entity.Property(e => e.Verified)
.IsRequired()
.HasDefaultValueSql("((0))");
Missing a primary key definition.
Finally a warning for Active, but not for Verified:
The column 'dbo.Account.Active' would normally be mapped to a non-nullable bool property, but it has a default constraint. Such a column is mapped to a nullable bool property to allow a difference between setting the property to false and invoking the default constraint.
Activity
olavivaino commentedon Oct 13, 2018
Struggling with the same issue. Its really not desirable to have "bool" in the DB and "bool?" in the code.
I'm wondering what is the reasoning behind this:
christopher-landress commentedon Oct 13, 2018
The most
It seems to be completely indiscriminate too. Verified was defined in the model correctly but is missing the default value in OnModelCreating whereas Active was defined incorrectly in the model but correctly in OnModelCreating. It also got the unique index, but ignored the primary key completely. Makes it impossible to use scaffolding in a build task without constantly fudging with the model. M(sometimes)VC.
ajcvickers commentedon Oct 14, 2018
@christopher-landress @olavivaino With regard to the boolean default values, can you give some details on you wish these default values to be used from EF? Should they be included in the model, and, if so, why?
@christopher-landress If a primary key is being ignored, can you post the details on the key that is being ignored--this sounds like a bug.
christopher-landress commentedon Oct 14, 2018
The default value should be whatever the constraint specifies. In the above case Verified should be 0 and Active should be 1... specified by :
CONSTRAINT DF_Account_Verified DEFAULT (0)and
CONSTRAINT DF_Account_Active DEFAULT (1)If NOT NULL & a default constraint are defined for a bit field, it should never be specified with a Boolean? type. I would argue that Boolean? should only be specified when the value could be null, but even then if there is a default it shouldn't be a nullable type.
As far as the primary key is concerned, I would have expected to see the following:
This one appears to be present :
I expected to see the following but it was missing :
This happens with the release version of the tools as well as the pre-release. Very simple to test with the above sql script.
ajcvickers commentedon Oct 14, 2018
@christopher-landress Assume that the reverse engineering is doing what you expect, such that you have two bool properties in your model, both with HasDefaultValue (or HasDefaultValueSql) set. Can you describe how you will then use these properties? What will the default value do? Why is it there?
(I'm not asking to be mean, but because how to deal with column defaults depends on what they are used for, so I'm trying to understand what your expectation is once the default gets into the model.)
christopher-landress commentedon Oct 14, 2018
The way I see it, and maybe I am over/under thinking it, is the advantage of having default values for future use. Neither of which are initially important to the insert. Verified would be updated to true once the email address is validated, and Active would work as a soft delete for the record. This could absolutely be accomplished with different schema but I feel like that defeats the purpose of db first. I don't feel like it's unreasonable to set the initial state of the record without intervention, especially when those values aren't designed to be visible. Some of us like to stuff as much logic as we can into the database, assuming it can be used across multiple backends. What is the advantage of having to set Verified and Active explicitly before inserting when my schema is designed to set those values for me?
I also use PostgreSQL, which supports schema inheritance. Having a base table that supports keys, timestamps, and soft deletes is very helpful if I can assume the values that get set are standardized.
Perhaps I am missing your point. Maybe it's a disconnect between developers who like to stuff as much logic into the database as possible and those who don't. I'm having a hard time believing other people aren't having this same issue.
ajcvickers commentedon Oct 15, 2018
@christopher-landress Consider this entity type:
where IsValid has a column default of
trueand this is mapped correctly in the EF model.After inserting the the following entity instance, what would you expect the IsValid column value to be set to?
Likewise, for this:
And for this one:
christopher-landress commentedon Oct 15, 2018
I understand the limitation of the model's type.
Is there no way to fix this in OnModelCreating without using a nullable type?
ajcvickers commentedon Oct 15, 2018
@christopher-landress We're open to ideas.
ajcvickers commentedon Oct 15, 2018
Note for triage: thought about this some more last night and while this doesn't work now maybe we could make it work:
The idea is that the update pipeline reads from the backing field and so can tell whether or not an explicit value has been set. But the entity type still has a non-nullable property so application code doesn't need to be changed to handle this.
[-]Default values for bit fields breaks scaffolding.[/-][+]Update pattern for scaffolding column default constraints[/+]31 remaining items
kingmotley commentedon Jun 2, 2023
I believe the confusion comes in because some may want the database default value to hold special meaning in the EF model object.
I personally really don't have any expectation of that sort. The default value is for migrations so the database knows what value to use when I add the new Boolean column and for 3rd party non-EF software to use. I just expect EF to be able to create/migrate the default value constraint and otherwise ignore it. I don't try to conflate the database default value to having any effect on the C#/EF property at all. If I "new" that class, it won't necessarily initialize to the value of the database default value.
In SQL if you have a bit (same for int/varchar), if you do the equivalent:
INSERT INTO MyTable(mycol) VALUES (0);the database does not say oh, he's inserting the default value into mycol, but that column has a default value of 1, then he meant to insert a 1. Nor will it accept it if you specify NULL in that column since that column isn't nullable. However, if you do the equivalent:
INSERT INTO MyTable() VALUES ();then since you have not specified THE COLUMN (which isn't possible in EF, or a model that has a property for every column), then it will use the database default value for that column.
I see these as two separate things. If you could do the following:
public class MyTable { public int Id {get;set;} public bool MyCol {get;set;}}and you_context.MyTables.Add(new MyTable());then the value of MyCol is false, and it does not matter what the database default value is, it should get inserted as false. However, given the same table, and you also had this class:public class MyTableWithMissingProperty { public int Id {get;set;} }and you_context.MyTables.Add(new MyTableWithMissingProperty());then yes, since that does not even have the MyCol property, EF should generate an insert statement without the column even mentioned, and the database would then insert the default value. EF currently does not accept a class that does not have all the properties (It can't be both a collection of MyTable and MyTableWithMissingProperty), so that just isn't possible today.Of course, if I generate my model, and then through another tool (SSMS) add a bool column and assign a default value, and my code continues to use the old model, EF will work as I expect. It will insert rows into the table and use the default value.
I hope that doesn't make things even less clear.
Parse simple default constraint literals when scaffolding
Parse default value literals for SQLite
No longer scaffold nullable booleans when column has a default value
mhkolk commentedon Mar 13, 2024
EF Core 8 still issues this warning at runtime
for the following property
public bool IsDeleted { get; set; }As before I still don't understand why the warning - what it is that we should be warned about in this case - don't get it.
If those warnings weren't removed from the new EF version, is there at least an option to disable them?