Adds unexpected constraints when using together with [Required] attribute

[hmundle]

When updating to version 8, EFCore.CheckConstraints breaks EF code first usage.
I mainly use Postgres.

It turned out, that the usage of [Required] attribute creates additional constraints, which did not show up in version 6 (and probably in version 7).

• together with [Range(1, 5)] on type int, MinLength will added, at least with Postgres this will fail, because LENGTH() is only supported for strings
• together with the Id property of type int, the same will happen
• together with StringLength() two MinLength constraints will be added, which fails immediately.

This can be tested easily with EFCore.CheckConstraints.Test unit tests:
Add [Required] to Blog.Id, Blog.Rating and Blog.Required.

Add this test as well:

    [Fact]
    public virtual void RequiredNoString()
    {
        var entityType = BuildEntityType<Blog>();

        var constraints = entityType.GetCheckConstraints();
        Assert.DoesNotContain(constraints, c => c.Name == "CK_Blog_Id_MinLength");
        Assert.DoesNotContain(constraints, c => c.Name == "CK_Blog_Rating_MinLength");
    }

It would be great, if this situation can be improved.

[roji]

@hmundle can you please post a quick code sample showing the code that worked before 8.0 and now fails? That's always much better than trying to describe the situation with words.

[hmundle]

For testing the issue I used https://github.com/efcore/EFCore.CheckConstraints repository, tag v8.0.1 and v7.0.2.

On v8.0.1 I updated EFCore.CheckConstraints.Test/ValidationCheckConstraintTest.cs to

diff --git a/EFCore.CheckConstraints.Test/ValidationCheckConstraintTest.cs b/EFCore.CheckConstraints.Test/ValidationCheckConstraintTest.cs
index 9036f50..4a6b289 100644
--- a/EFCore.CheckConstraints.Test/ValidationCheckConstraintTest.cs
+++ b/EFCore.CheckConstraints.Test/ValidationCheckConstraintTest.cs
@@ -16,6 +16,16 @@ namespace EFCore.CheckConstraints.Test;
 
 public class ValidationCheckConstraintTest
 {
+    [Fact]
+    public virtual void RequiredNoString()
+    {
+        var entityType = BuildEntityType<Blog>();
+
+        var constraints = entityType.GetCheckConstraints();
+        Assert.DoesNotContain(constraints, c => c.Name == "CK_Blog_Id_MinLength");
+        Assert.DoesNotContain(constraints, c => c.Name == "CK_Blog_Rating_MinLength");
+    }
+
     [Fact]
     public virtual void Range()
     {
@@ -197,8 +207,10 @@ public virtual void Properties_on_complex_type()
     // ReSharper disable UnusedMember.Local
     class Blog
     {
+        [Required]
         public int Id { get; set; }
 
+        [Required]
         [Range(1, 5)]
         public int Rating { get; set; }
 
@@ -214,6 +226,7 @@ class Blog
         [Length(2, 5)]
         public required string StringWithLength { get; set; }
 
+        //[Required]
         [StringLength(100, MinimumLength = 1)]
         public required string Required { get; set; }
 

On v7.0.2 to:

diff --git a/EFCore.CheckConstraints.Test/ValidationCheckConstraintTest.cs b/EFCore.CheckConstraints.Test/ValidationCheckConstraintTest.cs
index eea007b..d44867d 100644
--- a/EFCore.CheckConstraints.Test/ValidationCheckConstraintTest.cs
+++ b/EFCore.CheckConstraints.Test/ValidationCheckConstraintTest.cs
@@ -16,6 +16,16 @@ namespace EFCore.CheckConstraints.Test;
 
 public class ValidationCheckConstraintTest
 {
+    [Fact]
+    public virtual void RequiredNoString()
+    {
+        var entityType = BuildEntityType<Blog>();
+
+        var constraints = entityType.GetCheckConstraints();
+        Assert.DoesNotContain(constraints, c => c.Name == "CK_Blog_Id_MinLength");
+        Assert.DoesNotContain(constraints, c => c.Name == "CK_Blog_Rating_MinLength");
+    }
+
     [Fact]
     public virtual void Range()
     {
@@ -117,14 +127,17 @@ public virtual void RegularExpression()
     // ReSharper disable UnusedMember.Local
     class Blog
     {
+        [Required]
         public int Id { get; set; }
 
+        [Required]
         [Range(1, 5)]
         public int Rating { get; set; }
 
         [MinLength(4)]
         public string Name { get; set; } = null!;
 
+        [Required]
         [StringLength(100, MinimumLength = 1)]
         public string Required { get; set; } = null!;
 

Test EFCore.CheckConstraints.Test.ValidationCheckConstraintTest.RequiredNoString fails on 8.0.1, on 7.0.2 it runs without issue.
If you uncomment the //[Required] part in 8.0.1, you will get an System.InvalidOperationException, the issue in the third bullet.

[cremor]

I have a similar problem after updating from 7.0.2 to 8.0.1 with MS SQL Server. My version properties for optimistic concurrency checks are implemented like this:

[Timestamp, Required]
public byte[]? Version { get; set; }

EFCore.CheckConstraints 8.0.1 now creates a new unnecessary MinLength constraint for this:

t.HasCheckConstraint("CK_TodoItems_Version_MinLength", "LEN([Version]) >= 1");

Using Required(AllowEmptyStrings = true) instead seems to be a workaround for my case - the check constraint isn't created with that. But this doesn't really make sense, so it shouldn't be required.

[smint80]

I can confirm this is not only a problem with Postgres. In SQL server I get an exception building the model as it tries to add to check constraints with the same name on the same table.

It does seem that it creates a MinLength constraints both for Required and StringLength with MinimumLength set.

Like this property:

[Required]
[StringLength(30, MinimumLength: 2)]
public string TestString { get; set; } = null!;

Like cremor wrote it can be circumvented by setting AllowEmptyStrings to true.