Skip to content

Commit

Permalink
Fix to #8315 - Argument types don't match for queries projecting cond…
Browse files Browse the repository at this point in the history
…itional expression with anonymous type result

Initial problem was that when translating anonymous type we change the expression type from the anonymous type to Expression[].
In case of conditional expression, if the second result is null constant, it's type stays the same, and when we try to update ConditionalExpression, type mismatch is thrown.
However, even if the types are compensated for, we can't translate NewExpression to SQL, apart from it being used in comparison (e.g. in composite key join scenarios)

Fix is to recognize the pattern and force client eval.

Also fixes small bug around alias generation.
  • Loading branch information
maumar committed May 9, 2017
1 parent 68d973c commit e848e3d
Show file tree
Hide file tree
Showing 5 changed files with 227 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,10 @@ protected override Expression VisitBinary(BinaryExpression expression)
var left = Visit(expression.Left);
var right = Visit(expression.Right);

return left != null && right != null
return left != null
&& right != null
&& left.Type != typeof(Expression[])
&& right.Type != typeof(Expression[])
? expression.Update(left, expression.Conversion, right)
: null;
}
Expand Down Expand Up @@ -241,6 +244,13 @@ protected override Expression VisitConditional(ConditionalExpression expression)
&& ifTrue != null
&& ifFalse != null)
{
// 'test ? new { ... } : null' case can't be translated
if (ifTrue.Type == typeof(Expression[])
|| ifFalse.Type == typeof(Expression[]))
{
return null;
}

if (ifTrue.IsComparisonOperation()
|| ifFalse.IsComparisonOperation())
{
Expand Down
5 changes: 3 additions & 2 deletions src/EFCore.Relational/Query/Expressions/SelectExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -613,12 +613,13 @@ private Expression CreateUniqueProjection(Expression expression, string newAlias
}

var currentAlias = GetColumnName(expression);
var uniqueAlias = newAlias ?? currentAlias ?? ColumnAliasPrefix;
var uniqueAliasBase = newAlias ?? currentAlias ?? ColumnAliasPrefix;
var uniqueAlias = uniqueAliasBase;
var counter = 0;

while (_projection.Select(GetColumnName).Any(p => string.Equals(p, uniqueAlias, StringComparison.OrdinalIgnoreCase)))
{
uniqueAlias = currentAlias + counter++;
uniqueAlias = uniqueAliasBase + counter++;
}

var updatedExpression
Expand Down
120 changes: 120 additions & 0 deletions src/EFCore.Specification.Tests/GearsOfWarQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,126 @@ from g2 in context.Gears
}
}

[ConditionalFact]
public virtual void Select_conditional_with_anonymous_type_and_null_constant()
{
using (var context = CreateContext())
{
var query = from g in context.Gears
orderby g.Nickname
select g.LeaderNickname != null ? new { g.HasSoulPatch } : null;

var result = query.ToList();
Assert.Equal(5, result.Count);
Assert.True(result[0].HasSoulPatch);
Assert.False(result[1].HasSoulPatch);
Assert.False(result[2].HasSoulPatch);
Assert.Equal(null, result[3]);
Assert.False(result[4].HasSoulPatch);
}
}

[ConditionalFact]
public virtual void Select_conditional_with_anonymous_types()
{
using (var context = CreateContext())
{
var query = from g in context.Gears
orderby g.Nickname
select g.LeaderNickname != null ? new { Name = g.Nickname } : new { Name = g.FullName };

var result = query.ToList();
Assert.Equal(5, result.Count);
}
}

[ConditionalFact]
public virtual void Where_conditional_with_anonymous_type()
{
using (var context = CreateContext())
{
var query = from g in context.Gears
orderby g.Nickname
where (g.LeaderNickname != null ? new { g.HasSoulPatch } : null) == null
select g.Nickname;

var result = query.ToList();
Assert.Equal(1, result.Count);
Assert.Equal("Marcus", result[0]);
}
}

[ConditionalFact]
public virtual void Select_coalesce_with_anonymous_types()
{
using (var context = CreateContext())
{
var query = from g in context.Gears
orderby g.Nickname
select new { Name = g.LeaderNickname } ?? new { Name = g.FullName };

var result = query.ToList();
Assert.Equal(5, result.Count);
}
}

[ConditionalFact]
public virtual void Where_coalesce_with_anonymous_types()
{
using (var context = CreateContext())
{
var query = from g in context.Gears
where (new { Name = g.LeaderNickname } ?? new { Name = g.FullName }) != null
select g.Nickname;

var result = query.ToList();
Assert.Equal(5, result.Count);
}
}

[ConditionalFact]
public virtual void Where_compare_anonymous_types()
{
using (var context = CreateContext())
{
var query = from g in context.Gears
from o in context.Gears.OfType<Officer>()
where new { Name = g.LeaderNickname, Squad = g.LeaderSquadId, Five = 5 } == new { Name = o.Nickname, Squad = o.SquadId, Five = 5 }
select g.Nickname;

var result = query.ToList();
Assert.Equal(4, result.Count);
}
}

[ConditionalFact]
public virtual void Where_member_access_on_anonymous_type()
{
using (var context = CreateContext())
{
var query = from g in context.Gears
where new { Name = g.LeaderNickname, Squad = g.LeaderSquadId }.Name == "Marcus"
select g.Nickname;

var result = query.ToList();
Assert.Equal(3, result.Count);
}
}

[ConditionalFact]
public virtual void Where_compare_anonymous_types_with_uncorrelated_members()
{
using (var context = CreateContext())
{
var query = from g in context.Gears
where new { Five = 5 } == new { Five = 5 }
select g.Nickname;

var result = query.ToList();
Assert.Equal(0, result.Count);
}
}

[ConditionalFact]
public virtual void Select_Where_Navigation()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1710,7 +1710,7 @@ public override void Order_by_key_of_anonymous_type_projected_navigation_doesnt_
AssertSql(
@"@__p_0: 10
SELECT TOP(@__p_0) [l3.OneToOne_Required_FK_Inverse].[Id], [l3.OneToOne_Required_FK_Inverse].[Date], [l3.OneToOne_Required_FK_Inverse].[Level1_Optional_Id], [l3.OneToOne_Required_FK_Inverse].[Level1_Required_Id], [l3.OneToOne_Required_FK_Inverse].[Name], [l3.OneToOne_Required_FK_Inverse].[OneToMany_Optional_InverseId], [l3.OneToOne_Required_FK_Inverse].[OneToMany_Optional_Self_InverseId], [l3.OneToOne_Required_FK_Inverse].[OneToMany_Required_InverseId], [l3.OneToOne_Required_FK_Inverse].[OneToMany_Required_Self_InverseId], [l3.OneToOne_Required_FK_Inverse].[OneToOne_Optional_PK_InverseId], [l3.OneToOne_Required_FK_Inverse].[OneToOne_Optional_SelfId], [l3].[Name] AS [Name0]
SELECT TOP(@__p_0) [l3.OneToOne_Required_FK_Inverse].[Id], [l3.OneToOne_Required_FK_Inverse].[Date], [l3.OneToOne_Required_FK_Inverse].[Level1_Optional_Id], [l3.OneToOne_Required_FK_Inverse].[Level1_Required_Id], [l3.OneToOne_Required_FK_Inverse].[Name], [l3.OneToOne_Required_FK_Inverse].[OneToMany_Optional_InverseId], [l3.OneToOne_Required_FK_Inverse].[OneToMany_Optional_Self_InverseId], [l3.OneToOne_Required_FK_Inverse].[OneToMany_Required_InverseId], [l3.OneToOne_Required_FK_Inverse].[OneToMany_Required_Self_InverseId], [l3.OneToOne_Required_FK_Inverse].[OneToOne_Optional_PK_InverseId], [l3.OneToOne_Required_FK_Inverse].[OneToOne_Optional_SelfId], [l3].[Name] AS [name0]
FROM [Level3] AS [l3]
INNER JOIN [Level2] AS [l3.OneToOne_Required_FK_Inverse] ON [l3].[Level2_Required_Id] = [l3.OneToOne_Required_FK_Inverse].[Id]
ORDER BY [l3].[Level2_Required_Id]");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,98 @@ CROSS JOIN [Gear] AS [g2]
WHERE [g1].[Discriminator] IN (N'Officer', N'Gear')");
}

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

AssertSql(
@"SELECT CASE
WHEN [g].[LeaderNickname] IS NOT NULL
THEN CAST(1 AS BIT) ELSE CAST(0 AS BIT)
END, [g].[HasSoulPatch]
FROM [Gear] AS [g]
WHERE [g].[Discriminator] IN (N'Officer', N'Gear')
ORDER BY [g].[Nickname]");
}

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

AssertSql(
@"SELECT CASE
WHEN [g].[LeaderNickname] IS NOT NULL
THEN CAST(1 AS BIT) ELSE CAST(0 AS BIT)
END, [g].[Nickname] AS [Name], [g].[FullName] AS [Name0]
FROM [Gear] AS [g]
WHERE [g].[Discriminator] IN (N'Officer', N'Gear')
ORDER BY [Name]");

}

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

AssertSql(
@"SELECT [g].[LeaderNickname], [g].[HasSoulPatch], [g].[Nickname]
FROM [Gear] AS [g]
WHERE [g].[Discriminator] IN (N'Officer', N'Gear')
ORDER BY [g].[Nickname]");
}

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

AssertSql(
@"SELECT [g].[LeaderNickname] AS [Name], [g].[FullName] AS [Name0]
FROM [Gear] AS [g]
WHERE [g].[Discriminator] IN (N'Officer', N'Gear')
ORDER BY [g].[Nickname]");
}

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

AssertSql(
@"SELECT [g].[LeaderNickname], [g].[FullName], [g].[Nickname]
FROM [Gear] AS [g]
WHERE [g].[Discriminator] IN (N'Officer', N'Gear')");
}

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

AssertSql(
@"SELECT [g].[Nickname]
FROM [Gear] AS [g]
CROSS JOIN [Gear] AS [o]
WHERE [g].[Discriminator] IN (N'Officer', N'Gear') AND ((([g].[LeaderNickname] = [o].[Nickname]) AND ([g].[LeaderSquadId] = [o].[SquadId])) AND (5 = 5))");
}

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

AssertSql(
@"SELECT [g].[Nickname]
FROM [Gear] AS [g]
WHERE [g].[Discriminator] IN (N'Officer', N'Gear') AND ([g].[LeaderNickname] = N'Marcus')");
}

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

AssertSql(
@"SELECT [g].[Nickname]
FROM [Gear] AS [g]
WHERE 0 = 1");
}

public override void Select_Where_Navigation_Scalar_Equals_Navigation_Scalar()
{
base.Select_Where_Navigation_Scalar_Equals_Navigation_Scalar();
Expand Down

0 comments on commit e848e3d

Please sign in to comment.