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 21, 2020
1 parent 139cedd commit 3ffa5e9
Show file tree
Hide file tree
Showing 7 changed files with 73 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 associated 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 @@ -1415,6 +1415,11 @@ public Expression ApplyCollectionJoin(
else
{
var parentIdentifierList = _identifier.Except(_childIdentifiers).ToList();
if (parentIdentifierList.Count == 0)
{
throw new NotSupportedException(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 #20441")]
public override void Collection_correlated_with_keyless_entity_in_predicate_works()
{
base.Collection_correlated_with_keyless_entity_in_predicate_works();

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,20 @@ protected NorthwindKeylessEntitiesQueryRelationalTestBase(TFixture fixture)

protected virtual bool CanExecuteQueryString => false;

[ConditionalFact]
public virtual void Projecting_collection_correlated_with_keyless_entity_throws()
{
using var context = CreateContext();
var message = Assert.Throws<NotSupportedException>(() => context.Set<ProductQuery>().Select(pv => new
{
pv.ProductID,
pv.ProductName,
OrderDetailIds = context.Set<OrderDetail>().Where(od => od.ProductID == pv.ProductID).ToList()
}).OrderBy(x => x.ProductID).Take(2).ToList()).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,20 @@ from pv in grouping.DefaultIfEmpty()

Assert.Equal(830, results.Count);
}

[ConditionalFact]
public virtual void Collection_correlated_with_keyless_entity_in_predicate_works()
{
using var context = CreateContext();

var results = context.Set<ProductQuery>()
.Where(pv => context.Set<OrderDetail>().Where(od => od.ProductID == pv.ProductID).Any())
.Select(pv => new { pv.ProductID, pv.ProductName })
.OrderBy(x => x.ProductID)
.Take(2)
.ToList();

Assert.Equal(2, results.Count);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,22 @@ FROM [Orders] AS [o]
LEFT JOIN [Alphabetical list of products] AS [a] ON [o].[CustomerID] = [a].[CategoryName]");
}

public override void Collection_correlated_with_keyless_entity_in_predicate_works()
{
base.Collection_correlated_with_keyless_entity_in_predicate_works();

AssertSql(
@"@__p_0='2'
SELECT TOP(@__p_0) [p].[ProductID], [p].[ProductName]
FROM [Products] AS [p]
WHERE ([p].[Discontinued] <> CAST(1 AS bit)) AND EXISTS (
SELECT 1
FROM [Order Details] AS [o]
WHERE [o].[ProductID] = [p].[ProductID])
ORDER BY [p].[ProductID]");
}

private void AssertSql(params string[] expected)
=> Fixture.TestSqlLoggerFactory.AssertBaseline(expected);

Expand Down

0 comments on commit 3ffa5e9

Please sign in to comment.