Skip to content

Added metadata for complextypes for AddViews / TVF in RelationalModel #36894

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Added metadata for complextypes for AddViews / TVF in RelationalModel #36894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d1f0c1
Added test for select complex property from tvf
kirolous-nashaat Sep 28, 2025
a4f32d1
Fixed Update_complex_type_with_view_mapping tests
kirolous-nashaat Oct 12, 2025
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
39 changes: 36 additions & 3 deletions src/EFCore.Relational/Metadata/Internal/RelationalModel.cs
View file
Open in desktop
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.Data;
using System.Text;

namespace Microsoft.EntityFrameworkCore.Metadata.Internal;
Expand Down Expand Up @@ -713,8 +714,8 @@ private static void AddViews(

private static void CreateViewMapping(
IRelationalTypeMappingSource relationalTypeMappingSource,
IEntityType entityType,
IEntityType mappedType,
ITypeBase entityType,
ITypeBase mappedType,
StoreObjectIdentifier mappedView,
RelationalModel databaseModel,
List<ViewMapping> viewMappings,
Expand Down Expand Up @@ -770,11 +771,43 @@ private static void CreateViewMapping(
}
}

// TODO: Change this to call GetComplexProperties()
// Issue #31248
foreach (var complexProperty in mappedType.GetDeclaredComplexProperties())
{
var complexType = complexProperty.ComplexType;

var complexViewMappings =
(List<ViewMapping>?)complexType.FindRuntimeAnnotationValue(RelationalAnnotationNames.ViewMappings);
if (complexViewMappings == null)
{
complexViewMappings = [];
complexType.AddRuntimeAnnotation(RelationalAnnotationNames.ViewMappings, complexViewMappings);
}

CreateViewMapping(
relationalTypeMappingSource,
complexType,
complexType,
mappedView,
databaseModel,
complexViewMappings,
includesDerivedTypes: true,
isSplitEntityTypePrincipal: isSplitEntityTypePrincipal == true ? false : isSplitEntityTypePrincipal);
}

if (((ITableMappingBase)viewMapping).ColumnMappings.Any()
|| viewMappings.Count == 0)
{
viewMappings.Add(viewMapping);
view.EntityTypeMappings.Add(viewMapping);
if(entityType is IEntityType)
{
view.EntityTypeMappings.Add(viewMapping);
}
else
{
view.ComplexTypeMappings.Add(viewMapping);
}
}
}

Expand Down
7 changes: 3 additions & 4 deletions src/EFCore.Relational/Metadata/Internal/RelationalTypeBaseExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ public static class RelationalTypeBaseExtensions
public static IEnumerable<ITableMappingBase> GetViewOrTableMappings(this ITypeBase typeBase)
{
typeBase.Model.EnsureRelationalModel();
return (IEnumerable<ITableMappingBase>?)(typeBase.FindRuntimeAnnotationValue(
RelationalAnnotationNames.ViewMappings)
?? typeBase.FindRuntimeAnnotationValue(RelationalAnnotationNames.TableMappings))
?? [];
var viewMapping = typeBase.FindRuntimeAnnotationValue(RelationalAnnotationNames.ViewMappings);
var tableMapping = typeBase.FindRuntimeAnnotationValue(RelationalAnnotationNames.TableMappings);
return (IEnumerable<ITableMappingBase>?)(viewMapping ?? tableMapping) ?? [];
}
}
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Metadata/Internal/ViewMapping.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class ViewMapping : TableMappingBase<ViewColumnMapping>, IViewMapping
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public ViewMapping(
IEntityType entityType,
ITypeBase entityType,
View view,
bool? includesDerivedTypes)
: base(entityType, view, includesDerivedTypes)
Expand Down
6 changes: 3 additions & 3 deletions ...e.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.ExecuteUpdate.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ bool TryTranslateMemberAccess(
[NotNullWhen(true)] out IPropertyBase? property)
{
if (IsMemberAccess(expression, QueryCompilationContext.Model, out var baseExpression, out var member)
&& _sqlTranslator.TryBindMember(_sqlTranslator.Visit(baseExpression), member, out var target, out var targetProperty))
&& _sqlTranslator.TryBindMember(_sqlTranslator.Visit(baseExpression), member, out var target, out var targetProperty, forUpdate: true))
{
translation = target;
property = targetProperty;
Expand Down Expand Up @@ -540,7 +540,7 @@ void ProcessComplexType(StructuralTypeShaperExpression shaperExpression, Express
RelationalStrings.ExecuteUpdateOverJsonIsNotSupported(complexProperty.ComplexType.DisplayName()));
}

var nestedShaperExpression = (StructuralTypeShaperExpression)projection.BindComplexProperty(complexProperty);
var nestedShaperExpression = (StructuralTypeShaperExpression)projection.BindComplexProperty(complexProperty, forUpdate: true);
var nestedValueExpression = CreateComplexPropertyAccessExpression(valueExpression, complexProperty);
ProcessComplexType(nestedShaperExpression, nestedValueExpression);
}
Expand Down Expand Up @@ -635,7 +635,7 @@ SqlParameterExpression parameter
StructuralType: IComplexType,
ValueBufferExpression: StructuralTypeProjectionExpression projection
}
=> projection.BindComplexProperty(complexProperty),
=> projection.BindComplexProperty(complexProperty, forUpdate: true),

_ => throw new UnreachableException()
};
Expand Down
9 changes: 5 additions & 4 deletions src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,8 @@ public virtual bool TryBindMember(
Expression? source,
MemberIdentity member,
[NotNullWhen(true)] out Expression? expression,
[NotNullWhen(true)] out IPropertyBase? property)
[NotNullWhen(true)] out IPropertyBase? property,
bool forUpdate = false)
{
if (source is not StructuralTypeReferenceExpression typeReference)
{
Expand Down Expand Up @@ -1247,7 +1248,7 @@ public virtual bool TryBindMember(

if (complexProperty is not null)
{
expression = BindComplexProperty(typeReference, complexProperty);
expression = BindComplexProperty(typeReference, complexProperty, forUpdate: forUpdate);
property = complexProperty;
return true;
}
Expand Down Expand Up @@ -1359,7 +1360,7 @@ private SqlExpression BindProperty(StructuralTypeReferenceExpression typeReferen
}
}

private Expression BindComplexProperty(StructuralTypeReferenceExpression typeReference, IComplexProperty complexProperty)
private Expression BindComplexProperty(StructuralTypeReferenceExpression typeReference, IComplexProperty complexProperty, bool forUpdate = false)
{
switch (typeReference)
{
Expand All @@ -1370,7 +1371,7 @@ private Expression BindComplexProperty(StructuralTypeReferenceExpression typeRef
// TODO: Move all this logic into StructuralTypeProjectionExpression, #31376
Check.DebugAssert(structuralTypeProjection.IsNullable == shaper.IsNullable, "Nullability mismatch");

return structuralTypeProjection.BindComplexProperty(complexProperty) switch
return structuralTypeProjection.BindComplexProperty(complexProperty, forUpdate: forUpdate) switch
{
StructuralTypeShaperExpression s => new StructuralTypeReferenceExpression(s),
CollectionResultExpression c => c,
Expand Down
5 changes: 3 additions & 2 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2855,14 +2855,15 @@ static TableExpressionBase FindRootTableExpressionForColumn(SelectExpression sel
[EntityFrameworkInternal]
public static Expression GenerateComplexPropertyShaperExpression(
StructuralTypeProjectionExpression containerProjection,
IComplexProperty complexProperty)
IComplexProperty complexProperty,
bool forUpdate = false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roji What do you think about this?

Copy link
Member

@roji roji Oct 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the ping.

Yeah, I don't we should be doing this - not in this PR, in any case. We don't currently flow "for query" vs "for update" into the query pipeline for any other case; our current approach to this is instead to rewrite the main target table from the view to the table, specifically when translating ExecuteUpdate (see code). The advantage of that approach is keeping the view vs. table concern out of the query pipeline, and dealing with it only where it's relevant.

So the existing code (linked to above) should take care of everything (including for complex properties) - in fact I'm surprised it does not do so already. What exactly prompted you to introduce this change?

I'm also noting that we should only be referencing the table for the mutation; any property accesses elsewhere (e.g. in the WHERE clause) should reference the view.

If we do want to change the way we're dealing with this and filter the information into the query pipeline, we should probably do this more generally in a separate PR, and remove the current code switching from the view to the table which I linked to above.

Copy link
Author

@kirolous-nashaat kirolous-nashaat Oct 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please check this comment for initial thoughts about the issue
#34627 (comment)

before the change, we didnt have view metadata for complex types

the test mentioned above broke cuz of this change
it was relying on complex props not having mapping in the view ( container expression ) , it throws key not found , as expected in the test

after the change
now we have metadata for complex types, execute update find a mapping in the view
try to execute something like this
update viewx set column = ..

which throws exception cuz viewx isnot a table and can't be updated

so to keep the old test behavior and not break it, i had to know if i need table mappings only or viewortablemappings

select expression mapping of complex property does use getviewortablemapping
which return = view ?? table
so you need to distinguish if you need select expression for update / read statements accordingly

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also noting that we should only be referencing the table for the mutation; any property accesses elsewhere (e.g. in the WHERE clause) should reference the view.

not sure about this part.
for example i want to update table where a column is x

there is a where condition but because we are in an update statement we cant use the view at all

so mutation uses table regardless of where/set

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after the change
now we have metadata for complex types, execute update find a mapping in the view
try to execute something like this
update viewx set column = ..

So as I wrote above, we already have code in the ExecuteUpdate translation which is supposed to take care of this - I'm wondering what it doesn't work.

Note that I'm not necessarily opposed to the approach here: flowing forUpdate in like this PR proposes is in some ways probably better than the current approach of doing a translation, and then rewriting that translation to transform the view to the table (always better do just do the right thing immediately rather than do one thing and rewrite later, and this would certainly simplify the ExecuteUpdate logic by removing the transformation...).

But if we do go in this direction, we should switch to it fully, rather than doing one thing for complex properties and another for regular entities.

@AndriySvyryd any thoughts?

Copy link
Author

@kirolous-nashaat kirolous-nashaat Oct 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so @roji , you mean i should try to make the execute update work or do we still need it to fail in case of complex types + view + table ?

by figuring out why the snippet you sent didn't work as expected

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExecuteUpdate definitely shouldn't fail for complex types, though we can also decide to handle that separately if we want - it's OK for this PR to just handle the metadata side, and then a separate issue/PR for making sure everything works in the query pipeline.

But in general, yes, we should figure out why the existing code (which I sent) doesn't take care of things. As an alternative, we could decide that your way (flowing forUpdate into the query pipeline) is better, but then we need everything to work that way - include non-complex (entity) types. Your PR, as is, would mean that there are two different approaches to the same problem: for regular entity types, the code snippet I sent handles this in ExecuteUpdate (by rewriting the view to a table), whereas for complex types, we flow forUpdate in. We shouldn't have two ways of doing this.

Am mainly wanting to hear @AndriySvyryd's opinion on this.

{
var complexType = complexProperty.ComplexType;
var propertyExpressionMap = new Dictionary<IProperty, ColumnExpression>();

// We do not support complex type splitting, so we will only ever have a single table/view mapping to it.
// See Issue #32853 and Issue #31248
var complexTypeTable = complexType.GetViewOrTableMappings().Single().Table;
ITableBase complexTypeTable = forUpdate ? complexType.GetTableMappings().Single().Table : complexType.GetViewOrTableMappings().Single().Table;

if (!containerProjection.TableMap.TryGetValue(complexTypeTable, out var tableAlias))
{
Expand Down
7 changes: 6 additions & 1 deletion src/EFCore.Relational/Query/StructuralTypeProjectionExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,14 @@ public virtual ColumnExpression BindProperty(IProperty property)
/// Binds a complex property with this structural type projection to get a shaper expression for the target complex type.
/// </summary>
/// <param name="complexProperty">A complex property to bind.</param>
/// <param name="forUpdate">Is the mapping for read (false) or update (true).</param>
/// <returns>A shaper expression for the target complex type.</returns>
public virtual Expression BindComplexProperty(IComplexProperty complexProperty)
public virtual Expression BindComplexProperty(IComplexProperty complexProperty, bool forUpdate = false)
{
if (forUpdate)
{
return SelectExpression.GenerateComplexPropertyShaperExpression(this, complexProperty, forUpdate: true);
}
if (_complexPropertyCache is null || !_complexPropertyCache.TryGetValue(complexProperty, out var resultShaper))
{
_complexPropertyCache ??= new Dictionary<IComplexProperty, Expression>();
Expand Down
65 changes: 64 additions & 1 deletion test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs
View file
Open in desktop
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.ComponentModel.DataAnnotations.Schema;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;

namespace Microsoft.EntityFrameworkCore.Query;
Expand All @@ -17,12 +18,23 @@ protected UDFSqlContext CreateContext()

#region Model

[ComplexType]
public class Phone
{
public Phone(int code, int number)
{
Code = code;
Number = number;
}

public int Code { get; set; }
public int Number { get; set; }
}
public class Customer
{
public int Id { get; set; }
public string FirstName { get; set; }
public string LastName { get; set; }

public List<Order> Orders { get; set; }
public List<Address> Addresses { get; set; }
}
Expand Down Expand Up @@ -92,6 +104,31 @@ public class TopSellingProduct
public int? AmountSold { get; set; }
}

[ComplexType]
public class ComplexGpsCoordinates
{
public ComplexGpsCoordinates(double latitude, double longitude)
{
Latitude = latitude;
Longitude = longitude;
}

public double Latitude { get; set; }
public double Longitude { get; set; }
}

public class MapLocation
{
public int Id { get; set; }
public ComplexGpsCoordinates GpsCoordinates { get; set; }
}

public class MapLocationData
{
public int Id { get; set; }
public ComplexGpsCoordinates GpsCoordinates { get; set; }
}

public class CustomerData
{
public int Id { get; set; }
Expand All @@ -107,6 +144,7 @@ protected class UDFSqlContext(DbContextOptions options) : PoolableDbContext(opti
public DbSet<Order> Orders { get; set; }
public DbSet<Product> Products { get; set; }
public DbSet<Address> Addresses { get; set; }
public DbSet<MapLocation> MapLocations { get; set; }

#endregion

Expand Down Expand Up @@ -355,6 +393,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
modelBuilder.Entity<OrderByYear>().HasNoKey();
modelBuilder.Entity<TopSellingProduct>().HasNoKey().ToFunction("GetTopTwoSellingProducts");
modelBuilder.Entity<CustomerData>().ToView("Customers");
modelBuilder.Entity<MapLocationData>().ToView("MapLocations");
}
}

Expand Down Expand Up @@ -520,11 +559,22 @@ protected override async Task SeedAsync(DbContext context)
]
};

var location1 = new MapLocation
{
GpsCoordinates = new ComplexGpsCoordinates(1.0, 2.0),
};

var location2 = new MapLocation
{
GpsCoordinates = new ComplexGpsCoordinates(1.0, 2.0),
};

((UDFSqlContext)context).Products.AddRange(product1, product2, product3, product4, product5);
((UDFSqlContext)context).Addresses.AddRange(
address11, address12, address21, address31, address32, address41, address42, address43);
((UDFSqlContext)context).Customers.AddRange(customer1, customer2, customer3, customer4);
((UDFSqlContext)context).Orders.AddRange(order11, order12, order13, order21, order22, order31);
((UDFSqlContext)context).MapLocations.AddRange(location1, location2);
}
}

Expand Down Expand Up @@ -2182,6 +2232,19 @@ orderby t.FirstName
}
}

[ConditionalFact]
public virtual void TVF_backing_entity_type_with_complextype_mapped_to_view()
{
using (var context = CreateContext())
{
var locations = (from t in context.Set<MapLocationData>()
orderby t.Id
select t).ToList();

Assert.Equal(2, locations.Count);
}
}

[ConditionalFact]
public virtual void Udf_with_argument_being_comparison_to_null_parameter()
{
Expand Down
12 changes: 12 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Query/UdfDbFunctionSqlServerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,18 @@ ORDER BY [c].[FirstName]
""");
}

public override void TVF_backing_entity_type_with_complextype_mapped_to_view()
{
base.TVF_backing_entity_type_with_complextype_mapped_to_view();

AssertSql(
"""
SELECT [m].[Id], [m].[GpsCoordinates_Latitude], [m].[GpsCoordinates_Longitude]
FROM [MapLocations] AS [m]
ORDER BY [m].[Id]
""");
}

public override void Udf_with_argument_being_comparison_to_null_parameter()
{
base.Udf_with_argument_being_comparison_to_null_parameter();
Expand Down
Loading