From de3ffea06f1dd8c1f5f08637acfb3580db5ca381 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Wed, 19 Jun 2024 17:57:34 +0200 Subject: [PATCH] Stop generating includes in nav expansion for primitive collection properties Fixes #34008 --- ...osmosProjectionBindingExpressionVisitor.cs | 28 +- ...ingExpressionVisitor.ExpressionVisitors.cs | 1 + ...aredPrimitiveCollectionsQueryCosmosTest.cs | 405 ++++++++++++++++++ ...SharedPrimitiveCollectionsQueryTestBase.cs | 37 ++ ...dPrimitiveCollectionsQuerySqlServerTest.cs | 11 + 5 files changed, 462 insertions(+), 20 deletions(-) create mode 100644 test/EFCore.Cosmos.FunctionalTests/Query/NonSharedPrimitiveCollectionsQueryCosmosTest.cs diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs index cfe7205c340..345e5254253 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs @@ -236,8 +236,7 @@ protected override Expression VisitExtension(Expression extensionExpression) return QueryCompilationContext.NotTranslatedExpression; } - if (!(includeExpression.Navigation is INavigation includableNavigation - && includableNavigation.IsEmbedded())) + if (includeExpression.Navigation is not INavigation includableNavigation || !includableNavigation.IsEmbedded()) { throw new InvalidOperationException( CosmosStrings.NonEmbeddedIncludeNotSupported(includeExpression.Navigation)); @@ -459,26 +458,16 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp StructuralTypeShaperExpression? shaperExpression; switch (visitedSource) { - case StructuralTypeShaperExpression shaper: - shaperExpression = shaper; + case StructuralTypeShaperExpression s: + shaperExpression = s; break; - case UnaryExpression unaryExpression: - shaperExpression = unaryExpression.Operand as StructuralTypeShaperExpression; - if (shaperExpression == null - || unaryExpression.NodeType != ExpressionType.Convert) - { - return QueryCompilationContext.NotTranslatedExpression; - } - + case UnaryExpression { NodeType: ExpressionType.Convert, Operand: StructuralTypeShaperExpression s }: + shaperExpression = s; break; - case ParameterExpression parameterExpression: - if (!_collectionShaperMapping.TryGetValue(parameterExpression, out var collectionShaper)) - { - return QueryCompilationContext.NotTranslatedExpression; - } - + case ParameterExpression parameterExpression + when _collectionShaperMapping.TryGetValue(parameterExpression, out var collectionShaper): shaperExpression = (StructuralTypeShaperExpression)collectionShaper.InnerShaper; break; @@ -507,8 +496,7 @@ UnaryExpression unaryExpression navigationProjection = innerEntityProjection.BindMember( memberName, visitedSource.Type, clientEval: true, out var propertyBase); - if (propertyBase is not INavigation projectedNavigation - || !projectedNavigation.IsEmbedded()) + if (propertyBase is not INavigation projectedNavigation || !projectedNavigation.IsEmbedded()) { return QueryCompilationContext.NotTranslatedExpression; } diff --git a/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs b/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs index 6abc3319b0e..565f0505609 100644 --- a/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs +++ b/src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.ExpressionVisitors.cs @@ -564,6 +564,7 @@ protected override Expression VisitExtension(Expression extensionExpression) case MaterializeCollectionNavigationExpression: case IncludeExpression: + case PrimitiveCollectionReference: return extensionExpression; } diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NonSharedPrimitiveCollectionsQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NonSharedPrimitiveCollectionsQueryCosmosTest.cs new file mode 100644 index 00000000000..c2396f72b87 --- /dev/null +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NonSharedPrimitiveCollectionsQueryCosmosTest.cs @@ -0,0 +1,405 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.Query; + +public class NonSharedPrimitiveCollectionsQueryCosmosTest : NonSharedPrimitiveCollectionsQueryTestBase +{ + #region Support for specific element types + + public override async Task Array_of_string() + { + await base.Array_of_string(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = "a")) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_int() + { + await base.Array_of_int(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = 1)) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_long() + { + await base.Array_of_long(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = 1)) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_short() + { + await base.Array_of_short(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = 1)) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_byte() + { + // TODO + await Assert.ThrowsAsync(() => base.Array_of_byte()); + + AssertSql(); + } + + public override async Task Array_of_double() + { + await base.Array_of_double(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = 1.0)) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_float() + { + await base.Array_of_float(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = 1.0)) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_decimal() + { + await base.Array_of_decimal(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = 1.0)) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_DateTime() + { + await base.Array_of_DateTime(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = "2023-01-01T12:30:00")) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_DateTime_with_milliseconds() + { + await base.Array_of_DateTime_with_milliseconds(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = "2023-01-01T12:30:00.123")) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_DateTime_with_microseconds() + { + await base.Array_of_DateTime_with_microseconds(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = "2023-01-01T12:30:00.123456")) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_DateOnly() + { + await base.Array_of_DateOnly(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = "2023-01-01")) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_TimeOnly() + { + await base.Array_of_TimeOnly(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = "12:30:00")) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_TimeOnly_with_milliseconds() + { + await base.Array_of_TimeOnly_with_milliseconds(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = "12:30:00.123")) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_TimeOnly_with_microseconds() + { + await base.Array_of_TimeOnly_with_microseconds(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = "12:30:00.123456")) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_DateTimeOffset() + { + await base.Array_of_DateTimeOffset(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = "2023-01-01T12:30:00+02:00")) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_bool() + { + await base.Array_of_bool(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = true)) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_Guid() + { + await base.Array_of_Guid(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = "dc8c903d-d655-4144-a0fd-358099d40ae1")) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_byte_array() + { + // TODO + await Assert.ThrowsAsync(() => base.Array_of_byte_array()); + + AssertSql(""" +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = "AQI=")) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Array_of_enum() + { + await base.Array_of_enum(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN c["SomeArray"] + WHERE (i = 0)) = 2)) +OFFSET 0 LIMIT 2 +"""); + } + + [ConditionalFact] + public override Task Multidimensional_array_is_not_supported() + => base.Multidimensional_array_is_not_supported(); + + #endregion Support for specific element types + + public override async Task Column_with_custom_converter() + { + await base.Column_with_custom_converter(); + + AssertSql( + """ +@__ints_0='1,2,3' + +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (c["Ints"] = @__ints_0)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Parameter_with_inferred_value_converter() + { + await base.Parameter_with_inferred_value_converter(); + + AssertSql(); + } + + public override async Task Constant_with_inferred_value_converter() + { + // TODO + await Assert.ThrowsAsync(() => base.Constant_with_inferred_value_converter()); + + AssertSql(); + } + + public override async Task Inline_collection_in_query_filter() + { + await base.Inline_collection_in_query_filter(); + + AssertSql( + """ +SELECT c +FROM root c +WHERE ((c["Discriminator"] = "TestEntity") AND (( + SELECT VALUE COUNT(1) + FROM i IN (SELECT VALUE [1, 2, 3]) + WHERE (i > c["Id"])) = 1)) +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Project_collection_from_entity_type_with_owned() + { + await base.Project_collection_from_entity_type_with_owned(); + + AssertSql( + """ +SELECT c["Ints"] +FROM root c +WHERE (c["Discriminator"] = "TestEntityWithOwned") +"""); + } + + [ConditionalFact] + public virtual void Check_all_tests_overridden() + => TestHelpers.AssertAllMethodsOverridden(GetType()); + + protected override ITestStoreFactory TestStoreFactory + => CosmosTestStoreFactory.Instance; + + protected override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder builder) + => base.AddOptions(builder).ConfigureWarnings( + w => w.Ignore(CosmosEventId.NoPartitionKeyDefined)); + + protected TestSqlLoggerFactory TestSqlLoggerFactory + => (TestSqlLoggerFactory)ListLoggerFactory; + + protected void ClearLog() + => TestSqlLoggerFactory.Clear(); + + protected void AssertSql(params string[] expected) + => TestSqlLoggerFactory.AssertBaseline(expected); +} diff --git a/test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs index 80f63e43914..efe6255a46d 100644 --- a/test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs @@ -210,6 +210,43 @@ public virtual async Task Inline_collection_in_query_filter() Assert.Equal(2, result.Id); } + [ConditionalFact] + public virtual async Task Project_collection_from_entity_type_with_owned() + { + var contextFactory = await InitializeAsync( + onModelCreating: mb => mb.Entity( + b => + { + b.Property(b => b.Id).ValueGeneratedNever(); + b.OwnsOne(b => b.Owned); + }), + seed: context => + { + context.AddRange( + new TestEntityWithOwned { Id = 1, Ints = [1, 2], Owned = new Owned { Foo = 0 } }, + new TestEntityWithOwned { Id = 2, Ints = [3, 4], Owned = new Owned { Foo = 1 } }); + return context.SaveChangesAsync(); + }); + + await using var context = contextFactory.CreateContext(); + + var results = await context.Set().Select(t => t.Ints).ToListAsync(); + Assert.True(results.Any(r => r?.SequenceEqual([1, 2]) == true)); + Assert.True(results.Any(r => r?.SequenceEqual([3, 4]) == true)); + } + + private class TestEntityWithOwned + { + public int Id { get; set; } + public int[]? Ints { get; set; } + public Owned Owned { get; set; } = default!; + } + + private class Owned + { + public int Foo { get; set; } + } + /// /// A utility that allows easy testing of querying out arbitrary element types from a primitive collection, provided two distinct /// element values. diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NonSharedPrimitiveCollectionsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NonSharedPrimitiveCollectionsQuerySqlServerTest.cs index d6bfce3f8ff..b446632b715 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NonSharedPrimitiveCollectionsQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NonSharedPrimitiveCollectionsQuerySqlServerTest.cs @@ -768,6 +768,17 @@ WHERE JSON_VALUE([t].[Owned], '$.Strings[1]') = N'bar' """); } + public override async Task Project_collection_from_entity_type_with_owned() + { + await base.Project_collection_from_entity_type_with_owned(); + + AssertSql( + """ +SELECT [t].[Ints] +FROM [TestEntityWithOwned] AS [t] +"""); + } + [ConditionalFact] public virtual async Task Same_parameter_with_different_type_mappings() {