Skip to content

Add support for optional complex types to model building #35614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,7 @@ public virtual void Generate(IStoredProcedureMapping sprocMapping, CSharpRuntime
private void GenerateAddMapping(
ITableMappingBase tableMapping,
string tableVariable,
string entityTypeVariable,
string structuralTypeVariable,
string tableMappingsVariable,
string tableMappingVariable,
string mappingType,
Expand All @@ -1510,7 +1510,7 @@ private void GenerateAddMapping(
var typeBase = tableMapping.TypeBase;

mainBuilder
.Append($"var {tableMappingVariable} = new {mappingType}({entityTypeVariable}, ")
.Append($"var {tableMappingVariable} = new {mappingType}({structuralTypeVariable}, ")
.Append($"{tableVariable}, {additionalParameter ?? ""}{code.Literal(tableMapping.IncludesDerivedTypes)}");

if (tableMapping.IsSharedTablePrincipal.HasValue
Expand Down Expand Up @@ -1549,7 +1549,7 @@ private void GenerateAddMapping(
foreach (var internalForeignKey in table.GetRowInternalForeignKeys(entityType))
{
mainBuilder
.Append(tableVariable).Append($".AddRowInternalForeignKey({entityTypeVariable}, ")
.Append(tableVariable).Append($".AddRowInternalForeignKey({structuralTypeVariable}, ")
.AppendLine("RelationalModel.GetForeignKey(this,").IncrementIndent()
.AppendLine($"{code.Literal(internalForeignKey.DeclaringEntityType.Name)},")
.AppendLine($"{code.Literal(internalForeignKey.Properties.Select(p => p.Name).ToArray())},")
Expand Down
115 changes: 74 additions & 41 deletions src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs
Original file line number Diff line number Diff line change
@@ -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.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Runtime.CompilerServices;
using System.Text;
Expand Down Expand Up @@ -230,21 +231,54 @@ public static string GetDefaultColumnName(this IReadOnlyProperty property)
StringBuilder? builder = null;
var currentStoreObject = storeObject;
if (property.DeclaringType is IReadOnlyEntityType entityType)
{
builder = CreateOwnershipPrefix(entityType, storeObject, builder);
}
else if (StoreObjectIdentifier.Create(property.DeclaringType, currentStoreObject.StoreObjectType) == currentStoreObject
|| property.DeclaringType.GetMappingFragments(storeObject.StoreObjectType)
.Any(f => f.StoreObject == currentStoreObject))
{
builder = CreateComplexPrefix((IReadOnlyComplexType)property.DeclaringType, storeObject, builder);
}

var baseName = storeObject.StoreObjectType == StoreObjectType.Table ? property.GetDefaultColumnName() : property.Name;
if (builder == null)
{
return baseName;
}

builder.Append(baseName);
baseName = builder.ToString();

return Uniquifier.Truncate(baseName, property.DeclaringType.Model.GetMaxIdentifierLength());

[return: NotNullIfNotNull("builder")]
static StringBuilder? CreateOwnershipPrefix(IReadOnlyEntityType entityType, in StoreObjectIdentifier storeObject, StringBuilder? builder)
{
while (true)
{
var ownership = entityType.GetForeignKeys().SingleOrDefault(fk => fk.IsOwnership);
if (ownership == null)
{
break;
return builder;
}

var ownerType = ownership.PrincipalEntityType;
if (StoreObjectIdentifier.Create(ownerType, currentStoreObject.StoreObjectType) != currentStoreObject
&& ownerType.GetMappingFragments(storeObject.StoreObjectType)
.All(f => f.StoreObject != currentStoreObject))
if (StoreObjectIdentifier.Create(ownerType, storeObject.StoreObjectType) != storeObject)
{
break;
var foundMappedFragment = false;
foreach (var fragment in ownerType.GetMappingFragments(storeObject.StoreObjectType))
{
if (fragment.StoreObject == storeObject)
{
foundMappedFragment = true;
}
}

if (!foundMappedFragment)
{
return builder;
}
}

builder ??= new StringBuilder();
Expand All @@ -254,31 +288,27 @@ public static string GetDefaultColumnName(this IReadOnlyProperty property)
entityType = ownerType;
}
}
else if (StoreObjectIdentifier.Create(property.DeclaringType, currentStoreObject.StoreObjectType) == currentStoreObject
|| property.DeclaringType.GetMappingFragments(storeObject.StoreObjectType)
.Any(f => f.StoreObject == currentStoreObject))

static StringBuilder CreateComplexPrefix(IReadOnlyComplexType complexType, in StoreObjectIdentifier storeObject, StringBuilder? builder)
{
var complexType = (IReadOnlyComplexType)property.DeclaringType;
builder ??= new StringBuilder();
while (complexType != null)
while (true)
{
builder.Insert(0, "_");
builder.Insert(0, complexType.ComplexProperty.Name);

complexType = complexType.ComplexProperty.DeclaringType as IReadOnlyComplexType;
switch (complexType.ComplexProperty.DeclaringType)
{
case IReadOnlyComplexType declaringComplexType:
complexType = declaringComplexType;
break;
case IReadOnlyEntityType declaringEntityType:
return CreateOwnershipPrefix(declaringEntityType, storeObject, builder);
default:
return builder;
}
}
}

var baseName = storeObject.StoreObjectType == StoreObjectType.Table ? property.GetDefaultColumnName() : property.Name;
if (builder == null)
{
return baseName;
}

builder.Append(baseName);
baseName = builder.ToString();

return Uniquifier.Truncate(baseName, property.DeclaringType.Model.GetMaxIdentifierLength());
}

/// <summary>
Expand Down Expand Up @@ -1183,15 +1213,21 @@ public static void SetIsFixedLength(this IMutableProperty property, bool? fixedL
/// <remarks>
/// This depends on the property itself and also how it is mapped. For example,
/// derived non-nullable properties in a TPH type hierarchy will be mapped to nullable columns.
/// As well as properties on optional types sharing the same table.
/// </remarks>
/// <param name="property">The <see cref="IReadOnlyProperty" />.</param>
/// <returns><see langword="true" /> if the mapped column is nullable; <see langword="false" /> otherwise.</returns>
public static bool IsColumnNullable(this IReadOnlyProperty property)
=> property.IsNullable
|| (property.DeclaringType.ContainingEntityType is IReadOnlyEntityType entityType
&& entityType.BaseType != null
&& entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy);
&& entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy)
|| (property.DeclaringType is IReadOnlyComplexType complexType
&& IsNullable(complexType.ComplexProperty));

private static bool IsNullable(IReadOnlyComplexProperty complexProperty)
=> complexProperty.IsNullable
|| (complexProperty.DeclaringType is IReadOnlyComplexType complexType
&& IsNullable(complexType.ComplexProperty));

/// <summary>
/// Checks whether the column mapped to the given property will be nullable
Expand Down Expand Up @@ -1222,7 +1258,9 @@ public static bool IsColumnNullable(this IReadOnlyProperty property, in StoreObj
|| (property.DeclaringType.ContainingEntityType is IReadOnlyEntityType entityType
&& ((entityType.BaseType != null
&& entityType.GetMappingStrategy() == RelationalAnnotationNames.TphMappingStrategy)
|| IsOptionalSharingDependent(entityType, storeObject, 0)));
|| IsOptionalSharingDependent(entityType, storeObject, 0)))
|| (property.DeclaringType is IReadOnlyComplexType complexType
&& IsNullable(complexType.ComplexProperty));
}

private static bool IsOptionalSharingDependent(
Expand Down Expand Up @@ -1476,7 +1514,7 @@ public static RelationalTypeMapping GetRelationalTypeMapping(this IReadOnlyPrope
if (property.DeclaringType.IsMappedToJson())
{
//JSON-splitting is not supported
//issue #28574
//Issue #28574
return null;
}

Expand All @@ -1494,20 +1532,15 @@ public static RelationalTypeMapping GetRelationalTypeMapping(this IReadOnlyPrope
// Using a hashset is detrimental to the perf when there are no cycles
for (var i = 0; i < Metadata.Internal.RelationalEntityTypeExtensions.MaxEntityTypesSharingTable; i++)
{
var entityType = rootProperty.DeclaringType as IReadOnlyEntityType;
if (entityType == null)
{
break;
}

var entityType = rootProperty.DeclaringType.ContainingEntityType;
IReadOnlyProperty? linkedProperty = null;
foreach (var p in entityType
foreach (var principalProperty in entityType
.FindRowInternalForeignKeys(storeObject)
.SelectMany(fk => fk.PrincipalEntityType.GetProperties()))
.SelectMany(static fk => fk.PrincipalEntityType.GetProperties()))
{
if (p.GetColumnName(storeObject) == column)
if (principalProperty.GetColumnName(storeObject) == column)
{
linkedProperty = p;
linkedProperty = principalProperty;
break;
}
}
Expand Down Expand Up @@ -1538,8 +1571,8 @@ public static RelationalTypeMapping GetRelationalTypeMapping(this IReadOnlyPrope
// Using a hashset is detrimental to the perf when there are no cycles
for (var i = 0; i < Metadata.Internal.RelationalEntityTypeExtensions.MaxEntityTypesSharingTable; i++)
{
var entityType = principalProperty.DeclaringType as IReadOnlyEntityType;
var linkingRelationship = entityType?.FindRowInternalForeignKeys(storeObject).FirstOrDefault();
var entityType = principalProperty.DeclaringType.ContainingEntityType;
var linkingRelationship = entityType.FindRowInternalForeignKeys(storeObject).FirstOrDefault();
if (linkingRelationship == null)
{
break;
Expand All @@ -1566,8 +1599,8 @@ public static RelationalTypeMapping GetRelationalTypeMapping(this IReadOnlyPrope
// Using a hashset is detrimental to the perf when there are no cycles
for (var i = 0; i < Metadata.Internal.RelationalEntityTypeExtensions.MaxEntityTypesSharingTable; i++)
{
var entityType = principalProperty.DeclaringType as IReadOnlyEntityType;
var linkingRelationship = entityType?.FindRowInternalForeignKeys(storeObject).FirstOrDefault();
var entityType = principalProperty.DeclaringType.ContainingEntityType;
var linkingRelationship = entityType.FindRowInternalForeignKeys(storeObject).FirstOrDefault();
if (linkingRelationship == null)
{
break;
Expand Down Expand Up @@ -1595,7 +1628,7 @@ public static RelationalTypeMapping GetRelationalTypeMapping(this IReadOnlyPrope
/// <param name="property">The property.</param>
/// <returns>The property facet overrides.</returns>
public static IEnumerable<IReadOnlyRelationalPropertyOverrides> GetOverrides(this IReadOnlyProperty property)
=> RelationalPropertyOverrides.Get(property) ?? Enumerable.Empty<IReadOnlyRelationalPropertyOverrides>();
=> RelationalPropertyOverrides.Get(property) ?? [];

/// <summary>
/// <para>
Expand Down
107 changes: 64 additions & 43 deletions src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,22 @@ static void ValidateType(ITypeBase typeBase)
}
}

/// <inheritdoc/>
protected override void ValidatePropertyMapping(IConventionComplexProperty complexProperty)
{
base.ValidatePropertyMapping(complexProperty);

var typeBase = complexProperty.DeclaringType;

if (!typeBase.IsMappedToJson()
&& complexProperty.IsNullable
&& complexProperty.ComplexType.GetProperties().All(m => m.IsNullable))
{
throw new InvalidOperationException(
RelationalStrings.ComplexPropertyOptionalTableSharing(typeBase.DisplayName(), complexProperty.Name));
}
}

/// <summary>
/// Validates the mapping/configuration of SQL queries in the model.
/// </summary>
Expand Down Expand Up @@ -1239,16 +1255,56 @@ protected virtual void ValidateSharedColumnsCompatibility(
if (missingConcurrencyTokens != null)
{
missingConcurrencyTokens.Clear();
foreach (var (key, readOnlyProperties) in concurrencyColumns!)
foreach (var (concurrencyColumn, concurrencyProperties) in concurrencyColumns!)
{
if (TableSharingConcurrencyTokenConvention.IsConcurrencyTokenMissing(readOnlyProperties, entityType, mappedTypes))
if (TableSharingConcurrencyTokenConvention.IsConcurrencyTokenMissing(concurrencyProperties, entityType, mappedTypes))
{
missingConcurrencyTokens.Add(key);
missingConcurrencyTokens.Add(concurrencyColumn);
}
}
}

foreach (var property in entityType.GetDeclaredProperties())
ValidateCompatible(entityType, storeObject, propertyMappings, missingConcurrencyTokens, logger);

if (missingConcurrencyTokens != null)
{
foreach (var concurrencyColumn in missingConcurrencyTokens)
{
throw new InvalidOperationException(
RelationalStrings.MissingConcurrencyColumn(
entityType.DisplayName(), concurrencyColumn, storeObject.DisplayName()));
}
}
}

var columnOrders = new Dictionary<int, List<string>>();
foreach (var property in propertyMappings.Values)
{
var columnOrder = property.GetColumnOrder(storeObject);
if (!columnOrder.HasValue)
{
continue;
}

var columns = columnOrders.GetOrAddNew(columnOrder.Value);
columns.Add(property.GetColumnName(storeObject)!);
}

if (columnOrders.Any(g => g.Value.Count > 1))
{
logger.DuplicateColumnOrders(
storeObject,
columnOrders.Where(g => g.Value.Count > 1).SelectMany(g => g.Value).ToList());
}

void ValidateCompatible(
ITypeBase structuralType,
in StoreObjectIdentifier storeObject,
Dictionary<string, IProperty> propertyMappings,
HashSet<string>? missingConcurrencyTokens,
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
{
foreach (var property in structuralType.GetDeclaredProperties())
{
var columnName = property.GetColumnName(storeObject);
if (columnName == null)
Expand Down Expand Up @@ -1276,39 +1332,14 @@ protected virtual void ValidateSharedColumnsCompatibility(
storeObject.DisplayName()));
}

ValidateCompatible(property, duplicateProperty, columnName, storeObject, logger);
this.ValidateCompatible(property, duplicateProperty, columnName, storeObject, logger);
}

if (missingConcurrencyTokens != null)
foreach (var complexProperty in structuralType.GetDeclaredComplexProperties())
{
foreach (var missingColumn in missingConcurrencyTokens)
{
throw new InvalidOperationException(
RelationalStrings.MissingConcurrencyColumn(
entityType.DisplayName(), missingColumn, storeObject.DisplayName()));
}
ValidateCompatible(complexProperty.ComplexType, storeObject, propertyMappings, missingConcurrencyTokens, logger);
}
}

var columnOrders = new Dictionary<int, List<string>>();
foreach (var property in propertyMappings.Values)
{
var columnOrder = property.GetColumnOrder(storeObject);
if (!columnOrder.HasValue)
{
continue;
}

var columns = columnOrders.GetOrAddNew(columnOrder.Value);
columns.Add(property.GetColumnName(storeObject)!);
}

if (columnOrders.Any(g => g.Value.Count > 1))
{
logger.DuplicateColumnOrders(
storeObject,
columnOrders.Where(g => g.Value.Count > 1).SelectMany(g => g.Value).ToList());
}
}

/// <summary>
Expand All @@ -1326,17 +1357,7 @@ protected virtual void ValidateCompatible(
in StoreObjectIdentifier storeObject,
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
{
if (property.IsColumnNullable(storeObject) != duplicateProperty.IsColumnNullable(storeObject))
{
throw new InvalidOperationException(
RelationalStrings.DuplicateColumnNameNullabilityMismatch(
duplicateProperty.DeclaringType.DisplayName(),
duplicateProperty.Name,
property.DeclaringType.DisplayName(),
property.Name,
columnName,
storeObject.DisplayName()));
}
// NB: Properties can have different nullability, the resulting column will be non-nullable if any of the properties is non-nullable

var currentMaxLength = property.GetMaxLength(storeObject);
var previousMaxLength = duplicateProperty.GetMaxLength(storeObject);
Expand Down
Loading