Skip to content

Commit

Permalink
Fix to #21607 - Incorrect results returned when joining a key-less vi…
Browse files Browse the repository at this point in the history
…ew 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
  • Loading branch information
maumar committed Jul 22, 2020
1 parent d0ec185 commit fadcb58
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 0 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,9 @@
<data name="SequenceContainsNoElements" xml:space="preserve">
<value>Sequence contains no elements.</value>
</data>
<data name="ProjectingCollectionOnKeylessEntityNotSupported" xml:space="preserve">
<value>Projecting collection correlated with keyless entity is not supported.</value>
</data>
<data name="LogBatchExecutorFailedToRollbackToSavepoint" xml:space="preserve">
<value>An error occurred while the batch executor was rolling back the transaction to a savepoint, after an exception occured.</value>
<comment>Debug RelationalEventId.BatchExecutorFailedToRollbackToSavepoint</comment>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1417,6 +1417,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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,14 @@ public override async Task KeylesEntity_groupby(bool async)
AssertSql(@"");
}

[ConditionalFact(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);

Expand Down
Original file line number Diff line number Diff line change
@@ -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
{
Expand All @@ -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<InvalidOperationException>(
() => AssertQuery(
async,
ss => ss.Set<CustomerQuery>().Select(cq => new
{
cq.City,
cq.CompanyName,
OrderDetailIds = ss.Set<Customer>().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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<CustomerQuery>()
.Where(cq => ss.Set<Customer>().Where(c => c.City == cq.City).Any())
.Select(pv => new { pv.City, pv.ContactName })
.OrderBy(x => x.ContactName)
.Take(2));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public IReadOnlyDictionary<Type, object> 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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -48,11 +49,21 @@ public NorthwindData()

_customerQueries = customerQueries.ToArray();

var productQueries = new List<ProductQuery>();

foreach (var product in _products)
{
product.OrderDetails = new List<OrderDetail>();

if (!product.Discontinued)
{
productQueries.Add(
new ProductQuery { CategoryName = "Food", ProductID = product.ProductID, ProductName = product.ProductName });
}
}

_productQueries = productQueries.ToArray();

var orderQueries = new List<OrderQuery>();

foreach (var order in _orders)
Expand Down Expand Up @@ -125,6 +136,11 @@ public IQueryable<TEntity> Set<TEntity>()
return (IQueryable<TEntity>)_orderQueries.AsQueryable();
}

if (typeof(TEntity) == typeof(ProductQuery))
{
return (IQueryable<TEntity>)_productQueries.AsQueryable();
}

throw new InvalidOperationException("Invalid entity type: " + typeof(TEntity));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit fadcb58

Please sign in to comment.