Skip to content

Commit

Permalink
Fix to #12175 - OrderBy with contains and mapping fails duplicate Que…
Browse files Browse the repository at this point in the history
…ry source - expression association

Problem was that for queries that project constant bool which gets produced as a result of optimization (e.g. Contains on empty collection) we would not add explicit cast to bool, which resulted in value coming back as int.

Fix is to apply optimizations before we perform the check that determines whether the explicit cast is needed - this way the value is already collapsed into constant at this point and explicit cast is correctly applied.
  • Loading branch information
maumar committed Jun 19, 2018
1 parent 0dc07a6 commit e063067
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 9 deletions.
10 changes: 9 additions & 1 deletion src/EFCore.Relational/Query/Sql/DefaultQuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,15 @@ private Expression ApplyNullSemantics(Expression expression)
/// </summary>
/// <param name="projection"> The projection expression. </param>
protected virtual void GenerateProjection([NotNull] Expression projection)
=> Visit(ApplyOptimizations(projection, searchCondition: false));
=> Visit(
ApplyExplicitCastToBoolInProjectionOptimization(
ApplyOptimizations(projection, searchCondition: false)));

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
protected virtual Expression ApplyExplicitCastToBoolInProjectionOptimization(Expression expression) => expression;

/// <summary>
/// Visit the predicate in SQL WHERE clause
Expand Down
16 changes: 16 additions & 0 deletions src/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5053,6 +5053,22 @@ public virtual void Cast_ordered_subquery_to_base_type_using_typed_ToArray()
elementAsserter: CollectionAsserter<Gear>(e => e.Nickname, (e, a) => Assert.Equal(e.Nickname, a.Nickname)));
}

[ConditionalFact]
public virtual void Correlated_collection_with_complex_order_by_funcletized_to_constant_bool()
{
var nicknames = new List<string>();
AssertQuery<Gear>(
gs => from g in gs
orderby nicknames.Contains(g.Nickname) descending
select new { g.Nickname, Weapons = g.Weapons.Select(w => w.Name).ToList() },
elementSorter: e => e.Nickname,
elementAsserter: (e, a) =>
{
Assert.Equal(e.Nickname, a.Nickname);
CollectionAsserter<string>(ee => ee)(e.Weapons, a.Weapons);
});
}

// Remember to add any new tests to Async version of this test class

protected GearsOfWarContext CreateContext() => Fixture.CreateContext();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,17 +132,16 @@ public override Expression VisitSqlFunction(SqlFunctionExpression sqlFunctionExp
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
protected override void GenerateProjection(Expression projection)
protected override Expression ApplyExplicitCastToBoolInProjectionOptimization(Expression expression)
{
var aliasedProjection = projection as AliasExpression;
var expressionToProcess = aliasedProjection?.Expression ?? projection;
var aliasedProjection = expression as AliasExpression;
var expressionToProcess = aliasedProjection?.Expression ?? expression;

var updatedExpression = ExplicitCastToBool(expressionToProcess);

expressionToProcess = aliasedProjection != null
return aliasedProjection != null
? new AliasExpression(aliasedProjection.Alias, updatedExpression)
: updatedExpression;

base.GenerateProjection(expressionToProcess);
}

private static Expression ExplicitCastToBool(Expression expression)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7414,6 +7414,26 @@ WHERE [c.StationedGears].[Discriminator] IN (N'Officer', N'Gear')
ORDER BY [t].[Name], [c.StationedGears].[Nickname] DESC");
}

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

AssertSql(
@"SELECT [g].[Nickname], [g].[FullName]
FROM [Gears] AS [g]
WHERE [g].[Discriminator] IN (N'Officer', N'Gear')
ORDER BY (SELECT 1) DESC, [g].[Nickname], [g].[SquadId], [g].[FullName]",
//
@"SELECT [t].[c], [t].[Nickname], [t].[SquadId], [t].[FullName], [g.Weapons].[Name], [g.Weapons].[OwnerFullName]
FROM [Weapons] AS [g.Weapons]
INNER JOIN (
SELECT CAST(0 AS bit) AS [c], [g0].[Nickname], [g0].[SquadId], [g0].[FullName]
FROM [Gears] AS [g0]
WHERE [g0].[Discriminator] IN (N'Officer', N'Gear')
) AS [t] ON [g.Weapons].[OwnerFullName] = [t].[FullName]
ORDER BY [t].[c] DESC, [t].[Nickname], [t].[SquadId], [t].[FullName]");
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1495,7 +1495,7 @@ ORDER BY (SELECT 1), [c].[CustomerID]
SELECT [c.Orders].[OrderID], [c.Orders].[CustomerID], [c.Orders].[EmployeeID], [c.Orders].[OrderDate]
FROM [Orders] AS [c.Orders]
INNER JOIN (
SELECT [c0].[CustomerID], 0 AS [c]
SELECT [c0].[CustomerID], CAST(0 AS bit) AS [c]
FROM [Customers] AS [c0]
WHERE [c0].[CustomerID] LIKE N'A' + N'%' AND (LEFT([c0].[CustomerID], LEN(N'A')) = N'A')
ORDER BY [c], [c0].[CustomerID]
Expand All @@ -1522,7 +1522,7 @@ ORDER BY (SELECT 1), [c].[CustomerID]
SELECT [c.Orders].[OrderID], [c.Orders].[CustomerID], [c.Orders].[EmployeeID], [c.Orders].[OrderDate]
FROM [Orders] AS [c.Orders]
INNER JOIN (
SELECT [c0].[CustomerID], 1 AS [c]
SELECT [c0].[CustomerID], CAST(1 AS bit) AS [c]
FROM [Customers] AS [c0]
WHERE [c0].[CustomerID] LIKE N'A' + N'%' AND (LEFT([c0].[CustomerID], LEN(N'A')) = N'A')
ORDER BY [c], [c0].[CustomerID]
Expand Down

0 comments on commit e063067

Please sign in to comment.