Fix to #33004 - Unfulfillable nullable contraints are set for complex types with TPH entities (#33054)

When figuring out nullability of columns representing a given property, if property is declared on derived entity in TPH, we make that column nullable. For complex type properties we should be doing the same (and we did), but declaring type of that property is the complex type itself. Instead we should look at the ContainingEntityType rather than just DeclaringType.

Author: maumar

src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs

@@ -20,6 +20,9 @@ public static class RelationalPropertyExtensions
     private static readonly bool UseOldBehavior32763 =
         AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue32763", out var enabled32763) && enabled32763;
 
+    private static readonly bool UseOldBehavior33004 =
+            AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue33004", out var enabled33004) && enabled33004;
+
     private static readonly MethodInfo GetFieldValueMethod =
         typeof(DbDataReader).GetRuntimeMethod(nameof(DbDataReader.GetFieldValue), new[] { typeof(int) })!;
 
@@ -1191,8 +1194,12 @@ public static bool IsColumnNullable(this IReadOnlyProperty property, in StoreObj
             return sharedTableRootProperty.IsColumnNullable(storeObject);
         }
 
+        var declaringEntityType = UseOldBehavior33004
+            ? property.DeclaringType
+            : property.DeclaringType.ContainingEntityType;
+
         return property.IsNullable
-            || (property.DeclaringType is IReadOnlyEntityType entityType
+            || (declaringEntityType is IReadOnlyEntityType entityType
                 && ((entityType.BaseType != null
                         && entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy)
                     || IsOptionalSharingDependent(entityType, storeObject, 0)));

test/EFCore.Relational.Specification.Tests/Migrations/MigrationsTestBase.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.ComponentModel.DataAnnotations;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
 using Microsoft.EntityFrameworkCore.Migrations.Internal;
 using Microsoft.EntityFrameworkCore.Scaffolding.Metadata;
@@ -2141,6 +2142,72 @@ public virtual Task Create_table_with_optional_primitive_collection()
                     Assert.Single(customersTable.PrimaryKey!.Columns));
             });
 
+    [ConditionalFact]
+    public virtual Task Create_table_with_complex_type_with_required_properties_on_derived_entity_in_TPH()
+        => Test(
+            builder => { },
+            builder =>
+            {
+                builder.Entity(
+                    "Contact", e =>
+                    {
+                        e.Property<int>("Id").ValueGeneratedOnAdd();
+                        e.HasKey("Id");
+                        e.Property<string>("Name");
+                        e.ToTable("Contacts");
+                    });
+                builder.Entity(
+                    "Supplier", e =>
+                    {
+                        e.HasBaseType("Contact");
+                        e.Property<int>("Number");
+                        e.ComplexProperty<MyComplex>("MyComplex", ct =>
+                        {
+                            ct.ComplexProperty<MyNestedComplex>("MyNestedComplex").IsRequired();
+                        });
+                    });
+            },
+            model =>
+            {
+                var contactsTable = Assert.Single(model.Tables.Where(t => t.Name == "Contacts"));
+                Assert.Collection(
+                    contactsTable.Columns,
+                    c => Assert.Equal("Id", c.Name),
+                    c => Assert.Equal("Discriminator", c.Name),
+                    c => Assert.Equal("Name", c.Name),
+                    c => Assert.Equal("Number", c.Name),
+                    c =>
+                    {
+                        Assert.Equal("MyComplex_Prop", c.Name);
+                        Assert.Equal(true, c.IsNullable);
+                    },
+                    c =>
+                    {
+                        Assert.Equal("MyComplex_MyNestedComplex_Bar", c.Name);
+                        Assert.Equal(true, c.IsNullable);
+                    },
+                    c =>
+                    {
+                        Assert.Equal("MyComplex_MyNestedComplex_Foo", c.Name);
+                        Assert.Equal(true, c.IsNullable);
+                    });
+            });
+
+    protected class MyComplex
+    {
+        [Required]
+        public string Prop { get; set; }
+
+        [Required]
+        public MyNestedComplex Nested { get; set; }
+    }
+
+    public class MyNestedComplex
+    {
+        public int Foo { get; set; }
+        public DateTime Bar { get; set; }
+    }
+
     protected class Person
     {
         public int Id { get; set; }

test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs

@@ -9328,6 +9328,25 @@ CONSTRAINT [PK_Customers] PRIMARY KEY ([Id])
 """);
     }
 
+    public override async Task Create_table_with_complex_type_with_required_properties_on_derived_entity_in_TPH()
+    {
+        await base.Create_table_with_complex_type_with_required_properties_on_derived_entity_in_TPH();
+
+        AssertSql(
+"""
+CREATE TABLE [Contacts] (
+    [Id] int NOT NULL IDENTITY,
+    [Discriminator] nvarchar(8) NOT NULL,
+    [Name] nvarchar(max) NULL,
+    [Number] int NULL,
+    [MyComplex_Prop] nvarchar(max) NULL,
+    [MyComplex_MyNestedComplex_Bar] datetime2 NULL,
+    [MyComplex_MyNestedComplex_Foo] int NULL,
+    CONSTRAINT [PK_Contacts] PRIMARY KEY ([Id])
+);
+""");
+    }
+
     protected override string NonDefaultCollation
         => _nonDefaultCollation ??= GetDatabaseCollation() == "German_PhoneBook_CI_AS"
             ? "French_CI_AS"

test/EFCore.Sqlite.FunctionalTests/Migrations/MigrationsSqliteTest.cs

@@ -1702,6 +1702,24 @@ await Test(
 """);
     }
 
+    public override async Task Create_table_with_complex_type_with_required_properties_on_derived_entity_in_TPH()
+    {
+        await base.Create_table_with_complex_type_with_required_properties_on_derived_entity_in_TPH();
+
+        AssertSql(
+"""
+CREATE TABLE "Contacts" (
+    "Id" INTEGER NOT NULL CONSTRAINT "PK_Contacts" PRIMARY KEY AUTOINCREMENT,
+    "Discriminator" TEXT NOT NULL,
+    "Name" TEXT NULL,
+    "Number" INTEGER NULL,
+    "MyComplex_Prop" TEXT NULL,
+    "MyComplex_MyNestedComplex_Bar" TEXT NULL,
+    "MyComplex_MyNestedComplex_Foo" INTEGER NULL
+);
+""");
+    }
+
     public override Task Create_sequence()
         => AssertNotSupportedAsync(base.Create_sequence, SqliteStrings.SequencesNotSupported);