Skip to content

Commit fbe4a9d

Browse files
CopilotAndriySvyryd
authored andcommitted
Fix duplicate JSON column in TPT child tables for complex types declared on base entity type
When a complex property is mapped to JSON on a base entity type in a TPT hierarchy, the JSON column was being incorrectly created in child tables in addition to the base table. This is because `GetComplexProperties()` returns inherited properties, and there was no equivalent of the `GetColumnName(mappedTable) == null` filter used for scalar properties. The fix adds a check in `CreateTableMapping` to skip complex properties declared on base entity types that map to a different table (TPT scenario). TPC is excluded from this check since each TPC concrete table needs all properties including inherited ones. Fixes #38535
1 parent 9f11437 commit fbe4a9d

6 files changed

Lines changed: 286 additions & 5 deletions

File tree

src/EFCore.Relational/EFCore.Relational.baseline.json

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7352,10 +7352,10 @@
73527352
"Member": "virtual Microsoft.EntityFrameworkCore.Query.JsonQueryExpression BindStructuralProperty(Microsoft.EntityFrameworkCore.Metadata.IPropertyBase structuralProperty);"
73537353
},
73547354
{
7355-
"Member": "virtual Microsoft.EntityFrameworkCore.Metadata.IRelationalJsonElement? FindJsonElement(Microsoft.EntityFrameworkCore.Metadata.IPropertyBase propertyBase);"
7355+
"Member": "override bool Equals(object? obj);"
73567356
},
73577357
{
7358-
"Member": "override bool Equals(object? obj);"
7358+
"Member": "virtual Microsoft.EntityFrameworkCore.Metadata.IRelationalJsonElement? FindJsonElement(Microsoft.EntityFrameworkCore.Metadata.IPropertyBase propertyBase);"
73597359
},
73607360
{
73617361
"Member": "override int GetHashCode();"
@@ -17425,6 +17425,9 @@
1742517425
{
1742617426
"Member": "static string? GetContainerColumnName(this Microsoft.EntityFrameworkCore.Metadata.IReadOnlyTypeBase typeBase);"
1742717427
},
17428+
{
17429+
"Member": "static string? GetContainerColumnName(this Microsoft.EntityFrameworkCore.Metadata.IReadOnlyTypeBase typeBase, in Microsoft.EntityFrameworkCore.Metadata.StoreObjectIdentifier storeObject);"
17430+
},
1742817431
{
1742917432
"Member": "static Microsoft.EntityFrameworkCore.Metadata.ConfigurationSource? GetContainerColumnNameConfigurationSource(this Microsoft.EntityFrameworkCore.Metadata.IConventionTypeBase typeBase);"
1743017433
},

src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,10 @@ public static string GetDefaultColumnName(this IReadOnlyProperty property)
230230
}
231231
else if (StoreObjectIdentifier.Create(property.DeclaringType, currentStoreObject.StoreObjectType) == currentStoreObject
232232
|| property.DeclaringType.GetMappingFragments(storeObject.StoreObjectType)
233-
.Any(f => f.StoreObject == currentStoreObject))
233+
.Any(f => f.StoreObject == currentStoreObject)
234+
|| (property.IsPrimaryKey()
235+
&& property.DeclaringType.ContainingEntityType.GetDerivedTypes()
236+
.Any(e => StoreObjectIdentifier.Create(e, currentStoreObject.StoreObjectType) == currentStoreObject)))
234237
{
235238
builder = CreateComplexPrefix((IReadOnlyComplexType)property.DeclaringType, storeObject, builder);
236239
}

src/EFCore.Relational/Extensions/RelationalTypeBaseExtensions.cs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,74 @@ public static bool IsMappedToJson(this IReadOnlyTypeBase typeBase)
388388
: ((IReadOnlyComplexType)typeBase).ComplexProperty.DeclaringType.GetContainerColumnName();
389389
}
390390

391+
/// <summary>
392+
/// Gets the container column name to which the type is mapped for a particular table-like store object.
393+
/// </summary>
394+
/// <param name="typeBase">The type to get the container column name for.</param>
395+
/// <param name="storeObject">The identifier of the table-like store object containing the column.</param>
396+
/// <returns>
397+
/// The container column name to which the type is mapped, or <see langword="null" /> if the type is not mapped
398+
/// to a container column in the given store object.
399+
/// </returns>
400+
public static string? GetContainerColumnName(this IReadOnlyTypeBase typeBase, in StoreObjectIdentifier storeObject)
401+
{
402+
var annotation = typeBase.FindAnnotation(RelationalAnnotationNames.ContainerColumnName);
403+
if (annotation != null)
404+
{
405+
var containerColumnName = (string?)annotation.Value;
406+
if (string.IsNullOrEmpty(containerColumnName))
407+
{
408+
return containerColumnName;
409+
}
410+
411+
if (storeObject.StoreObjectType is StoreObjectType.Function or StoreObjectType.SqlQuery)
412+
{
413+
return containerColumnName;
414+
}
415+
416+
var containingEntityType = typeBase.ContainingEntityType;
417+
if (containingEntityType.GetMappingStrategy() == RelationalAnnotationNames.TpcMappingStrategy)
418+
{
419+
return containerColumnName;
420+
}
421+
422+
// TODO: Support entity splitting with JSON columns. Issue #36172
423+
var declaringStoreObject = StoreObjectIdentifier.Create(typeBase, storeObject.StoreObjectType);
424+
if (declaringStoreObject == null)
425+
{
426+
var tableFound = false;
427+
var queue = new Queue<IReadOnlyEntityType>();
428+
queue.Enqueue(containingEntityType);
429+
while (queue.Count > 0 && !tableFound)
430+
{
431+
foreach (var derivedType in queue.Dequeue().GetDirectlyDerivedTypes())
432+
{
433+
var derivedStoreObject = StoreObjectIdentifier.Create(derivedType, storeObject.StoreObjectType);
434+
if (derivedStoreObject == null)
435+
{
436+
queue.Enqueue(derivedType);
437+
continue;
438+
}
439+
440+
if (derivedStoreObject == storeObject)
441+
{
442+
tableFound = true;
443+
break;
444+
}
445+
}
446+
}
447+
448+
return tableFound ? containerColumnName : null;
449+
}
450+
451+
return declaringStoreObject == storeObject ? containerColumnName : null;
452+
}
453+
454+
return typeBase is IReadOnlyEntityType entityType
455+
? entityType.FindOwnership()?.PrincipalEntityType.GetContainerColumnName(storeObject)
456+
: ((IReadOnlyComplexType)typeBase).ComplexProperty.DeclaringType.GetContainerColumnName(storeObject);
457+
}
458+
391459
/// <summary>
392460
/// Sets the name of the container column to which the type is mapped.
393461
/// </summary>

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ public virtual void ProcessModelFinalizing(
8888
{
8989
var pk = entityType.FindPrimaryKey();
9090
if (pk != null
91+
&& pk.Properties.All(p => p.DeclaringType is IConventionEntityType)
9192
&& !entityType.FindDeclaredForeignKeys(pk.Properties)
9293
.Any(fk => fk.PrincipalKey.IsPrimaryKey()
9394
&& fk.PrincipalEntityType.IsAssignableFrom(entityType)

src/EFCore.Relational/Metadata/Internal/RelationalModel.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ private static void CreateTableMapping(
509509
IsSplitEntityTypePrincipal = isSplitEntityTypePrincipal
510510
};
511511

512-
var containerColumnName = mappedType.GetContainerColumnName();
512+
var containerColumnName = mappedType.GetContainerColumnName(mappedTable);
513513
var containerColumnType = mappedType.GetContainerColumnType();
514514
if (!string.IsNullOrEmpty(containerColumnName))
515515
{
@@ -1028,7 +1028,7 @@ private static void CreateViewMapping(
10281028
IsSplitEntityTypePrincipal = isSplitEntityTypePrincipal
10291029
};
10301030

1031-
var containerColumnName = mappedType.GetContainerColumnName();
1031+
var containerColumnName = mappedType.GetContainerColumnName(mappedView);
10321032
var containerColumnType = mappedType.GetContainerColumnType();
10331033
if (!string.IsNullOrEmpty(containerColumnName))
10341034
{

test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3381,6 +3381,181 @@ public void Complex_property_json_column_is_nullable_in_TPH_hierarchy()
33813381
Assert.IsType<JsonColumn>(jsonColumn);
33823382
}
33833383

3384+
[Fact]
3385+
public void Complex_property_json_column_is_not_duplicated_in_TPT_child_tables()
3386+
{
3387+
var modelBuilder = CreateConventionModelBuilder();
3388+
3389+
modelBuilder.Entity<TptBaseEntityWithComplexProperty>()
3390+
.UseTptMappingStrategy()
3391+
.ComplexProperty(e => e.ComplexProperty, b => b.ToJson());
3392+
modelBuilder.Entity<TptDerivedEntityWithoutComplexProperty>();
3393+
3394+
var model = modelBuilder.FinalizeModel();
3395+
var relationalModel = model.GetRelationalModel();
3396+
3397+
var baseTable = relationalModel.Tables.Single(t => t.Name == nameof(TptBaseEntityWithComplexProperty));
3398+
var childTable = relationalModel.Tables.Single(t => t.Name == nameof(TptDerivedEntityWithoutComplexProperty));
3399+
3400+
// The JSON column for the base complex property must appear only in the base table
3401+
Assert.Contains(baseTable.Columns, c => c.Name == nameof(TptBaseEntityWithComplexProperty.ComplexProperty));
3402+
Assert.DoesNotContain(childTable.Columns, c => c.Name == nameof(TptBaseEntityWithComplexProperty.ComplexProperty));
3403+
}
3404+
3405+
[Fact]
3406+
public void Complex_property_columns_are_not_duplicated_in_TPT_child_tables()
3407+
{
3408+
var modelBuilder = CreateConventionModelBuilder();
3409+
3410+
modelBuilder.Entity<TptBaseEntityWithComplexProperty>()
3411+
.UseTptMappingStrategy()
3412+
.ComplexProperty(e => e.ComplexProperty);
3413+
modelBuilder.Entity<TptDerivedEntityWithoutComplexProperty>();
3414+
3415+
var model = modelBuilder.FinalizeModel();
3416+
var relationalModel = model.GetRelationalModel();
3417+
3418+
var baseTable = relationalModel.Tables.Single(t => t.Name == nameof(TptBaseEntityWithComplexProperty));
3419+
var childTable = relationalModel.Tables.Single(t => t.Name == nameof(TptDerivedEntityWithoutComplexProperty));
3420+
3421+
// Non-JSON complex property columns appear only on the base table.
3422+
var valueColumnName = nameof(TptBaseEntityWithComplexProperty.ComplexProperty) + "_" + nameof(ComplexData.Value);
3423+
var numberColumnName = nameof(TptBaseEntityWithComplexProperty.ComplexProperty) + "_" + nameof(ComplexData.Number);
3424+
Assert.Contains(baseTable.Columns, c => c.Name == valueColumnName);
3425+
Assert.Contains(baseTable.Columns, c => c.Name == numberColumnName);
3426+
Assert.DoesNotContain(childTable.Columns, c => c.Name == valueColumnName);
3427+
Assert.DoesNotContain(childTable.Columns, c => c.Name == numberColumnName);
3428+
}
3429+
3430+
[Fact]
3431+
public void Complex_property_json_column_is_created_in_every_TPC_table()
3432+
{
3433+
var modelBuilder = CreateConventionModelBuilder();
3434+
3435+
modelBuilder.Entity<TpcBaseEntityWithComplexProperty>(b =>
3436+
{
3437+
b.UseTpcMappingStrategy();
3438+
b.ComplexProperty(e => e.ComplexProperty, cb => cb.ToJson());
3439+
});
3440+
modelBuilder.Entity<TpcDerivedEntityWithoutComplexProperty>();
3441+
3442+
var model = modelBuilder.FinalizeModel();
3443+
var relationalModel = model.GetRelationalModel();
3444+
3445+
var baseTable = relationalModel.Tables.Single(t => t.Name == nameof(TpcBaseEntityWithComplexProperty));
3446+
var derivedTable = relationalModel.Tables.Single(t => t.Name == nameof(TpcDerivedEntityWithoutComplexProperty));
3447+
3448+
// In TPC the JSON container column appears on every concrete table.
3449+
Assert.Contains(baseTable.Columns, c => c.Name == nameof(TpcBaseEntityWithComplexProperty.ComplexProperty));
3450+
Assert.Contains(derivedTable.Columns, c => c.Name == nameof(TpcBaseEntityWithComplexProperty.ComplexProperty));
3451+
}
3452+
3453+
[Fact]
3454+
public void GetContainerColumnName_with_StoreObjectIdentifier_resolves_per_table()
3455+
{
3456+
var modelBuilder = CreateConventionModelBuilder();
3457+
3458+
modelBuilder.Entity<TptBaseEntityWithComplexProperty>()
3459+
.UseTptMappingStrategy()
3460+
.ComplexProperty(e => e.ComplexProperty, cb => cb.ToJson());
3461+
modelBuilder.Entity<TptDerivedEntityWithoutComplexProperty>();
3462+
3463+
var model = modelBuilder.FinalizeModel();
3464+
var baseEntity = model.FindEntityType(typeof(TptBaseEntityWithComplexProperty))!;
3465+
var complexProperty = baseEntity.FindComplexProperty(nameof(TptBaseEntityWithComplexProperty.ComplexProperty))!;
3466+
var complexType = complexProperty.ComplexType;
3467+
3468+
var baseTable = StoreObjectIdentifier.Table(nameof(TptBaseEntityWithComplexProperty));
3469+
var childTable = StoreObjectIdentifier.Table(nameof(TptDerivedEntityWithoutComplexProperty));
3470+
var unrelatedTable = StoreObjectIdentifier.Table("SomeOtherTable");
3471+
3472+
Assert.Equal(nameof(TptBaseEntityWithComplexProperty.ComplexProperty), complexType.GetContainerColumnName(baseTable));
3473+
Assert.Null(complexType.GetContainerColumnName(childTable));
3474+
Assert.Null(complexType.GetContainerColumnName(unrelatedTable));
3475+
}
3476+
3477+
[Fact]
3478+
public void GetContainerColumnName_with_StoreObjectIdentifier_returns_column_for_every_TPC_table()
3479+
{
3480+
var modelBuilder = CreateConventionModelBuilder();
3481+
3482+
modelBuilder.Entity<TpcBaseEntityWithComplexProperty>(b =>
3483+
{
3484+
b.UseTpcMappingStrategy();
3485+
b.ComplexProperty(e => e.ComplexProperty, cb => cb.ToJson());
3486+
});
3487+
modelBuilder.Entity<TpcDerivedEntityWithoutComplexProperty>();
3488+
3489+
var model = modelBuilder.FinalizeModel();
3490+
var baseEntity = model.FindEntityType(typeof(TpcBaseEntityWithComplexProperty))!;
3491+
var complexProperty = baseEntity.FindComplexProperty(nameof(TpcBaseEntityWithComplexProperty.ComplexProperty))!;
3492+
var complexType = complexProperty.ComplexType;
3493+
3494+
var baseTable = StoreObjectIdentifier.Table(nameof(TpcBaseEntityWithComplexProperty));
3495+
var derivedTable = StoreObjectIdentifier.Table(nameof(TpcDerivedEntityWithoutComplexProperty));
3496+
3497+
Assert.Equal(nameof(TpcBaseEntityWithComplexProperty.ComplexProperty), complexType.GetContainerColumnName(baseTable));
3498+
Assert.Equal(nameof(TpcBaseEntityWithComplexProperty.ComplexProperty), complexType.GetContainerColumnName(derivedTable));
3499+
}
3500+
3501+
[Fact]
3502+
public void Complex_type_with_PK_property_creates_column_mapping_in_TPT_child_table()
3503+
{
3504+
var modelBuilder = CreateConventionModelBuilder();
3505+
3506+
modelBuilder.Entity<TptBaseWithComplexTypePK>(b =>
3507+
{
3508+
b.UseTptMappingStrategy();
3509+
b.ComplexProperty(e => e.Key);
3510+
b.HasKey(e => e.Key.Id);
3511+
b.Property(e => e.Key.Id).ValueGeneratedNever();
3512+
});
3513+
modelBuilder.Entity<TptDerivedWithComplexTypePK>();
3514+
3515+
var model = modelBuilder.FinalizeModel();
3516+
var relationalModel = model.GetRelationalModel();
3517+
3518+
var baseTable = relationalModel.Tables.Single(t => t.Name == nameof(TptBaseWithComplexTypePK));
3519+
var childTable = relationalModel.Tables.Single(t => t.Name == nameof(TptDerivedWithComplexTypePK));
3520+
3521+
// The PK column for the complex-typed key must appear in both the base table and the child table.
3522+
Assert.Contains(baseTable.Columns, c => c.Name == "Key_Id");
3523+
Assert.Contains(childTable.Columns, c => c.Name == "Key_Id");
3524+
3525+
// Non-key properties of the derived entity stay on the child table.
3526+
Assert.Contains(childTable.Columns, c => c.Name == nameof(TptDerivedWithComplexTypePK.Extra));
3527+
Assert.DoesNotContain(baseTable.Columns, c => c.Name == nameof(TptDerivedWithComplexTypePK.Extra));
3528+
}
3529+
3530+
[Fact]
3531+
public void Complex_property_json_column_is_not_duplicated_in_TPT_child_views()
3532+
{
3533+
var modelBuilder = CreateConventionModelBuilder();
3534+
3535+
modelBuilder.Entity<TptBaseEntityWithComplexProperty>(b =>
3536+
{
3537+
b.UseTptMappingStrategy();
3538+
b.ToTable(nameof(TptBaseEntityWithComplexProperty));
3539+
b.ToView(nameof(TptBaseEntityWithComplexProperty) + "View");
3540+
b.ComplexProperty(e => e.ComplexProperty, cb => cb.ToJson());
3541+
});
3542+
modelBuilder.Entity<TptDerivedEntityWithoutComplexProperty>(b =>
3543+
{
3544+
b.ToTable(nameof(TptDerivedEntityWithoutComplexProperty));
3545+
b.ToView(nameof(TptDerivedEntityWithoutComplexProperty) + "View");
3546+
});
3547+
3548+
var model = modelBuilder.FinalizeModel();
3549+
var relationalModel = model.GetRelationalModel();
3550+
3551+
var baseView = relationalModel.Views.Single(v => v.Name == nameof(TptBaseEntityWithComplexProperty) + "View");
3552+
var childView = relationalModel.Views.Single(v => v.Name == nameof(TptDerivedEntityWithoutComplexProperty) + "View");
3553+
3554+
// The JSON column for the base complex property must appear only in the base view.
3555+
Assert.Contains(baseView.Columns, c => c.Name == nameof(TptBaseEntityWithComplexProperty.ComplexProperty));
3556+
Assert.DoesNotContain(childView.Columns, c => c.Name == nameof(TptBaseEntityWithComplexProperty.ComplexProperty));
3557+
}
3558+
33843559
[Fact]
33853560
public void Json_element_tree_is_built_for_owned_entity_json_columns()
33863561
{
@@ -3994,6 +4169,37 @@ private class TphEntityWithComplexProperty : TphBaseEntity
39944169
public ComplexData ComplexProperty { get; set; }
39954170
}
39964171

4172+
private abstract class TptBaseEntityWithComplexProperty
4173+
{
4174+
public int Id { get; set; }
4175+
public ComplexData ComplexProperty { get; set; }
4176+
}
4177+
4178+
private class TptDerivedEntityWithoutComplexProperty : TptBaseEntityWithComplexProperty;
4179+
4180+
private class TpcBaseEntityWithComplexProperty
4181+
{
4182+
public int Id { get; set; }
4183+
public ComplexData ComplexProperty { get; set; }
4184+
}
4185+
4186+
private class TpcDerivedEntityWithoutComplexProperty : TpcBaseEntityWithComplexProperty;
4187+
4188+
private abstract class TptBaseWithComplexTypePK
4189+
{
4190+
public TptComplexKey Key { get; set; } = null!;
4191+
}
4192+
4193+
private class TptComplexKey
4194+
{
4195+
public int Id { get; set; }
4196+
}
4197+
4198+
private class TptDerivedWithComplexTypePK : TptBaseWithComplexTypePK
4199+
{
4200+
public int Extra { get; set; }
4201+
}
4202+
39974203
private class EntityWithJsonOwnedWithCollection
39984204
{
39994205
public int Id { get; set; }

0 commit comments

Comments
 (0)