From 36f0e5302279fbefcc5d23f98f0e46340ae2ad20 Mon Sep 17 00:00:00 2001 From: maumar Date: Sat, 15 Oct 2022 01:01:54 -0700 Subject: [PATCH] Fix to #29355 - FromSqlRaw throws Exception when querying all objects that contain a certain string property in a json array column Problem was that were not creating navigation bindings for JSON entities in the SelectExpression ctor which takes TableExpressionBase as argument. This would cause error in SharedTypeEntityExpandingExpressionVisitor which depends on those bindings to be present for entities mapped to JSON. Fix is to use the same logic we use in the "main" SelectExpression ctor. Fixes #29355 --- .../Query/SqlExpressions/SelectExpression.cs | 100 +++++----- .../Query/JsonQuerySqlServerTest.cs | 180 ++++++++++++++++++ 2 files changed, 234 insertions(+), 46 deletions(-) diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index e6c7ddf1997..bf688dac9ec 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -440,52 +440,7 @@ void GenerateNonHierarchyNonSplittingEntityType(ITableBase table, TableExpressio } var entityProjection = new EntityProjectionExpression(entityType, propertyExpressions); - - foreach (var ownedJsonNavigation in GetAllNavigationsInHierarchy(entityType) - .Where( - n => n.ForeignKey.IsOwnership - && n.TargetEntityType.IsMappedToJson() - && n.ForeignKey.PrincipalToDependent == n)) - { - var targetEntityType = ownedJsonNavigation.TargetEntityType; - var jsonColumnName = targetEntityType.GetContainerColumnName()!; - var jsonColumnTypeMapping = targetEntityType.GetContainerColumnTypeMapping()!; - - var jsonColumn = new ConcreteColumnExpression( - jsonColumnName, - tableReferenceExpression, - jsonColumnTypeMapping.ClrType, - jsonColumnTypeMapping, - nullable: !ownedJsonNavigation.ForeignKey.IsRequiredDependent || ownedJsonNavigation.IsCollection); - - // for json collections we need to skip ordinal key (which is always the last one) - // simple copy from parent is safe here, because we only do it at top level - // so there is no danger of multiple keys being synthesized (like we have in multi-level nav chains) - var keyPropertiesMap = new Dictionary(); - var keyProperties = targetEntityType.FindPrimaryKey()!.Properties; - var keyPropertiesCount = ownedJsonNavigation.IsCollection - ? keyProperties.Count - 1 - : keyProperties.Count; - - for (var i = 0; i < keyPropertiesCount; i++) - { - var correspondingParentKeyProperty = ownedJsonNavigation.ForeignKey.PrincipalKey.Properties[i]; - keyPropertiesMap[keyProperties[i]] = propertyExpressions[correspondingParentKeyProperty]; - } - - var entityShaperExpression = new RelationalEntityShaperExpression( - targetEntityType, - new JsonQueryExpression( - targetEntityType, - jsonColumn, - keyPropertiesMap, - ownedJsonNavigation.ClrType, - ownedJsonNavigation.IsCollection), - !ownedJsonNavigation.ForeignKey.IsRequiredDependent); - - entityProjection.AddNavigationBinding(ownedJsonNavigation, entityShaperExpression); - } - + AddJsonNavigationBindings(entityType, entityProjection, propertyExpressions, tableReferenceExpression); _projectionMapping[new ProjectionMember()] = entityProjection; var primaryKey = entityType.FindPrimaryKey(); @@ -522,6 +477,7 @@ internal SelectExpression(IEntityType entityType, TableExpressionBase tableExpre } var entityProjection = new EntityProjectionExpression(entityType, propertyExpressions); + AddJsonNavigationBindings(entityType, entityProjection, propertyExpressions, tableReferenceExpression); _projectionMapping[new ProjectionMember()] = entityProjection; var primaryKey = entityType.FindPrimaryKey(); @@ -534,6 +490,58 @@ internal SelectExpression(IEntityType entityType, TableExpressionBase tableExpre } } + private void AddJsonNavigationBindings( + IEntityType entityType, + EntityProjectionExpression entityProjection, + Dictionary propertyExpressions, + TableReferenceExpression tableReferenceExpression) + { + foreach (var ownedJsonNavigation in GetAllNavigationsInHierarchy(entityType) + .Where( + n => n.ForeignKey.IsOwnership + && n.TargetEntityType.IsMappedToJson() + && n.ForeignKey.PrincipalToDependent == n)) + { + var targetEntityType = ownedJsonNavigation.TargetEntityType; + var jsonColumnName = targetEntityType.GetContainerColumnName()!; + var jsonColumnTypeMapping = targetEntityType.GetContainerColumnTypeMapping()!; + + var jsonColumn = new ConcreteColumnExpression( + jsonColumnName, + tableReferenceExpression, + jsonColumnTypeMapping.ClrType, + jsonColumnTypeMapping, + nullable: !ownedJsonNavigation.ForeignKey.IsRequiredDependent || ownedJsonNavigation.IsCollection); + + // for json collections we need to skip ordinal key (which is always the last one) + // simple copy from parent is safe here, because we only do it at top level + // so there is no danger of multiple keys being synthesized (like we have in multi-level nav chains) + var keyPropertiesMap = new Dictionary(); + var keyProperties = targetEntityType.FindPrimaryKey()!.Properties; + var keyPropertiesCount = ownedJsonNavigation.IsCollection + ? keyProperties.Count - 1 + : keyProperties.Count; + + for (var i = 0; i < keyPropertiesCount; i++) + { + var correspondingParentKeyProperty = ownedJsonNavigation.ForeignKey.PrincipalKey.Properties[i]; + keyPropertiesMap[keyProperties[i]] = propertyExpressions[correspondingParentKeyProperty]; + } + + var entityShaperExpression = new RelationalEntityShaperExpression( + targetEntityType, + new JsonQueryExpression( + targetEntityType, + jsonColumn, + keyPropertiesMap, + ownedJsonNavigation.ClrType, + ownedJsonNavigation.IsCollection), + !ownedJsonNavigation.ForeignKey.IsRequiredDependent); + + entityProjection.AddNavigationBinding(ownedJsonNavigation, entityShaperExpression); + } + } + /// /// The list of tags applied to this . /// diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs index ff149a2e764..e2610de0ca7 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/JsonQuerySqlServerTest.cs @@ -1,6 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.Data.SqlClient; +using Microsoft.EntityFrameworkCore.TestModels.JsonQuery; + namespace Microsoft.EntityFrameworkCore.Query; public class JsonQuerySqlServerTest : JsonQueryTestBase @@ -838,6 +841,183 @@ FROM [JsonEntitiesAllTypes] AS [j] """); } + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task FromSql_on_entity_with_json_basic(bool async) + { + await AssertQuery( + async, + ss => ((DbSet)ss.Set()).FromSqlRaw( + Fixture.TestStore.NormalizeDelimitersInRawString("SELECT * FROM [JsonEntitiesBasic] AS j")), + ss => ss.Set(), + entryCount: 40); + + AssertSql( +""" +SELECT [m].[Id], [m].[EntityBasicId], [m].[Name], JSON_QUERY([m].[OwnedCollectionRoot],'$'), JSON_QUERY([m].[OwnedReferenceRoot],'$') +FROM ( + SELECT * FROM "JsonEntitiesBasic" AS j +) AS [m] +"""); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task FromSqlInterpolated_on_entity_with_json_with_predicate(bool async) + { + var parameter = new SqlParameter { ParameterName = "prm", Value = 1 }; + await AssertQuery( + async, + ss => ((DbSet)ss.Set()).FromSql( + Fixture.TestStore.NormalizeDelimitersInInterpolatedString($"SELECT * FROM [JsonEntitiesBasic] AS j WHERE [j].[Id] = {parameter}")), + ss => ss.Set(), + entryCount: 40); + + AssertSql( +""" +prm='1' + +SELECT [m].[Id], [m].[EntityBasicId], [m].[Name], JSON_QUERY([m].[OwnedCollectionRoot],'$'), JSON_QUERY([m].[OwnedReferenceRoot],'$') +FROM ( + SELECT * FROM "JsonEntitiesBasic" AS j WHERE "j"."Id" = @prm +) AS [m] +"""); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task FromSql_on_entity_with_json_project_json_reference(bool async) + { + await AssertQuery( + async, + ss => ((DbSet)ss.Set()).FromSqlRaw( + Fixture.TestStore.NormalizeDelimitersInRawString("SELECT * FROM [JsonEntitiesBasic] AS j")) + .AsNoTracking() + .Select(x => x.OwnedReferenceRoot.OwnedReferenceBranch), + ss => ss.Set().Select(x => x.OwnedReferenceRoot.OwnedReferenceBranch)); + + AssertSql( +""" +SELECT JSON_QUERY([m].[OwnedReferenceRoot],'$.OwnedReferenceBranch'), [m].[Id] +FROM ( + SELECT * FROM "JsonEntitiesBasic" AS j +) AS [m] +"""); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task FromSql_on_entity_with_json_project_json_collection(bool async) + { + await AssertQuery( + async, + ss => ((DbSet)ss.Set()).FromSqlRaw( + Fixture.TestStore.NormalizeDelimitersInRawString("SELECT * FROM [JsonEntitiesBasic] AS j")) + .AsNoTracking() + .Select(x => x.OwnedReferenceRoot.OwnedCollectionBranch), + ss => ss.Set().Select(x => x.OwnedReferenceRoot.OwnedCollectionBranch), + elementAsserter: (e, a) => AssertCollection(e, a, elementSorter: ee => (ee.Date, ee.Enum, ee.Fraction))); + + AssertSql( +""" +SELECT JSON_QUERY([m].[OwnedReferenceRoot],'$.OwnedCollectionBranch'), [m].[Id] +FROM ( + SELECT * FROM "JsonEntitiesBasic" AS j +) AS [m] +"""); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task FromSql_on_entity_with_json_inheritance_on_base(bool async) + { + await AssertQuery( + async, + ss => ((DbSet)ss.Set()).FromSqlRaw( + Fixture.TestStore.NormalizeDelimitersInRawString("SELECT * FROM [JsonEntitiesInheritance] AS j")), + ss => ss.Set(), + entryCount: 38); + + AssertSql( +""" +SELECT [m].[Id], [m].[Discriminator], [m].[Name], [m].[Fraction], JSON_QUERY([m].[CollectionOnBase],'$'), JSON_QUERY([m].[ReferenceOnBase],'$'), JSON_QUERY([m].[CollectionOnDerived],'$'), JSON_QUERY([m].[ReferenceOnDerived],'$') +FROM ( + SELECT * FROM "JsonEntitiesInheritance" AS j +) AS [m] +"""); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task FromSql_on_entity_with_json_inheritance_on_derived(bool async) + { + await AssertQuery( + async, + ss => ((DbSet)ss.Set()).FromSqlRaw( + Fixture.TestStore.NormalizeDelimitersInRawString("SELECT * FROM [JsonEntitiesInheritance] AS j")), + ss => ss.Set(), + entryCount: 25); + + AssertSql( +""" +SELECT [m].[Id], [m].[Discriminator], [m].[Name], [m].[Fraction], JSON_QUERY([m].[CollectionOnBase],'$'), JSON_QUERY([m].[ReferenceOnBase],'$'), JSON_QUERY([m].[CollectionOnDerived],'$'), JSON_QUERY([m].[ReferenceOnDerived],'$') +FROM ( + SELECT * FROM "JsonEntitiesInheritance" AS j +) AS [m] +WHERE [m].[Discriminator] = N'JsonEntityInheritanceDerived' +"""); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task FromSql_on_entity_with_json_inheritance_project_reference_on_base(bool async) + { + await AssertQuery( + async, + ss => ((DbSet)ss.Set()).FromSqlRaw( + Fixture.TestStore.NormalizeDelimitersInRawString("SELECT * FROM [JsonEntitiesInheritance] AS j")) + .AsNoTracking() + .OrderBy(x => x.Id) + .Select(x => x.ReferenceOnBase), + ss => ss.Set().OrderBy(x => x.Id).Select(x => x.ReferenceOnBase), + assertOrder: true); + + AssertSql( +""" +SELECT JSON_QUERY([m].[ReferenceOnBase],'$'), [m].[Id] +FROM ( + SELECT * FROM "JsonEntitiesInheritance" AS j +) AS [m] +ORDER BY [m].[Id] +"""); + } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task FromSql_on_entity_with_json_inheritance_project_reference_on_derived(bool async) + { + await AssertQuery( + async, + ss => ((DbSet)ss.Set()).FromSqlRaw( + Fixture.TestStore.NormalizeDelimitersInRawString("SELECT * FROM [JsonEntitiesInheritance] AS j")) + .AsNoTracking() + .OrderBy(x => x.Id) + .Select(x => x.CollectionOnDerived), + ss => ss.Set().OrderBy(x => x.Id).Select(x => x.CollectionOnDerived), + elementAsserter: (e, a) => AssertCollection(e, a, elementSorter: ee => (ee.Date, ee.Enum, ee.Fraction)), + assertOrder: true); + + AssertSql( +""" +SELECT JSON_QUERY([m].[CollectionOnDerived],'$'), [m].[Id] +FROM ( + SELECT * FROM "JsonEntitiesInheritance" AS j +) AS [m] +WHERE [m].[Discriminator] = N'JsonEntityInheritanceDerived' +ORDER BY [m].[Id] +"""); + } + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); }