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 (#21723)

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 authored Jul 22, 2020
1 parent 83eb20f commit 31c3a39
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 @@ -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;
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(@"");
}

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

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 31c3a39

Please sign in to comment.