Unexpected Reverse Engineering for Bit Field

[jostapotcf]

Describe what is not working as expected.

Given the SQL Server table

USE [CuratedPhase1]

SET ANSI_NULLS ON

SET QUOTED_IDENTIFIER ON

CREATE TABLE [dbo].[FanProductPerformanceCorrections](
    [FanProductId] [uniqueidentifier] NOT NULL,
    [FanPerformanceCorrectionId] [uniqueidentifier] NOT NULL,
    [CorrectionOrder] [smallint] NOT NULL,
    [MustHave] [bit] NOT NULL,
    [ApplyCorrection] [bit] NOT NULL,
 CONSTRAINT [PK_FanProductPerformanceCorrections] PRIMARY KEY CLUSTERED 
(
    [FanProductId] ASC,
    [FanPerformanceCorrectionId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON)
)

GO

Expect the following reverse engineering result

    public partial class FanProductPerformanceCorrection
    {
        public FanProductPerformanceCorrection()
        {
            FanProductPerformanceCorrectionVariables = new HashSet<FanProductPerformanceCorrectionVariable>();
        }

        public Guid FanProductId { get; set; }
        public Guid FanPerformanceCorrectionId { get; set; }
        public short CorrectionOrder { get; set; }
        public bool MustHave { get; set; }
        public bool ApplyCorrection { get; set; }

        public virtual FanPerformanceCorrection FanPerformanceCorrection { get; set; }
        public virtual FanProduct FanProduct { get; set; }
        public virtual ICollection<FanProductPerformanceCorrectionVariable> FanProductPerformanceCorrectionVariables { get; set; }
    }

but getting

 public partial class FanProductPerformanceCorrection
    {
        public FanProductPerformanceCorrection()
        {
            FanProductPerformanceCorrectionVariables = new HashSet<FanProductPerformanceCorrectionVariable>();
        }

        public Guid FanProductId { get; set; }
        public Guid FanPerformanceCorrectionId { get; set; }
        public short CorrectionOrder { get; set; }
        public bool MustHave { get; set; }
        public bool? ApplyCorrection { get; set; }

        public virtual FanPerformanceCorrection FanPerformanceCorrection { get; set; }
        public virtual FanProduct FanProduct { get; set; }
        public virtual ICollection<FanProductPerformanceCorrectionVariable> FanProductPerformanceCorrectionVariables { get; set; }
    }

Note that APPLYCORRECTION in the second C# example is marked as nullable (bool?).

This behavior has been present for at least the last six months, and is not specific to recent builds. (we've been fixing it by hand)

[ErikEJ]

Does the column have a default value defined?

[jostapotcf]

Hi Erik,

It does. It's ((1)). I re-scripted the full table spec, so straight from the horse's mouth.

USE [CuratedPhase1]
/****** Object:  Table [dbo].[FanProductPerformanceCorrections]    Script Date: 8/12/2019 10:55:48 AM ******/
SET ANSI_NULLS ON
SET QUOTED_IDENTIFIER ON
CREATE TABLE [dbo].[FanProductPerformanceCorrections](
    [FanProductId] [uniqueidentifier] NOT NULL,
    [FanPerformanceCorrectionId] [uniqueidentifier] NOT NULL,
    [CorrectionOrder] [smallint] NOT NULL,
    [MustHave] [bit] NOT NULL,
    [ApplyCorrection] [bit] NOT NULL,
 CONSTRAINT [PK_FanProductPerformanceCorrections] PRIMARY KEY CLUSTERED 
(
    [FanProductId] ASC,
    [FanPerformanceCorrectionId] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY]

ALTER TABLE [dbo].[FanProductPerformanceCorrections] ADD  CONSTRAINT [DF_FanProductPerformanceCorrections_MustHaveCorrection]  DEFAULT ((0)) FOR [MustHave]
ALTER TABLE [dbo].[FanProductPerformanceCorrections] ADD  CONSTRAINT [DF_FanProductPerformanceCorrections_MustNotHave]  DEFAULT ((1)) FOR [ApplyCorrection]
ALTER TABLE [dbo].[FanProductPerformanceCorrections]  WITH CHECK ADD  CONSTRAINT [FK_FanProductPerformanceCorrections_FanPerformanceCorrections] FOREIGN KEY([FanPerformanceCorrectionId])
REFERENCES [dbo].[FanPerformanceCorrections] ([FanPerformanceCorrectionId])
ALTER TABLE [dbo].[FanProductPerformanceCorrections] CHECK CONSTRAINT [FK_FanProductPerformanceCorrections_FanPerformanceCorrections]
ALTER TABLE [dbo].[FanProductPerformanceCorrections]  WITH CHECK ADD  CONSTRAINT [FK_FanProductPerformanceCorrections_FanProducts] FOREIGN KEY([FanProductId])
REFERENCES [dbo].[FanProducts] ([FanProductId])
ALTER TABLE [dbo].[FanProductPerformanceCorrections] CHECK CONSTRAINT [FK_FanProductPerformanceCorrections_FanProducts]

[ErikEJ]

It is because of the default that it is scripted as nullable. Drop the default (I think this might have changed in 3.0 @bricelam ? )

[jostapotcf]

Understood. I'm prepping my response for #235, and trying to be thorough, noting any strange/unexpected behaviors. (looking good so far)

Feel free to close this one.

[ErikEJ]

Thanks! I will close this, it is a well known EF Core quirk

[bricelam]

This is expected. EF Core uses the CLR default value to determine if it should use the SQL default. If it wasn't nullable you could never save false:

With a bool property:

.NET	SQL
false	1 (via DEFAULT)
true	1

With a bool? property:

.NET	SQL
null	1 (via DEFAULT)
false	0
true	1

[bricelam]

Note, if you want just a bool column, remove the HasDefaultValue call so EF Core always sends the value to the database and doesn't try to use the DEFAULT constraint.

[popcatalin81]

Devs are more likely to "correct" the nullable bool thinking it's an RE bug than to remove the SQL Default and to make the bool not nullable. Since this feature was introduced, I've seen it baffle every and each dev who encountered it and was working with a column which was not matching the CLR default (IE: post.Visible = false, inserts true in DB).

[bricelam]

👍 We've done a lot of thinking about how to improve it. See https://github.com/aspnet/EntityFrameworkCore/issues/13613

[ErikEJ]

A new advanced option is now available to work around this issue.

[jostapotcf]

A new advanced option is now available to work around this issue.

Hey Erik. Thanks for this!

We're going to be making a refactoring pass on our database here in the next few weeks (major version increment). I've added a devops note to check this. If it's helpful, I'll respond here with confirmation.