diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 583799b46..bf315647c 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -2,7 +2,7 @@ CS1591;NU5104;CS1573;CS9107;NU1608;NU1109 - 35.4.0 + 35.5.0 preview 1.0.0 EntityFrameworkCore, EntityFramework, GraphQL diff --git a/src/GraphQL.EntityFramework/ConnectionConverter.cs b/src/GraphQL.EntityFramework/ConnectionConverter.cs index 1fd73aa5f..ea347d508 100644 --- a/src/GraphQL.EntityFramework/ConnectionConverter.cs +++ b/src/GraphQL.EntityFramework/ConnectionConverter.cs @@ -32,14 +32,33 @@ internal static IQueryable WithoutOrdering(IQueryable queryable) => /// /// The query to count, without the operators that only shape the entities returned, which a - /// count does not return: ordering, includes, split queries, and tracking. + /// count does not return: the select projection, ordering, includes, split queries, and + /// tracking. /// static IQueryable ForCount(IQueryable queryable) => - Strip(queryable, _ => IsOrdering(_) || _ is + Strip(WithoutProjection(queryable), _ => IsOrdering(_) || _ is "Include" or "ThenInclude" or "AsSplitQuery" or "AsSingleQuery" or "AsNoTracking" or "AsNoTrackingWithIdentityResolution" or "AsTracking"); + /// + /// The select projection is the last operator applied, and creates the same type it reads, so + /// there are as many rows without it. EF drops it from the count it translates, but only after + /// the whole projection, and every collection in it, has been compiled as part of the query. + /// A select that changes the type, or one further down, belongs to the resolver and is kept. + /// + static IQueryable WithoutProjection(IQueryable queryable) + { + if (queryable.Expression is MethodCallExpression { Method.Name: "Select", Object: null, Arguments.Count: 2 } select && + select.Method.DeclaringType == typeof(Queryable) && + typeof(IQueryable).IsAssignableFrom(select.Arguments[0].Type)) + { + return queryable.Provider.CreateQuery(select.Arguments[0]); + } + + return queryable; + } + static bool IsOrdering(string methodName) => methodName is "OrderBy" or "OrderByDescending" or "ThenBy" or "ThenByDescending"; diff --git a/src/GraphQL.EntityFramework/ForeignKeyExtractor.cs b/src/GraphQL.EntityFramework/ForeignKeyExtractor.cs index ac006fa0d..8ce0c405c 100644 --- a/src/GraphQL.EntityFramework/ForeignKeyExtractor.cs +++ b/src/GraphQL.EntityFramework/ForeignKeyExtractor.cs @@ -30,6 +30,19 @@ static IReadOnlySet GetForeignKeys(IEntityType entity) } } + // A query for this type also returns its derived types, each projected as itself, so the + // foreign keys they declare are loaded too. A name is only bound on the types that have it. + foreach (var derived in entity.GetDerivedTypes()) + { + foreach (var foreignKey in derived.GetDeclaredForeignKeys()) + { + foreach (var property in foreignKey.Properties) + { + foreignKeyNames.Add(property.Name); + } + } + } + // Include TPH discriminator property so projected entities maintain correct type identity. // Without this, projected entities get the default discriminator value (e.g. enum value 0) // instead of the actual value, causing downstream code that switches on the discriminator to fail. diff --git a/src/GraphQL.EntityFramework/SelectProjection/HoistedNavigation.cs b/src/GraphQL.EntityFramework/SelectProjection/HoistedNavigation.cs new file mode 100644 index 000000000..c4fe3d405 --- /dev/null +++ b/src/GraphQL.EntityFramework/SelectProjection/HoistedNavigation.cs @@ -0,0 +1,64 @@ +/// +/// Sets a navigation on an entity the projection has already created. A type with derived types is +/// projected as a chain of type tests, each creating its own type, and a navigation that loads a +/// collection is bound once to the result of that chain rather than inside every branch of it. +/// There is no expression for assigning a property of an existing instance that EF accepts in a +/// projection, so it is a call EF evaluates on the client, as it does the member inits. +/// +static class HoistedNavigation +{ + static MethodInfo setMethod = typeof(HoistedNavigation) + .GetMethod(nameof(Set), BindingFlags.Static | BindingFlags.NonPublic)!; + + static ConcurrentDictionary setMethods = new(); + + public static Expression Assign(Expression entity, Type entityType, PropertyInfo property, Expression value) + { + var method = setMethods.GetOrAdd( + property, + _ => setMethod.MakeGenericMethod(_.ReflectedType!, _.PropertyType)); + + if (entity.Type != entityType) + { + entity = Expression.Convert(entity, entityType); + } + + if (value.Type != property.PropertyType) + { + value = Expression.Convert(value, property.PropertyType); + } + + // The name is a constant, so the expression, and with it EF's compiled query cache entry, + // depends only on which navigations were requested + return Expression.Call(null, method, entity, value, Expression.Constant(property.Name)); + } + + static TEntity? Set(TEntity? entity, TValue value, string name) + where TEntity : class + { + if (entity is not null) + { + Setter.Get(name)(entity, value); + } + + return entity; + } + + static class Setter + where TEntity : class + { + static ConcurrentDictionary> setters = new(); + + public static Action Get(string name) => + setters.GetOrAdd( + name, + _ => + { + var property = typeof(TEntity).GetProperty(_, BindingFlags.Instance | BindingFlags.Public)!; + var entity = Expression.Parameter(typeof(TEntity)); + var value = Expression.Parameter(typeof(TValue)); + var assign = Expression.Assign(Expression.Property(entity, property), value); + return Expression.Lambda>(assign, entity, value).Compile(); + }); + } +} diff --git a/src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs b/src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs index 1e8505363..a80c405de 100644 --- a/src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs +++ b/src/GraphQL.EntityFramework/SelectProjection/SelectExpressionBuilder.cs @@ -63,7 +63,7 @@ record PropertyMetadata(Type EntityType, PropertyInfo Property, bool CanWrite, b record EntityTypeMetadata( ParameterExpression Parameter, IReadOnlyDictionary Properties, - NewExpression NewInstance, + NewExpression? NewInstance, MethodInfo SelectMethod, MethodInfo ToListMethod, ConstantExpression NullConstant); @@ -122,11 +122,6 @@ public static bool TryBuild( argumentFields = state.ArgumentFields; var entityType = typeof(TEntity); - if (entityType.IsAbstract) - { - return false; - } - var parameter = GetEntityMetadata(entityType).Parameter; if (!TryBuildEntityInit(parameter, entityType, projection, state, null, out var body)) { @@ -143,6 +138,13 @@ public static bool TryBuild( /// matching type, most derived first. False when the entity, or one of the navigations under /// it that has no writable property to fall back to, cannot be projected. /// + /// + /// An abstract type cannot be created by a member init, but no row is ever of that type + /// itself: each is one of its concrete derived types, and the chain of type tests creates + /// those. So the chain ends in null where a concrete type's ends in its own member init. An + /// abstract type used to be loaded whole through includes, every column of it and of every + /// entity under it, whatever was selected. + /// static bool TryBuildEntityInit( Expression source, Type entityType, @@ -152,15 +154,37 @@ static bool TryBuildEntityInit( [NotNullWhen(true)] out Expression? expression) { expression = null; + state.DerivedTypes.TryGetValue(entityType, out var derived); - if (!TryBuildMemberInit(source, entityType, projection, state, path, out var memberInit)) + List<(PropertyMetadata Metadata, NavigationProjectionInfo Navigation)>? hoisted = null; + if (derived is not null) { - return false; + projection = Hoist(entityType, projection, out hoisted); + } + + Expression result; + if (entityType.IsAbstract) + { + if (!HasConcrete(derived)) + { + return false; + } + + result = GetEntityMetadata(entityType).NullConstant; + } + else + { + if (!TryBuildMemberInit(source, entityType, projection, state, path, out var memberInit)) + { + return false; + } + + result = memberInit; } - expression = memberInit; - if (!state.DerivedTypes.TryGetValue(entityType, out var derived)) + if (derived is null) { + expression = result; return true; } @@ -179,15 +203,90 @@ static bool TryBuildEntityInit( return false; } - expression = Expression.Condition( + result = Expression.Condition( Expression.TypeIs(source, derivedType), Expression.Convert(derivedInit, entityType), - expression); + result); } + if (hoisted is not null) + { + foreach (var (metadata, navigation) in hoisted) + { + var binding = BuildNavigationBinding(source, metadata, navigation, state, path); + if (binding is not null) + { + result = HoistedNavigation.Assign(result, entityType, metadata.Property, binding.Expression); + } + } + } + + expression = result; return true; } + /// + /// Takes the navigations out of the projection of a type with derived types. Each type test + /// in the chain creates its own member init, so a navigation bound in them was repeated once + /// per concrete type. EF does not recognise a repeated collection subquery as the same one, + /// so it joined, or split out, the collection once for every type. A reference navigation is + /// joined once regardless, but everything under it was repeated in the expression, and a + /// hierarchy under a hierarchy multiplied. These are instead bound once, to whichever type + /// the chain created. + /// + static FieldProjectionInfo Hoist( + Type entityType, + FieldProjectionInfo projection, + out List<(PropertyMetadata Metadata, NavigationProjectionInfo Navigation)>? hoisted) + { + hoisted = null; + if (projection.Navigations is null) + { + return projection; + } + + var properties = GetEntityMetadata(entityType).Properties; + Dictionary? kept = null; + foreach (var (name, navigation) in projection.Navigations) + { + if (!properties.TryGetValue(name, out var metadata) || + !metadata.CanWrite) + { + continue; + } + + hoisted ??= []; + hoisted.Add((metadata, navigation)); + kept ??= new(projection.Navigations); + kept.Remove(name); + } + + if (kept is null) + { + return projection; + } + + return projection with { Navigations = kept }; + } + + static bool HasConcrete(IReadOnlyList? types) + { + if (types is null) + { + return false; + } + + foreach (var type in types) + { + if (!type.IsAbstract) + { + return true; + } + } + + return false; + } + /// /// The fields selected through fragments on a derived type are recorded against the base /// projection: scalars by name, which resolve against the derived type's properties, and @@ -218,7 +317,7 @@ static bool TryBuildMemberInit( return false; } - memberInit = Expression.MemberInit(GetEntityMetadata(entityType).NewInstance, bindings); + memberInit = Expression.MemberInit(GetEntityMetadata(entityType).NewInstance!, bindings); return true; } @@ -300,22 +399,11 @@ static bool TryBuildBindings( continue; } - var navAccess = Expression.Property(source, metadata.Property); - var navPath = path is null ? metadata.Property.Name : $"{path}.{metadata.Property.Name}"; - if (!TryBuildNavigationBinding(navAccess, navProjection, state, navPath, out var binding)) + var binding = BuildNavigationBinding(source, metadata, navProjection, state, path); + if (binding is not null) { - if (!metadata.CanWrite) - { - continue; - } - - // Can't project navigation (e.g. read-only properties on target entity) - // Fall back to including the full navigation entity - binding = BuildFullNavigationBinding(navAccess, navProjection, state); - state.AddIncludePaths(navPath, navProjection.Projection); + bindings.Add(binding); } - - bindings.Add(binding); } } @@ -324,6 +412,35 @@ static bool TryBuildBindings( return true; } + /// + /// The binding of a navigation: projected where it can be, otherwise bound whole, with what + /// was requested under it loaded through includes. Null when it can be neither. + /// + static MemberAssignment? BuildNavigationBinding( + Expression source, + PropertyMetadata metadata, + NavigationProjectionInfo navProjection, + BuildState state, + string? path) + { + var navAccess = Expression.Property(source, metadata.Property); + var navPath = path is null ? metadata.Property.Name : $"{path}.{metadata.Property.Name}"; + if (TryBuildNavigationBinding(navAccess, navProjection, state, navPath, out var binding)) + { + return binding; + } + + if (!metadata.CanWrite) + { + return null; + } + + // Can't project navigation (e.g. read-only properties on target entity) + // Fall back to including the full navigation entity + state.AddIncludePaths(navPath, navProjection.Projection); + return BuildFullNavigationBinding(navAccess, navProjection, state); + } + static MemberAssignment Bind(Expression source, PropertyMetadata metadata) => Expression.Bind(metadata.Property, Expression.Property(source, metadata.Property)); @@ -346,9 +463,9 @@ static bool TryBuildNavigationBinding( binding = null; var navType = navProjection.EntityType; - // An abstract type cannot be created by a member init, and a navigation wanted whole has - // no field list to build one from. Both fall back to binding the navigation itself. - if (navType.IsAbstract || navProjection.IsWhole) + // A navigation wanted whole has no field list to build a member init from, so it falls + // back to binding the navigation itself. + if (navProjection.IsWhole) { return false; } @@ -554,7 +671,8 @@ static EntityTypeMetadata GetEntityMetadata(Type type) => dictionary[property.Name] = new(type, property, canWrite, isAutoProperty); } - var newInstance = Expression.New(type); + // Never created, so never asked for: see TryBuildEntityInit + var newInstance = type.IsAbstract ? null : Expression.New(type); var selectMethod = SelectExpressionBuilder.selectMethod.MakeGenericMethod(type, type); var toListMethod = SelectExpressionBuilder.toListMethod.MakeGenericMethod(type); var nullConstant = Expression.Constant(null, type); diff --git a/src/Tests/IntegrationTests/Graphs/TphBaseTypeInclude/TphRootEntity.cs b/src/Tests/IntegrationTests/Graphs/TphBaseTypeInclude/TphRootEntity.cs index 2cdb9d2a9..d973b68ee 100644 --- a/src/Tests/IntegrationTests/Graphs/TphBaseTypeInclude/TphRootEntity.cs +++ b/src/Tests/IntegrationTests/Graphs/TphBaseTypeInclude/TphRootEntity.cs @@ -3,4 +3,7 @@ public abstract class TphRootEntity public Guid Id { get; set; } = Guid.NewGuid(); public string? Property { get; set; } public IList Attachments { get; set; } = []; + + // Read only, so a select cannot bind it. Selecting it puts a query on the includes path. + public string Summary => $"Summary of {Property}"; } diff --git a/src/Tests/IntegrationTests/IntegrationTests.Abstract_root_type_collection_is_loaded_once.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Abstract_root_type_collection_is_loaded_once.verified.txt new file mode 100644 index 000000000..f796eb6c2 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Abstract_root_type_collection_is_loaded_once.verified.txt @@ -0,0 +1,45 @@ +{ + target: { + Data: { + baseEntities: [ + { + property: Parent1, + childrenFromInterface: { + items: [ + { + property: Child1 + }, + { + property: Child2 + } + ] + } + }, + { + property: Parent2, + childrenFromInterface: { + items: [] + } + } + ] + } + }, + sql: { + Text: +select case when b.Discriminator = N'DerivedWithNavigationEntity' then cast (1 as bit) else cast (0 as bit) end, + b.Id, + b.Property, + case when b.Discriminator = N'DerivedEntity' then cast (1 as bit) else cast (0 as bit) end, + d.Id, + d.ParentId, + d.Property, + d.TypedParentId +from BaseEntities as b + left outer join + DerivedChildEntities as d + on b.Id = d.ParentId +order by b.Property, + b.Id, + d.Property + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Abstract_root_type_query_falls_back_to_include.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Abstract_root_type_query_falls_back_to_include.verified.txt deleted file mode 100644 index ad657ace4..000000000 --- a/src/Tests/IntegrationTests/IntegrationTests.Abstract_root_type_query_falls_back_to_include.verified.txt +++ /dev/null @@ -1,20 +0,0 @@ -{ - target: { - Data: { - baseEntities: [ - { - property: Derived1 - } - ] - } - }, - sql: { - Text: -select b.Id, - b.Discriminator, - b.Property, - b.Status -from BaseEntities as b -order by b.Property - } -} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Abstract_root_type_query_is_projected.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Abstract_root_type_query_is_projected.verified.txt new file mode 100644 index 000000000..c80dc256e --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Abstract_root_type_query_is_projected.verified.txt @@ -0,0 +1,20 @@ +{ + target: { + Data: { + baseEntities: [ + { + property: Derived1 + } + ] + } + }, + sql: { + Text: +select case when b.Discriminator = N'DerivedWithNavigationEntity' then cast (1 as bit) else cast (0 as bit) end, + b.Id, + b.Property, + case when b.Discriminator = N'DerivedEntity' then cast (1 as bit) else cast (0 as bit) end +from BaseEntities as b +order by b.Property + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Batch_filter_on_derived_type_applies_to_base_typed_list.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Batch_filter_on_derived_type_applies_to_base_typed_list.verified.txt index b6f7a691c..7cd220e43 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Batch_filter_on_derived_type_applies_to_base_typed_list.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Batch_filter_on_derived_type_applies_to_base_typed_list.verified.txt @@ -13,10 +13,10 @@ }, sql: { Text: -select b.Id, - b.Discriminator, +select case when b.Discriminator = N'DerivedWithNavigationEntity' then cast (1 as bit) else cast (0 as bit) end, + b.Id, b.Property, - b.Status + case when b.Discriminator = N'DerivedEntity' then cast (1 as bit) else cast (0 as bit) end from BaseEntities as b order by b.Property }, diff --git a/src/Tests/IntegrationTests/IntegrationTests.Connection_split_query_with_derived_types_without_collection.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Connection_split_query_with_derived_types_without_collection.verified.txt index 131c19f69..16237afdc 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Connection_split_query_with_derived_types_without_collection.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Connection_split_query_with_derived_types_without_collection.verified.txt @@ -22,10 +22,10 @@ from BaseEntities as b }, { Text: -select b.Id, - b.Discriminator, +select case when b.Discriminator = N'DerivedWithNavigationEntity' then cast (1 as bit) else cast (0 as bit) end, + b.Id, b.Property, - b.Status + case when b.Discriminator = N'DerivedEntity' then cast (1 as bit) else cast (0 as bit) end from BaseEntities as b order by b.Property offset @p rows fetch next @p1 rows only, diff --git a/src/Tests/IntegrationTests/IntegrationTests.Derived_navigation_inline_fragment_inside_fragment_spread.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Derived_navigation_inline_fragment_inside_fragment_spread.verified.txt index 930c10123..554fe74ca 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Derived_navigation_inline_fragment_inside_fragment_spread.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Derived_navigation_inline_fragment_inside_fragment_spread.verified.txt @@ -16,14 +16,16 @@ }, sql: { Text: -select t.Id, - t.Discriminator, +select case when t.Discriminator = N'TphDerivedNavRegionEntity' then cast (1 as bit) else cast (0 as bit) end, + t.Id, t.OwnerId, t.Property, - t.CategoryId, t.RegionId, + case when t.Discriminator = N'TphDerivedNavCategoryEntity' then cast (1 as bit) else cast (0 as bit) end, + case when c.Id is null then cast (1 as bit) else cast (0 as bit) end, c.Id, - c.Name + c.Name, + t.CategoryId from TphDerivedNavBaseEntities as t left outer join CategoryEntities as c diff --git a/src/Tests/IntegrationTests/IntegrationTests.Derived_navigation_under_fragment_below_reference_navigation.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Derived_navigation_under_fragment_below_reference_navigation.verified.txt index a515aaedc..5b910653d 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Derived_navigation_under_fragment_below_reference_navigation.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Derived_navigation_under_fragment_below_reference_navigation.verified.txt @@ -17,14 +17,19 @@ sql: { Text: select t.Id, + case when t0.Id is null then cast (1 as bit) else cast (0 as bit) end, + case when t0.Discriminator = N'TphDerivedNavRegionEntity' + and t0.Discriminator is not null then cast (1 as bit) else cast (0 as bit) end, t0.Id, - t0.Discriminator, t0.OwnerId, t0.Property, - t0.CategoryId, t0.RegionId, + case when t0.Discriminator = N'TphDerivedNavCategoryEntity' + and t0.Discriminator is not null then cast (1 as bit) else cast (0 as bit) end, + case when c.Id is null then cast (1 as bit) else cast (0 as bit) end, c.Id, c.Name, + t0.CategoryId, t.ItemId, t.Property from TphDerivedNavOwners as t diff --git a/src/Tests/IntegrationTests/IntegrationTests.Derived_navigations_under_fragments_below_collection_navigation.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Derived_navigations_under_fragments_below_collection_navigation.verified.txt index c8dde315f..1f7dc182e 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Derived_navigations_under_fragments_below_collection_navigation.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Derived_navigations_under_fragments_below_collection_navigation.verified.txt @@ -26,36 +26,42 @@ Text: select t.Id, t.ItemId, + s.c, s.Id, - s.Discriminator, s.OwnerId, s.Property, - s.CategoryId, - s.RegionId, + s.c0, s.Id0, s.Name, + s.RegionId, + s.c1, + s.c2, s.Id1, s.Name0, + s.CategoryId, t.Property from TphDerivedNavOwners as t left outer join - (select t0.Id, - t0.Discriminator, + (select case when t0.Discriminator = N'TphDerivedNavRegionEntity' then cast (1 as bit) else cast (0 as bit) end as c, + t0.Id, t0.OwnerId, t0.Property, - t0.CategoryId, + case when r.Id is null then cast (1 as bit) else cast (0 as bit) end as c0, + r.Id as Id0, + r.Name, t0.RegionId, - c.Id as Id0, - c.Name, - r.Id as Id1, - r.Name as Name0 + case when t0.Discriminator = N'TphDerivedNavCategoryEntity' then cast (1 as bit) else cast (0 as bit) end as c1, + case when c.Id is null then cast (1 as bit) else cast (0 as bit) end as c2, + c.Id as Id1, + c.Name as Name0, + t0.CategoryId from TphDerivedNavBaseEntities as t0 - left outer join - CategoryEntities as c - on t0.CategoryId = c.Id left outer join RegionEntities as r - on t0.RegionId = r.Id) as s + on t0.RegionId = r.Id + left outer join + CategoryEntities as c + on t0.CategoryId = c.Id) as s on t.Id = s.OwnerId order by t.Id, s.Property, diff --git a/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_to_base_typed_list.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_to_base_typed_list.verified.txt index f6dfe2c2b..8e065d892 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_to_base_typed_list.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_to_base_typed_list.verified.txt @@ -13,10 +13,10 @@ }, sql: { Text: -select b.Id, - b.Discriminator, +select case when b.Discriminator = N'DerivedWithNavigationEntity' then cast (1 as bit) else cast (0 as bit) end, + b.Id, b.Property, - b.Status + case when b.Discriminator = N'DerivedEntity' then cast (1 as bit) else cast (0 as bit) end from BaseEntities as b order by b.Property } diff --git a/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_to_base_typed_navigation.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_to_base_typed_navigation.verified.txt index 99df4fb00..4ae1d45cb 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_to_base_typed_navigation.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_to_base_typed_navigation.verified.txt @@ -24,10 +24,13 @@ sql: { Text: select d.Id, + case when b.Id is null then cast (1 as bit) else cast (0 as bit) end, + case when b.Discriminator = N'DerivedWithNavigationEntity' + and b.Discriminator is not null then cast (1 as bit) else cast (0 as bit) end, b.Id, - b.Discriminator, b.Property, - b.Status, + case when b.Discriminator = N'DerivedEntity' + and b.Discriminator is not null then cast (1 as bit) else cast (0 as bit) end, d.ParentId, d.Property, d.TypedParentId diff --git a/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_to_base_typed_single.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_to_base_typed_single.verified.txt index 83bc7263b..7a7eed3f5 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_to_base_typed_single.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_to_base_typed_single.verified.txt @@ -5,10 +5,9 @@ }, sql: { Text: -select top (2) t.Id, - t.Discriminator, - t.Property, - t.LeafProperty +select top (2) cast (1 as bit), + t.Id, + t.Property from TphRootEntities as t where t.Id = @p1, Parameters: { diff --git a/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_when_not_first_in_list.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_when_not_first_in_list.verified.txt index 4c517c68c..4201d6f56 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_when_not_first_in_list.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_applies_when_not_first_in_list.verified.txt @@ -13,10 +13,10 @@ }, sql: { Text: -select b.Id, - b.Discriminator, +select case when b.Discriminator = N'DerivedWithNavigationEntity' then cast (1 as bit) else cast (0 as bit) end, + b.Id, b.Property, - b.Status + case when b.Discriminator = N'DerivedEntity' then cast (1 as bit) else cast (0 as bit) end from BaseEntities as b order by b.Property } diff --git a/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_requirements_loaded_by_base_typed_query.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_requirements_loaded_by_base_typed_query.verified.txt index a4225e387..1c0ee3b4c 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_requirements_loaded_by_base_typed_query.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Filter_on_derived_type_requirements_loaded_by_base_typed_query.verified.txt @@ -12,10 +12,11 @@ }, sql: { Text: -select b.Id, - b.Discriminator, +select case when b.Discriminator = N'DerivedWithNavigationEntity' then cast (1 as bit) else cast (0 as bit) end, + b.Id, b.Property, - b.Status + b.Status, + case when b.Discriminator = N'DerivedEntity' then cast (1 as bit) else cast (0 as bit) end from BaseEntities as b order by b.Property offset @p rows fetch next @p1 rows only, diff --git a/src/Tests/IntegrationTests/IntegrationTests.Filter_projection_with_abstract_navigation_should_not_throw.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Filter_projection_with_abstract_navigation_should_not_throw.verified.txt index c067671bf..3a825b921 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Filter_projection_with_abstract_navigation_should_not_throw.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Filter_projection_with_abstract_navigation_should_not_throw.verified.txt @@ -14,10 +14,12 @@ sql: { Text: select d.Id, - b.Id, - b.Discriminator, + case when b.Id is null then cast (1 as bit) else cast (0 as bit) end, + case when b.Discriminator = N'DerivedWithNavigationEntity' + and b.Discriminator is not null then cast (1 as bit) else cast (0 as bit) end, b.Property, - b.Status, + case when b.Discriminator = N'DerivedEntity' + and b.Discriminator is not null then cast (1 as bit) else cast (0 as bit) end, d.ParentId, d.Property, d.TypedParentId diff --git a/src/Tests/IntegrationTests/IntegrationTests.Navigation_to_abstract_type_falls_back_to_full_include.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Navigation_to_abstract_type_is_projected.verified.txt similarity index 54% rename from src/Tests/IntegrationTests/IntegrationTests.Navigation_to_abstract_type_falls_back_to_full_include.verified.txt rename to src/Tests/IntegrationTests/IntegrationTests.Navigation_to_abstract_type_is_projected.verified.txt index db220d52d..8a50120ac 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Navigation_to_abstract_type_falls_back_to_full_include.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Navigation_to_abstract_type_is_projected.verified.txt @@ -14,10 +14,13 @@ sql: { Text: select d.Id, + case when b.Id is null then cast (1 as bit) else cast (0 as bit) end, + case when b.Discriminator = N'DerivedWithNavigationEntity' + and b.Discriminator is not null then cast (1 as bit) else cast (0 as bit) end, b.Id, - b.Discriminator, b.Property, - b.Status, + case when b.Discriminator = N'DerivedEntity' + and b.Discriminator is not null then cast (1 as bit) else cast (0 as bit) end, d.ParentId, d.Property, d.TypedParentId diff --git a/src/Tests/IntegrationTests/IntegrationTests.Navigation_under_abstract_navigation_is_loaded.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Navigation_under_abstract_navigation_is_loaded.verified.txt index 5eba33c97..d3f9b3061 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Navigation_under_abstract_navigation_is_loaded.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Navigation_under_abstract_navigation_is_loaded.verified.txt @@ -24,10 +24,13 @@ sql: { Text: select d.Id, + case when b.Id is null then cast (1 as bit) else cast (0 as bit) end, + case when b.Discriminator = N'DerivedWithNavigationEntity' + and b.Discriminator is not null then cast (1 as bit) else cast (0 as bit) end, b.Id, - b.Discriminator, b.Property, - b.Status, + case when b.Discriminator = N'DerivedEntity' + and b.Discriminator is not null then cast (1 as bit) else cast (0 as bit) end, d0.Id, d0.ParentId, d0.Property, @@ -46,6 +49,7 @@ where d.Property = @p order by d.Property, d.Id, b.Id, + d0.Property, Parameters: { @p: { Value: Child1, diff --git a/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt index 52437ad2e..a8fc819df 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt @@ -1790,6 +1790,7 @@ input DiscriminatorDerivedBEntityOrderBy { interface TphMiddleEntity { id: ID! property: String + summary: String! attachments(id: ID, ids: [ID!], where: TphAttachmentEntityWhere, orderBy: [TphAttachmentEntityOrderBy!], skip: Int, take: Int): [TphAttachment!]! } @@ -2010,6 +2011,7 @@ type TphLeaf implements TphMiddleEntity { id: ID! leafProperty: String property: String + summary: String! } type TphDerivedNavCategory implements TphDerivedNavBaseEntity { diff --git a/src/Tests/IntegrationTests/IntegrationTests.Tph_base_type_include_skipped_with_filter.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Tph_base_type_include_skipped_with_filter.verified.txt index c8d6079cc..944c4414b 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Tph_base_type_include_skipped_with_filter.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Tph_base_type_include_skipped_with_filter.verified.txt @@ -4,6 +4,7 @@ tphMiddleEntities: [ { property: TheRequest, + summary: Summary of TheRequest, attachments: [ { property: TheAttachment diff --git a/src/Tests/IntegrationTests/IntegrationTests.Tph_base_type_include_skipped_with_filter_no_tracking.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Tph_base_type_include_skipped_with_filter_no_tracking.verified.txt index c8d6079cc..944c4414b 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Tph_base_type_include_skipped_with_filter_no_tracking.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Tph_base_type_include_skipped_with_filter_no_tracking.verified.txt @@ -4,6 +4,7 @@ tphMiddleEntities: [ { property: TheRequest, + summary: Summary of TheRequest, attachments: [ { property: TheAttachment diff --git a/src/Tests/IntegrationTests/IntegrationTests.Tph_base_type_single_include_skipped_with_filter.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Tph_base_type_single_include_skipped_with_filter.verified.txt index cb3900104..8ca467953 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Tph_base_type_single_include_skipped_with_filter.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Tph_base_type_single_include_skipped_with_filter.verified.txt @@ -3,6 +3,7 @@ Data: { tphMiddleEntity: { property: TheRequest, + summary: Summary of TheRequest, attachments: [ { property: TheAttachment diff --git a/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_base_fields_still_work.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_base_fields_still_work.verified.txt index 7a4e0b724..d1d38c888 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_base_fields_still_work.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_base_fields_still_work.verified.txt @@ -13,12 +13,13 @@ }, sql: { Text: -select t.Id, - t.Discriminator, +select case when t.Discriminator = N'TphDerivedNavRegionEntity' then cast (1 as bit) else cast (0 as bit) end, + t.Id, t.OwnerId, t.Property, - t.CategoryId, - t.RegionId + t.RegionId, + case when t.Discriminator = N'TphDerivedNavCategoryEntity' then cast (1 as bit) else cast (0 as bit) end, + t.CategoryId from TphDerivedNavBaseEntities as t order by t.Property } diff --git a/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_mixed_with_base_scalars.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_mixed_with_base_scalars.verified.txt index a0034bc74..899317c4e 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_mixed_with_base_scalars.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_mixed_with_base_scalars.verified.txt @@ -23,23 +23,26 @@ }, sql: { Text: -select t.Id, - t.Discriminator, +select case when t.Discriminator = N'TphDerivedNavRegionEntity' then cast (1 as bit) else cast (0 as bit) end, + t.Id, t.OwnerId, t.Property, - t.CategoryId, + case when r.Id is null then cast (1 as bit) else cast (0 as bit) end, + r.Id, + r.Name, t.RegionId, + case when t.Discriminator = N'TphDerivedNavCategoryEntity' then cast (1 as bit) else cast (0 as bit) end, + case when c.Id is null then cast (1 as bit) else cast (0 as bit) end, c.Id, c.Name, - r.Id, - r.Name + t.CategoryId from TphDerivedNavBaseEntities as t - left outer join - CategoryEntities as c - on t.CategoryId = c.Id left outer join RegionEntities as r on t.RegionId = r.Id + left outer join + CategoryEntities as c + on t.CategoryId = c.Id order by t.Property } } \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_via_inline_fragments.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_via_inline_fragments.verified.txt index 857d496de..8527f2281 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_via_inline_fragments.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_via_inline_fragments.verified.txt @@ -19,23 +19,26 @@ }, sql: { Text: -select t.Id, - t.Discriminator, +select case when t.Discriminator = N'TphDerivedNavRegionEntity' then cast (1 as bit) else cast (0 as bit) end, + t.Id, t.OwnerId, t.Property, - t.CategoryId, + case when r.Id is null then cast (1 as bit) else cast (0 as bit) end, + r.Id, + r.Name, t.RegionId, + case when t.Discriminator = N'TphDerivedNavCategoryEntity' then cast (1 as bit) else cast (0 as bit) end, + case when c.Id is null then cast (1 as bit) else cast (0 as bit) end, c.Id, c.Name, - r.Id, - r.Name + t.CategoryId from TphDerivedNavBaseEntities as t - left outer join - CategoryEntities as c - on t.CategoryId = c.Id left outer join RegionEntities as r on t.RegionId = r.Id + left outer join + CategoryEntities as c + on t.CategoryId = c.Id order by t.Property } } \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_via_inline_fragments_single.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_via_inline_fragments_single.verified.txt index f04dcb532..73fd10707 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_via_inline_fragments_single.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Tph_derived_navigation_via_inline_fragments_single.verified.txt @@ -11,14 +11,16 @@ }, sql: { Text: -select top (2) t.Id, - t.Discriminator, +select top (2) case when t.Discriminator = N'TphDerivedNavRegionEntity' then cast (1 as bit) else cast (0 as bit) end, + t.Id, t.OwnerId, t.Property, - t.CategoryId, t.RegionId, + case when t.Discriminator = N'TphDerivedNavCategoryEntity' then cast (1 as bit) else cast (0 as bit) end, + case when c.Id is null then cast (1 as bit) else cast (0 as bit) end, c.Id, - c.Name + c.Name, + t.CategoryId from TphDerivedNavBaseEntities as t left outer join CategoryEntities as c diff --git a/src/Tests/IntegrationTests/IntegrationTests.Unrelated_navigation_to_base_type_is_included.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Unrelated_navigation_to_base_type_is_included.verified.txt index 4c2fce542..f5e882289 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.Unrelated_navigation_to_base_type_is_included.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.Unrelated_navigation_to_base_type_is_included.verified.txt @@ -4,6 +4,7 @@ tphMiddleEntities: [ { property: TheRequest, + summary: Summary of TheRequest, attachments: [ { property: TheAttachment, diff --git a/src/Tests/IntegrationTests/IntegrationTests_abstract_navigation_includes.cs b/src/Tests/IntegrationTests/IntegrationTests_abstract_navigation_includes.cs index 5777505dc..f82729773 100644 --- a/src/Tests/IntegrationTests/IntegrationTests_abstract_navigation_includes.cs +++ b/src/Tests/IntegrationTests/IntegrationTests_abstract_navigation_includes.cs @@ -1,8 +1,8 @@ public partial class IntegrationTests { - // An abstract navigation type cannot be projected, so the navigation is bound whole. The - // navigations requested under it were then never loaded, since nothing added an include for - // them. They now arrive through includes applied alongside the select. + // A navigation to an abstract type is projected as a chain of type tests, and the collection + // under it is bound once to the result of that chain. The navigation used to be bound whole, + // with what was requested under it loaded through includes applied alongside the select. [Fact] public async Task Navigation_under_abstract_navigation_is_loaded() { diff --git a/src/Tests/IntegrationTests/IntegrationTests_abstract_type_include_fallback.cs b/src/Tests/IntegrationTests/IntegrationTests_abstract_type_include_fallback.cs index 0308b5749..562039c8d 100644 --- a/src/Tests/IntegrationTests/IntegrationTests_abstract_type_include_fallback.cs +++ b/src/Tests/IntegrationTests/IntegrationTests_abstract_type_include_fallback.cs @@ -1,7 +1,9 @@ public partial class IntegrationTests { + // An abstract type is projected as a chain of type tests, each creating a concrete derived + // type. It used to be loaded whole through includes. [Fact] - public async Task Abstract_root_type_query_falls_back_to_include() + public async Task Abstract_root_type_query_is_projected() { var derivedEntity = new DerivedEntity { @@ -31,8 +33,58 @@ ... on Derived await RunQuery(database, query, null, null, false, [derivedEntity, child]); } + // A collection selected on a type with derived types is bound once, to whichever type the + // chain of type tests created. Bound inside each type's member init it was joined once per + // concrete type. [Fact] - public async Task Navigation_to_abstract_type_falls_back_to_full_include() + public async Task Abstract_root_type_collection_is_loaded_once() + { + var parent = new DerivedEntity + { + Id = new("00000000-0000-0000-0000-000000000001"), + Property = "Parent1" + }; + var child1 = new DerivedChildEntity + { + Id = new("00000000-0000-0000-0000-000000000002"), + Property = "Child1", + Parent = parent + }; + var child2 = new DerivedChildEntity + { + Id = new("00000000-0000-0000-0000-000000000003"), + Property = "Child2", + Parent = parent + }; + var other = new DerivedWithNavigationEntity + { + Id = new("00000000-0000-0000-0000-000000000004"), + Property = "Parent2" + }; + + var query = + """ + { + baseEntities(orderBy: {property: ascending}) + { + property + childrenFromInterface(orderBy: {property: ascending}) + { + items + { + property + } + } + } + } + """; + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, null, false, [parent, child1, child2, other]); + } + + [Fact] + public async Task Navigation_to_abstract_type_is_projected() { var parent = new DerivedEntity { diff --git a/src/Tests/IntegrationTests/IntegrationTests_include_guard.cs b/src/Tests/IntegrationTests/IntegrationTests_include_guard.cs index 436da27be..9082a2f43 100644 --- a/src/Tests/IntegrationTests/IntegrationTests_include_guard.cs +++ b/src/Tests/IntegrationTests/IntegrationTests_include_guard.cs @@ -3,7 +3,8 @@ public partial class IntegrationTests // The include path skipped every navigation to a type already on the path, or to a base of // one. So a second, unrelated navigation from an attachment back to the root's base type was // never included, and came back null. Only the inverse of the navigation just traversed is - // skipped now, which is the one nested include EF rejects. + // skipped now, which is the one nested include EF rejects. summary is read only, so cannot be + // bound by a select, which is what puts these queries on the includes path. [Fact] public async Task Unrelated_navigation_to_base_type_is_included() { @@ -31,6 +32,7 @@ public async Task Unrelated_navigation_to_base_type_is_included() tphMiddleEntities(where: {property: {equal: "TheRequest"}}) { property + summary attachments { property @@ -69,6 +71,7 @@ public async Task Tph_base_type_include_skipped_with_filter_no_tracking() tphMiddleEntities { property + summary attachments { property diff --git a/src/Tests/IntegrationTests/IntegrationTests_named_type_filter_projection.cs b/src/Tests/IntegrationTests/IntegrationTests_named_type_filter_projection.cs index 603738da2..f85d01bb0 100644 --- a/src/Tests/IntegrationTests/IntegrationTests_named_type_filter_projection.cs +++ b/src/Tests/IntegrationTests/IntegrationTests_named_type_filter_projection.cs @@ -180,8 +180,8 @@ public async Task Filter_projection_with_TPH_inheritance_should_not_select_all_f /// /// Tests that filter projections accessing navigation properties of abstract types - /// do not throw InvalidOperationException. The projection system should fall back to - /// loading the full navigation entity when it cannot project an abstract type. + /// do not throw InvalidOperationException. The abstract navigation is projected as a + /// chain of type tests, each creating a concrete derived type. /// /// This reproduces the scenario where: /// - DerivedChildEntity (like Accommodation) has a navigation to BaseEntity (abstract) diff --git a/src/Tests/IntegrationTests/IntegrationTests_tph_base_type_include.cs b/src/Tests/IntegrationTests/IntegrationTests_tph_base_type_include.cs index b8f6cda8a..23436a44b 100644 --- a/src/Tests/IntegrationTests/IntegrationTests_tph_base_type_include.cs +++ b/src/Tests/IntegrationTests/IntegrationTests_tph_base_type_include.cs @@ -20,7 +20,7 @@ public async Task Tph_base_type_include_skipped_with_filter() }; leaf.Attachments.Add(attachment); - // Query the abstract middle type (TphMiddleEntity) which triggers the includes path. + // summary is read only, so cannot be bound by a select, which triggers the includes path. // The filter on TphAttachmentEntity accesses _.Request.Property, adding a navigation // back to TphRootEntity. Without the skip, this would cause: // .Include("Attachments.Request") -> the inverse of Attachments, a cycle EF rejects @@ -30,6 +30,7 @@ public async Task Tph_base_type_include_skipped_with_filter() tphMiddleEntities { property + summary attachments { property @@ -63,6 +64,7 @@ public async Task Tph_base_type_single_include_skipped_with_filter() tphMiddleEntity(id: "{{leaf.Id}}") { property + summary attachments { property