Validate that no properties on an entity type are mapped to the same …

…column

Fixes #26263

src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs

@@ -1122,10 +1122,14 @@ public static bool IsColumnNullable(this IReadOnlyProperty property, in StoreObj
             return false;
         }
 
-        var sharedTableRootProperty = property.FindSharedStoreObjectRootProperty(storeObject);
-        if (sharedTableRootProperty != null)
+        if (property is not IConventionProperty conventionProperty
+            || conventionProperty.GetIsNullableConfigurationSource() == null)
         {
-            return sharedTableRootProperty.IsColumnNullable(storeObject);
+            var sharedTableRootProperty = property.FindSharedStoreObjectRootProperty(storeObject);
+            if (sharedTableRootProperty != null)
+            {
+                return sharedTableRootProperty.IsColumnNullable(storeObject);
+            }
         }
 
         return property.IsNullable

src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs

@@ -1117,6 +1117,19 @@ protected virtual void ValidateSharedColumnsCompatibility(
                     continue;
                 }
 
+                if (property.DeclaringEntityType.IsAssignableFrom(duplicateProperty.DeclaringEntityType)
+                    || duplicateProperty.DeclaringEntityType.IsAssignableFrom(property.DeclaringEntityType))
+                {
+                    throw new InvalidOperationException(
+                        RelationalStrings.DuplicateColumnNameSameHierarchy(
+                            duplicateProperty.DeclaringEntityType.DisplayName(),
+                            duplicateProperty.Name,
+                            property.DeclaringEntityType.DisplayName(),
+                            property.Name,
+                            columnName,
+                            storeObject.DisplayName()));
+                }
+
                 ValidateCompatible(property, duplicateProperty, columnName, storeObject, logger);
             }
 

src/EFCore.Relational/Metadata/Conventions/TableSharingConcurrencyTokenConvention.cs

@@ -99,7 +99,7 @@ public virtual void ProcessModelFinalizing(
                     continue;
                 }
 
-                RemoveDerivedEntityTypes(entityTypesMissingConcurrencyColumn);
+                RemoveDerivedEntityTypes(entityTypesMissingConcurrencyColumn, mappedTypes);
 
                 foreach (var (conventionEntityType, exampleProperty) in entityTypesMissingConcurrencyColumn)
                 {
@@ -194,10 +194,11 @@ public static bool IsConcurrencyTokenMissing(
         {
             var declaringEntityType = mappedProperty.DeclaringEntityType;
             if (declaringEntityType.IsAssignableFrom(entityType)
+                || entityType.IsAssignableFrom(declaringEntityType)
                 || declaringEntityType.IsInOwnershipPath(entityType)
                 || entityType.IsInOwnershipPath(declaringEntityType))
             {
-                // The concurrency token is on the base type or in the same aggregate
+                // The concurrency token is on the base type, derived type or in the same aggregate
                 propertyMissing = false;
                 continue;
             }
@@ -220,21 +221,31 @@ public static bool IsConcurrencyTokenMissing(
         return propertyMissing;
     }
 
-    private static void RemoveDerivedEntityTypes<T>(Dictionary<IConventionEntityType, T> entityTypeDictionary)
+    private static void RemoveDerivedEntityTypes(
+        Dictionary<IConventionEntityType, IReadOnlyProperty> entityTypeDictionary,
+        List<IConventionEntityType> mappedTypes)
     {
-        foreach (var entityType in entityTypeDictionary.Keys)
+        foreach (var (entityType, property) in entityTypeDictionary)
         {
+            var removed = false;
             var baseType = entityType.BaseType;
             while (baseType != null)
             {
                 if (entityTypeDictionary.ContainsKey(baseType))
                 {
                     entityTypeDictionary.Remove(entityType);
+                    removed = true;
                     break;
                 }
 
                 baseType = baseType.BaseType;
             }
+
+            if (!removed
+                && entityType.IsAssignableFrom(property.DeclaringEntityType))
+            {
+                entityTypeDictionary.Remove(entityType);
+            }
         }
     }
 }

src/EFCore.Relational/Properties/RelationalStrings.resx

@@ -283,6 +283,9 @@
   <data name="DuplicateColumnNameProviderTypeMismatch" xml:space="preserve">
     <value>'{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}', but are configured to use differing provider types ('{type1}' and '{type2}').</value>
   </data>
+  <data name="DuplicateColumnNameSameHierarchy" xml:space="preserve">
+    <value>'{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}', but the properties are contained within the same hierarchy. All properties on an entity type must be mapped to unique different columns.</value>
+  </data>
   <data name="DuplicateColumnNameScaleMismatch" xml:space="preserve">
     <value>'{entityType1}.{property1}' and '{entityType2}.{property2}' are both mapped to column '{columnName}' in '{table}', but are configured with different scales ('{scale1}' and '{scale2}').</value>
   </data>
@@ -1008,9 +1011,6 @@
   <data name="StoredProcedureOutputParameterNotGenerated" xml:space="preserve">
     <value>The property '{entityType}.{property}' is mapped to an output parameter of the stored procedure '{sproc}', but it is not configured as store-generated. Either configure it as store-generated or don't configure the parameter as output.</value>
   </data>
-  <data name="StoredProcedureRowsAffectedNotPopulated" xml:space="preserve">
-    <value>Stored procedure '{sproc}' was configured with a rows affected output parameter or return value, but a valid value was not found when executing the procedure.</value>
-  </data>
   <data name="StoredProcedureOverrideMismatch" xml:space="preserve">
     <value>The property '{propertySpecification}' has specific configuration for the stored procedure '{sproc}', but it isn't mapped to a parameter or a result column on that stored procedure. Remove the specific configuration, or map an entity type that contains this property to '{sproc}'.</value>
   </data>
@@ -1038,6 +1038,9 @@
   <data name="StoredProcedureResultColumnParameterConflict" xml:space="preserve">
     <value>The property '{entityType}.{property}' is mapped to a result column of the stored procedure '{sproc}', but it is also mapped to an output parameter. A store-generated property can only be mapped to one of these.</value>
   </data>
+  <data name="StoredProcedureRowsAffectedNotPopulated" xml:space="preserve">
+    <value>Stored procedure '{sproc}' was configured with a rows affected output parameter or return value, but a valid value was not found when executing the procedure.</value>
+  </data>
   <data name="StoredProcedureRowsAffectedReturnConflictingParameter" xml:space="preserve">
     <value>The stored procedure '{sproc}' cannot be configured to return the rows affected because a rows affected parameter or a rows affected result column for this stored procedure already exists.</value>
   </data>

src/EFCore.SqlServer/Infrastructure/Internal/SqlServerModelValidator.cs

@@ -308,29 +308,11 @@ private static void ValidateTemporalPeriodProperty(IEntityType temporalEntityTyp
                     temporalEntityType.DisplayName(), periodProperty.Name));
         }
 
-        if (temporalEntityType.GetTableName() is string tableName)
+        if (periodProperty.ValueGenerated != ValueGenerated.OnAddOrUpdate)
         {
-            var storeObjectIdentifier = StoreObjectIdentifier.Table(tableName, temporalEntityType.GetSchema());
-            var periodColumnName = periodProperty.GetColumnName(storeObjectIdentifier);
-
-            var propertiesMappedToPeriodColumn = temporalEntityType.GetProperties().Where(
-                p => p.Name != periodProperty.Name && p.GetColumnName(storeObjectIdentifier) == periodColumnName).ToList();
-            foreach (var propertyMappedToPeriodColumn in propertiesMappedToPeriodColumn)
-            {
-                if (propertyMappedToPeriodColumn.ValueGenerated != ValueGenerated.OnAddOrUpdate)
-                {
-                    throw new InvalidOperationException(
-                        SqlServerStrings.TemporalPropertyMappedToPeriodColumnMustBeValueGeneratedOnAddOrUpdate(
-                            temporalEntityType.DisplayName(), propertyMappedToPeriodColumn.Name, nameof(ValueGenerated.OnAddOrUpdate)));
-                }
-
-                if (propertyMappedToPeriodColumn.TryGetDefaultValue(out var _))
-                {
-                    throw new InvalidOperationException(
-                        SqlServerStrings.TemporalPropertyMappedToPeriodColumnCantHaveDefaultValue(
-                            temporalEntityType.DisplayName(), propertyMappedToPeriodColumn.Name));
-                }
-            }
+            throw new InvalidOperationException(
+                SqlServerStrings.TemporalPropertyMappedToPeriodColumnMustBeValueGeneratedOnAddOrUpdate(
+                    temporalEntityType.DisplayName(), periodProperty.Name, nameof(ValueGenerated.OnAddOrUpdate)));
         }
 
         // TODO: check that period property is excluded from query (once the annotation is added)

src/EFCore.SqlServer/Metadata/Builders/TemporalPeriodPropertyBuilder.cs

@@ -9,7 +9,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Builders;
 ///     Instances of this class are returned from methods when using the <see cref="ModelBuilder" /> API
 ///     and it is not designed to be directly constructed in your application code.
 /// </summary>
-public class TemporalPeriodPropertyBuilder
+public class TemporalPeriodPropertyBuilder : IInfrastructure<PropertyBuilder>
 {
     private readonly PropertyBuilder _propertyBuilder;
 
@@ -59,6 +59,9 @@ public virtual TemporalPeriodPropertyBuilder HasPrecision(int precision)
         return this;
     }
 
+    PropertyBuilder IInfrastructure<PropertyBuilder>.Instance
+        => _propertyBuilder;
+
     #region Hidden System.Object members
 
     /// <summary>

src/EFCore.SqlServer/Properties/SqlServerStrings.resx

@@ -332,9 +332,6 @@
   <data name="TemporalPeriodPropertyMustBeNonNullableDateTime" xml:space="preserve">
     <value>Period property '{entityType}.{propertyName}' must be non-nullable and of type '{dateTimeType}'.</value>
   </data>
-  <data name="TemporalPropertyMappedToPeriodColumnCantHaveDefaultValue" xml:space="preserve">
-    <value>Property '{entityType}.{propertyName}' is mapped to the period column and can't have default value specified.</value>
-  </data>
   <data name="TemporalPropertyMappedToPeriodColumnMustBeValueGeneratedOnAddOrUpdate" xml:space="preserve">
     <value>Property '{entityType}.{propertyName}' is mapped to the period column and must have ValueGenerated set to '{valueGeneratedValue}'.</value>
   </data>

test/EFCore.Relational.Tests/Infrastructure/RelationalModelValidatorTest.cs

@@ -478,6 +478,20 @@ public virtual void Detects_incompatible_shared_columns_in_shared_table_with_dif
             modelBuilder);
     }
 
+    [ConditionalFact]
+    public virtual void Detects_properties_mapped_to_the_same_column_within_hierarchy()
+    {
+        var modelBuilder = CreateConventionModelBuilder();
+
+        modelBuilder.Entity<A>().Property(a => a.P0).HasColumnName(nameof(A.P0));
+        modelBuilder.Entity<C>().Property<int?>("PC").HasColumnName(nameof(A.P0));
+
+        VerifyError(
+            RelationalStrings.DuplicateColumnNameSameHierarchy(
+                nameof(A), nameof(A.P0), nameof(C), "PC", nameof(A.P0), nameof(A)),
+            modelBuilder);
+    }
+
     [ConditionalFact]
     public virtual void Detects_incompatible_shared_columns_in_shared_table_with_different_provider_types()
     {
@@ -869,35 +883,18 @@ public virtual void Detects_unmapped_foreign_keys_in_entity_splitting()
             LogLevel.Error);
     }
 
-
-
-    [ConditionalFact]
-    public virtual void Detects_duplicate_column_names()
-    {
-        var modelBuilder = CreateConventionModelBuilder();
-
-        modelBuilder.Entity<Animal>().Property(b => b.Id).HasColumnName("Name");
-        modelBuilder.Entity<Animal>().Property(d => d.Name).HasColumnName("Name").IsRequired();
-
-        VerifyError(
-            RelationalStrings.DuplicateColumnNameDataTypeMismatch(
-                nameof(Animal), nameof(Animal.Id),
-                nameof(Animal), nameof(Animal.Name), "Name", nameof(Animal), "default_int_mapping", "just_string(max)"),
-            modelBuilder);
-    }
-
     [ConditionalFact]
     public virtual void Detects_duplicate_columns_in_derived_types_with_different_types()
     {
         var modelBuilder = CreateConventionModelBuilder();
         modelBuilder.Entity<Animal>();
 
-        modelBuilder.Entity<Cat>().Property(c => c.Type).HasColumnName("Type").IsRequired();
-        modelBuilder.Entity<Dog>().Property(d => d.Type).HasColumnName("Type");
+        modelBuilder.Entity<Cat>().Property(c => c.Type).HasColumnName("Type").HasColumnType("someInt");
+        modelBuilder.Entity<Dog>().Property(d => d.Type).HasColumnName("Type").HasColumnType("default_int_mapping");
 
         VerifyError(
             RelationalStrings.DuplicateColumnNameDataTypeMismatch(
-                nameof(Cat), nameof(Cat.Type), nameof(Dog), nameof(Dog.Type), nameof(Cat.Type), nameof(Animal), "just_string(max)",
+                nameof(Cat), nameof(Cat.Type), nameof(Dog), nameof(Dog.Type), nameof(Cat.Type), nameof(Animal), "someInt",
                 "default_int_mapping"), modelBuilder);
     }
 
@@ -1018,16 +1015,17 @@ public virtual void Detects_duplicate_column_names_within_hierarchy_with_differe
     }
 
     [ConditionalFact]
-    public virtual void Detects_duplicate_column_names_within_hierarchy_with_different_nullability()
+    public virtual void Detects_duplicate_column_names_with_different_column_nullability()
     {
         var modelBuilder = CreateConventionModelBuilder();
-        modelBuilder.Entity<Animal>();
-        modelBuilder.Entity<Cat>();
-        modelBuilder.Entity<Dog>().Property<int?>("OtherId").HasColumnName("Id");
+
+        modelBuilder.Entity<A>().HasOne<B>().WithOne(b => b.A).HasForeignKey<A>(a => a.Id).HasPrincipalKey<B>(b => b.Id).IsRequired();
+        modelBuilder.Entity<A>().ToTable("Table").Property(a => a.P0).HasColumnName(nameof(A.P0)).IsRequired(false);
+        modelBuilder.Entity<B>().ToTable("Table").Property(b => b.P0).HasColumnName(nameof(A.P0)).IsRequired();
 
         VerifyError(
             RelationalStrings.DuplicateColumnNameNullabilityMismatch(
-                nameof(Animal), nameof(Animal.Id), nameof(Dog), "OtherId", nameof(Animal.Id), nameof(Animal)),
+                nameof(A), nameof(A.P0), nameof(B), nameof(A.P0), nameof(A.P0), "Table"),
             modelBuilder);
     }
 
@@ -1686,7 +1684,7 @@ private class Address
     }
 
     [ConditionalFact]
-    public virtual void Detects_missing_concurrency_token_on_the_base_type_without_convention()
+    public virtual void Passes_with_missing_concurrency_token_on_the_base_type_without_convention()
     {
         var modelBuilder = CreateModelBuilderWithoutConvention<TableSharingConcurrencyTokenConvention>();
         modelBuilder.Entity<Person>().ToTable(nameof(Animal))
@@ -1695,9 +1693,7 @@ public virtual void Detects_missing_concurrency_token_on_the_base_type_without_c
         modelBuilder.Entity<Cat>()
             .Property<byte[]>("Version").IsRowVersion().HasColumnName("Version");
 
-        VerifyError(
-            RelationalStrings.MissingConcurrencyColumn(nameof(Animal), "Version", nameof(Animal)),
-            modelBuilder);
+        Validate(modelBuilder);
     }
 
     [ConditionalFact]
@@ -1723,7 +1719,7 @@ public virtual void Passes_with_missing_concurrency_token_property_on_the_base_t
         modelBuilder.Entity<Cat>()
             .Property<byte[]>("Version").IsRowVersion().HasColumnName("Version");
 
-        var model = Validate(modelBuilder);
+        Validate(modelBuilder);
     }
 
     [ConditionalFact]