From 0d62111f626bc19e0f90e7b2f621902da0331da8 Mon Sep 17 00:00:00 2001 From: Maurycy Markowski Date: Thu, 23 Mar 2023 13:53:01 -0700 Subject: [PATCH] Fix to #30565 - Query/Json: projecting entity collection along with json collection generates invalid shaper When we project entity collection, the shaper uses result coordinator and generates different shaper. However we process this scenario as if "regular" code path was being executed. Basically, we would generate code for collection projection outside of `resultContext.Values == null` block (this needs to happen for regular collections due to result coordination). For JSON it's not needed, because entire object is loaded from a single row. So we can move the processing into the block and then just refer to it's product in the final projection. We also need to shuffle around the order in which we build the block. We should build expressions and json entities first, then populate resultContext and then build includes (which depend on values in the context). Before we were populating result values before we generated json entities, (which didn't matter because we were not using that info) but now is necessary. Fixes #30565 --- ...sitor.ShaperProcessingExpressionVisitor.cs | 11 ++- .../Query/JsonQueryTestBase.cs | 68 +++++++++++++++++++ .../Query/JsonQuerySqlServerTest.cs | 54 +++++++++++++++ 3 files changed, 131 insertions(+), 2 deletions(-) diff --git a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs index dcd6e6d5b6a..26d9c97efe7 100644 --- a/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs @@ -325,9 +325,9 @@ public LambdaExpression ProcessShaper( var valueArrayInitializationExpression = Expression.Assign( _valuesArrayExpression, Expression.NewArrayInit(typeof(object), _valuesArrayInitializers)); + _expressions.AddRange(_jsonEntityExpressions); _expressions.Add(valueArrayInitializationExpression); _expressions.AddRange(_includeExpressions); - _expressions.AddRange(_jsonEntityExpressions); if (_splitQuery) { @@ -567,7 +567,14 @@ when collectionResultExpression.Navigation is INavigation navigation var visitedShaperResult = Visit(shaperResult); - return visitedShaperResult; + var jsonCollectionParameter = Expression.Parameter(collectionResultExpression.Type); + + _variables.Add(jsonCollectionParameter); + _jsonEntityExpressions.Add(Expression.Assign(jsonCollectionParameter, visitedShaperResult)); + + return CompensateForCollectionMaterialization( + jsonCollectionParameter, + collectionResultExpression.Type); } case ProjectionBindingExpression projectionBindingExpression diff --git a/test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs index d4f08930fb3..96fe159db2d 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/JsonQueryTestBase.cs @@ -1460,6 +1460,74 @@ public virtual Task Json_with_projection_of_multiple_json_references_and_entity_ AssertEqual(e.Reference4, a.Reference4); }); + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Json_with_projection_of_json_collection_leaf_and_entity_collection(bool async) + => AssertQuery( + async, + ss => ss.Set().Select(x => new { x.OwnedReferenceRoot.OwnedReferenceBranch.OwnedCollectionLeaf, x.EntityCollection }).AsNoTracking(), + elementAsserter: (e, a) => + { + AssertCollection(e.OwnedCollectionLeaf, a.OwnedCollectionLeaf, ordered: true); + AssertCollection(e.EntityCollection, a.EntityCollection); + }); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Json_with_projection_of_json_collection_and_entity_collection(bool async) + => AssertQuery( + async, + ss => ss.Set().Select(x => new { x.OwnedCollectionRoot, x.EntityCollection }).AsNoTracking(), + elementAsserter: (e, a) => + { + AssertCollection(e.OwnedCollectionRoot, a.OwnedCollectionRoot, ordered: true); + AssertCollection(e.EntityCollection, a.EntityCollection); + }); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Json_with_projection_of_json_collection_element_and_entity_collection(bool async) + => AssertQuery( + async, + ss => ss.Set().Select(x => new { JsonCollectionElement = x.OwnedCollectionRoot[0], x.EntityReference, x.EntityCollection }).AsNoTracking(), + elementAsserter: (e, a) => + { + AssertEqual(e.JsonCollectionElement, a.JsonCollectionElement); + AssertEqual(e.EntityReference, a.EntityReference); + AssertCollection(e.EntityCollection, a.EntityCollection); + }); + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Json_with_projection_of_mix_of_json_collections_json_references_and_entity_collection(bool async) + => AssertQuery( + async, + ss => ss.Set().Select(x => new + { + Collection1 = x.OwnedReferenceRoot.OwnedReferenceBranch.OwnedCollectionLeaf, + x.EntityReference, + Reference1 = x.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf, + x.EntityCollection, + Reference2 = x.OwnedReferenceRoot.OwnedReferenceBranch.OwnedCollectionLeaf[0], + Collection2 = x.OwnedReferenceRoot.OwnedCollectionBranch, + Collection3 = x.OwnedCollectionRoot, + Reference3 = x.OwnedCollectionRoot[0].OwnedReferenceBranch, + Collection4 = x.OwnedCollectionRoot[0].OwnedCollectionBranch + }).AsNoTracking(), + elementAsserter: (e, a) => + { + AssertCollection(e.Collection1, a.Collection1, ordered: true); + AssertCollection(e.Collection2, a.Collection2, ordered: true); + AssertCollection(e.Collection3, a.Collection3, ordered: true); + AssertCollection(e.Collection4, a.Collection4, ordered: true); + AssertCollection(e.Collection1, a.Collection1, ordered: true); + AssertEqual(e.Reference1, a.Reference1); + AssertEqual(e.Reference2, a.Reference2); + AssertEqual(e.Reference3, a.Reference3); + AssertEqual(e.EntityReference, a.EntityReference); + AssertCollection(e.EntityCollection, a.EntityCollection); + }); + [ConditionalTheory] [MemberData(nameof(IsAsyncData))] public virtual Task Json_all_types_entity_projection(bool async) diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs index d5019533928..90c7a159643 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs @@ -1351,6 +1351,60 @@ ORDER BY [j].[Id] """); } + public override async Task Json_with_projection_of_json_collection_leaf_and_entity_collection(bool async) + { + await base.Json_with_projection_of_json_collection_leaf_and_entity_collection(async); + + AssertSql( +""" +SELECT JSON_QUERY([j].[OwnedReferenceRoot], '$.OwnedReferenceBranch.OwnedCollectionLeaf'), [j].[Id], [j0].[Id], [j0].[Name], [j0].[ParentId] +FROM [JsonEntitiesBasic] AS [j] +LEFT JOIN [JsonEntitiesBasicForCollection] AS [j0] ON [j].[Id] = [j0].[ParentId] +ORDER BY [j].[Id] +"""); + } + + public override async Task Json_with_projection_of_json_collection_and_entity_collection(bool async) + { + await base.Json_with_projection_of_json_collection_and_entity_collection(async); + + AssertSql( +""" +SELECT [j].[OwnedCollectionRoot], [j].[Id], [j0].[Id], [j0].[Name], [j0].[ParentId] +FROM [JsonEntitiesBasic] AS [j] +LEFT JOIN [JsonEntitiesBasicForCollection] AS [j0] ON [j].[Id] = [j0].[ParentId] +ORDER BY [j].[Id] +"""); + } + + public override async Task Json_with_projection_of_json_collection_element_and_entity_collection(bool async) + { + await base.Json_with_projection_of_json_collection_element_and_entity_collection(async); + + AssertSql( +""" +SELECT JSON_QUERY([j].[OwnedCollectionRoot], '$[0]'), [j].[Id], [j0].[Id], [j0].[Name], [j0].[ParentId], [j1].[Id], [j1].[Name], [j1].[ParentId] +FROM [JsonEntitiesBasic] AS [j] +LEFT JOIN [JsonEntitiesBasicForReference] AS [j0] ON [j].[Id] = [j0].[ParentId] +LEFT JOIN [JsonEntitiesBasicForCollection] AS [j1] ON [j].[Id] = [j1].[ParentId] +ORDER BY [j].[Id], [j0].[Id] +"""); + } + + public override async Task Json_with_projection_of_mix_of_json_collections_json_references_and_entity_collection(bool async) + { + await base.Json_with_projection_of_mix_of_json_collections_json_references_and_entity_collection(async); + + AssertSql( +""" +SELECT JSON_QUERY([j].[OwnedReferenceRoot], '$.OwnedReferenceBranch.OwnedCollectionLeaf'), [j].[Id], [j0].[Id], [j0].[Name], [j0].[ParentId], JSON_QUERY([j].[OwnedReferenceRoot], '$.OwnedReferenceBranch.OwnedReferenceLeaf'), [j1].[Id], [j1].[Name], [j1].[ParentId], JSON_QUERY([j].[OwnedReferenceRoot], '$.OwnedCollectionBranch'), [j].[OwnedCollectionRoot] +FROM [JsonEntitiesBasic] AS [j] +LEFT JOIN [JsonEntitiesBasicForReference] AS [j0] ON [j].[Id] = [j0].[ParentId] +LEFT JOIN [JsonEntitiesBasicForCollection] AS [j1] ON [j].[Id] = [j1].[ParentId] +ORDER BY [j].[Id], [j0].[Id] +"""); + } + public override async Task Json_all_types_entity_projection(bool async) { await base.Json_all_types_entity_projection(async);