Skip to content

Commit

Permalink
Query: Update OrderBy list properly when updating projection for alia…
Browse files Browse the repository at this point in the history
…s-ing

Issue:
Since the projection has same property repeated twice in the DTO, we would add alias from member name. We match projection based on unwrapped alias but we don't do that in ordering.
So we updated the projection but we did not update the ordering for new alias.
Fix:
Remember the removed projection and also use that while searching inside order by so we update ordering too and don't end up with incorrect column in order by list

Resolves #12180
  • Loading branch information
smitpatel committed May 31, 2018
1 parent 1ccdbac commit 549bb31
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 7 deletions.
19 changes: 12 additions & 7 deletions src/EFCore.Relational/Query/Expressions/SelectExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ internal void DetachContext()
public virtual TableExpressionBase ProjectStarTable
{
get => _projectStarTable ?? (_tables.Count == 1 ? _tables.Single() : null);
[param: CanBeNull] set => _projectStarTable = value;
[param: CanBeNull]
set => _projectStarTable = value;
}

/// <summary>
Expand Down Expand Up @@ -823,9 +824,11 @@ public virtual void SetProjectionExpression([NotNull] Expression expression)
private Expression CreateUniqueProjection(Expression expression, string newAlias = null)
{
var currentProjectionIndex = FindProjectionIndex(expression);
Expression removedProjection = null;

if (currentProjectionIndex != -1)
{
removedProjection = _projection[currentProjectionIndex];
_projection.RemoveAt(currentProjectionIndex);
}

Expand Down Expand Up @@ -854,7 +857,14 @@ private Expression CreateUniqueProjection(Expression expression, string newAlias
: new AliasExpression(uniqueAlias, updatedExpression);
}

var currentOrderingIndex = _orderBy.FindIndex(e => e.Expression.Equals(expression));
if (currentProjectionIndex != -1)
{
_projection.Insert(currentProjectionIndex, updatedExpression);
}

var currentOrderingIndex = _orderBy.FindIndex(
e => e.Expression.Equals(expression)
|| e.Expression.Equals(removedProjection));
if (currentOrderingIndex != -1)
{
var oldOrdering = _orderBy[currentOrderingIndex];
Expand All @@ -863,11 +873,6 @@ private Expression CreateUniqueProjection(Expression expression, string newAlias
_orderBy.Insert(currentOrderingIndex, new Ordering(updatedExpression, oldOrdering.OrderingDirection));
}

if (currentProjectionIndex != -1)
{
_projection.Insert(currentProjectionIndex, updatedExpression);
}

return updatedExpression;
}

Expand Down
13 changes: 13 additions & 0 deletions src/EFCore.Specification.Tests/Query/SimpleQueryTestBase.Select.cs
Original file line number Diff line number Diff line change
Expand Up @@ -883,5 +883,18 @@ public virtual void Anonymous_projection_AsNoTracking_Selector()
.AsNoTracking() // Just to cause a subquery
.Select(e => e.B));
}

[ConditionalFact]
public virtual void Anonymous_projection_with_repeated_property_being_ordered()
{
AssertQuery<Customer>(
cs => from c in cs
orderby c.CustomerID
select new
{
A = c.CustomerID,
B = c.CustomerID
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -896,5 +896,15 @@ public override void Anonymous_projection_AsNoTracking_Selector()
@"SELECT [o].[CustomerID] AS [A], [o].[OrderDate] AS [B]
FROM [Orders] AS [o]");
}

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

AssertSql(
@"SELECT [c].[CustomerID] AS [B]
FROM [Customers] AS [c]
ORDER BY [B]");
}
}
}

0 comments on commit 549bb31

Please sign in to comment.