From 5c6319e65210512a91c8c0fc3d906e3518617932 Mon Sep 17 00:00:00 2001 From: maumar Date: Tue, 21 Jul 2020 15:43:31 -0700 Subject: [PATCH] Fix to #21607 - Incorrect results returned when joining a key-less view to an entity Disabling the scenario when we try to add collection join to keyless entity parent, since we don't have identifying columns to properly bucket the results. Fixes #21607 --- .../Properties/RelationalStrings.Designer.cs | 6 +++++ .../Properties/RelationalStrings.resx | 3 +++ .../Query/SqlExpressions/SelectExpression.cs | 5 ++++ ...NorthwindKeylessEntitiesQueryCosmosTest.cs | 8 +++++++ ...dKeylessEntitiesQueryRelationalTestBase.cs | 23 +++++++++++++++++++ .../NorthwindKeylessEntitiesQueryTestBase.cs | 13 +++++++++++ .../Query/NorthwindQueryFixtureBase.cs | 1 + .../TestModels/Northwind/NorthwindData.cs | 16 +++++++++++++ ...thwindKeylessEntitiesQuerySqlServerTest.cs | 18 +++++++++++++++ 9 files changed, 93 insertions(+) diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 1809001e590..ceb9577ebed 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -997,6 +997,12 @@ public static string TableValuedFunctionNonTPH([CanBeNull] object dbFunction, [C GetString("TableValuedFunctionNonTPH", nameof(dbFunction), nameof(entityType)), dbFunction, entityType); + /// + /// Projecting collection correlated with keyless entity is not supported. + /// + public static string ProjectingCollectionOnKeylessEntityNotSupported + => GetString("ProjectingCollectionOnKeylessEntityNotSupported"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 579cd72c327..f4913f27845 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -700,6 +700,9 @@ Sequence contains no elements. + + Projecting collection correlated with keyless entity is not supported. + An error occurred while the batch executor was rolling back the transaction to a savepoint, after an exception occured. Debug RelationalEventId.BatchExecutorFailedToRollbackToSavepoint diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index bc98b0a1419..b72284c4c9f 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -1423,6 +1423,11 @@ public Expression ApplyCollectionJoin( else { var parentIdentifierList = _identifier.Except(_childIdentifiers).ToList(); + if (parentIdentifierList.Count == 0) + { + throw new InvalidOperationException(RelationalStrings.ProjectingCollectionOnKeylessEntityNotSupported); + } + var (parentIdentifier, parentIdentifierValueComparers) = GetIdentifierAccessor(parentIdentifierList); var (outerIdentifier, outerIdentifierValueComparers) = GetIdentifierAccessor(_identifier); var innerClientEval = innerSelectExpression.Projection.Count > 0; diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindKeylessEntitiesQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindKeylessEntitiesQueryCosmosTest.cs index 6a7f1f96f6a..07e4c1d2c1c 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindKeylessEntitiesQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/NorthwindKeylessEntitiesQueryCosmosTest.cs @@ -137,6 +137,14 @@ public override async Task KeylesEntity_groupby(bool async) AssertSql(@""); } + [ConditionalTheory(Skip = "Issue #17246")] + public override async Task Collection_correlated_with_keyless_entity_in_predicate_works(bool async) + { + await base.Collection_correlated_with_keyless_entity_in_predicate_works(async); + + AssertSql(@""); + } + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected); diff --git a/test/EFCore.Relational.Specification.Tests/Query/NorthwindKeylessEntitiesQueryRelationalTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/NorthwindKeylessEntitiesQueryRelationalTestBase.cs index d33f91b0f55..ba4297bf27b 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/NorthwindKeylessEntitiesQueryRelationalTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/NorthwindKeylessEntitiesQueryRelationalTestBase.cs @@ -1,7 +1,13 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.TestModels.Northwind; using Microsoft.EntityFrameworkCore.TestUtilities; +using Xunit; namespace Microsoft.EntityFrameworkCore.Query { @@ -15,6 +21,23 @@ protected NorthwindKeylessEntitiesQueryRelationalTestBase(TFixture fixture) protected virtual bool CanExecuteQueryString => false; + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task Projecting_collection_correlated_with_keyless_entity_throws(bool async) + { + var message = (await Assert.ThrowsAsync( + () => AssertQuery( + async, + ss => ss.Set().Select(cq => new + { + cq.City, + cq.CompanyName, + OrderDetailIds = ss.Set().Where(c => c.City == cq.City).ToList() + }).OrderBy(x => x.City).Take(2)))).Message; + + Assert.Equal(RelationalStrings.ProjectingCollectionOnKeylessEntityNotSupported, message); + } + protected override QueryAsserter CreateQueryAsserter(TFixture fixture) => new RelationalQueryAsserter(fixture, RewriteExpectedQueryExpression, RewriteServerQueryExpression, canExecuteQueryString: CanExecuteQueryString); } diff --git a/test/EFCore.Specification.Tests/Query/NorthwindKeylessEntitiesQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindKeylessEntitiesQueryTestBase.cs index f278f62b7fe..8d90ac4e520 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindKeylessEntitiesQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindKeylessEntitiesQueryTestBase.cs @@ -199,5 +199,18 @@ from pv in grouping.DefaultIfEmpty() Assert.Equal(830, results.Count); } + + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual Task Collection_correlated_with_keyless_entity_in_predicate_works(bool async) + { + return AssertQuery( + async, + ss => ss.Set() + .Where(cq => ss.Set().Where(c => c.City == cq.City).Any()) + .Select(pv => new { pv.City, pv.ContactName }) + .OrderBy(x => x.ContactName) + .Take(2)); + } } } diff --git a/test/EFCore.Specification.Tests/Query/NorthwindQueryFixtureBase.cs b/test/EFCore.Specification.Tests/Query/NorthwindQueryFixtureBase.cs index e92c2be6b86..55b39f4f98e 100644 --- a/test/EFCore.Specification.Tests/Query/NorthwindQueryFixtureBase.cs +++ b/test/EFCore.Specification.Tests/Query/NorthwindQueryFixtureBase.cs @@ -28,6 +28,7 @@ public IReadOnlyDictionary GetEntitySorters() { typeof(OrderQuery), e => ((OrderQuery)e)?.CustomerID }, { typeof(Employee), e => ((Employee)e)?.EmployeeID }, { typeof(Product), e => ((Product)e)?.ProductID }, + { typeof(ProductQuery), e => ((ProductQuery)e)?.ProductID }, { typeof(OrderDetail), e => (((OrderDetail)e)?.OrderID.ToString(), ((OrderDetail)e)?.ProductID.ToString()) } }.ToDictionary(e => e.Key, e => (object)e.Value); diff --git a/test/EFCore.Specification.Tests/TestModels/Northwind/NorthwindData.cs b/test/EFCore.Specification.Tests/TestModels/Northwind/NorthwindData.cs index 74fbb7291e3..7791c388740 100644 --- a/test/EFCore.Specification.Tests/TestModels/Northwind/NorthwindData.cs +++ b/test/EFCore.Specification.Tests/TestModels/Northwind/NorthwindData.cs @@ -16,6 +16,7 @@ public partial class NorthwindData : ISetSource private readonly CustomerQuery[] _customerQueries; private readonly Employee[] _employees; private readonly Product[] _products; + private readonly ProductQuery[] _productQueries; private readonly Order[] _orders; private readonly OrderQuery[] _orderQueries; private readonly OrderDetail[] _orderDetails; @@ -48,11 +49,21 @@ public NorthwindData() _customerQueries = customerQueries.ToArray(); + var productQueries = new List(); + foreach (var product in _products) { product.OrderDetails = new List(); + + if (!product.Discontinued) + { + productQueries.Add( + new ProductQuery { CategoryName = "Food", ProductID = product.ProductID, ProductName = product.ProductName }); + } } + _productQueries = productQueries.ToArray(); + var orderQueries = new List(); foreach (var order in _orders) @@ -125,6 +136,11 @@ public IQueryable Set() return (IQueryable)_orderQueries.AsQueryable(); } + if (typeof(TEntity) == typeof(ProductQuery)) + { + return (IQueryable)_productQueries.AsQueryable(); + } + throw new InvalidOperationException("Invalid entity type: " + typeof(TEntity)); } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindKeylessEntitiesQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindKeylessEntitiesQuerySqlServerTest.cs index 9e73973c85f..00d9c12a201 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindKeylessEntitiesQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/NorthwindKeylessEntitiesQuerySqlServerTest.cs @@ -168,6 +168,24 @@ FROM [Orders] AS [o] LEFT JOIN [Alphabetical list of products] AS [a] ON [o].[CustomerID] = [a].[CategoryName]"); } + public override async Task Collection_correlated_with_keyless_entity_in_predicate_works(bool async) + { + await base.Collection_correlated_with_keyless_entity_in_predicate_works(async); + + AssertSql( + @"@__p_0='2' + +SELECT TOP(@__p_0) [c].[City], [c].[ContactName] +FROM ( + SELECT [c].[CustomerID] + N'' as [CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region] FROM [Customers] AS [c] +) AS [c] +WHERE EXISTS ( + SELECT 1 + FROM [Customers] AS [c0] + WHERE ([c0].[City] = [c].[City]) OR ([c0].[City] IS NULL AND [c].[City] IS NULL)) +ORDER BY [c].[ContactName]"); + } + private void AssertSql(params string[] expected) => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);