Skip to content

Commit

Permalink
Fix to #20759 - Query: client eval followed by aggregate operation th…
Browse files Browse the repository at this point in the history
…rows KeyNotFoundException: The given key 'EmptyProjectionMember' was not present in the dictionary.

Problem is that when client eval happens in the projection before the aggregate operation SelectExpression projection mapping can/will be empty. However when we try to access them using GetMappedProjection we assume that we always find what we want.
Fix is to check that selectExpression.Projection is empty. Otherwise we know client eval happened and we can't translate aggregate.

Fixes #20759
  • Loading branch information
maumar committed May 13, 2020
1 parent 4571d37 commit 21d92de
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,16 @@ protected override ShapedQueryExpression TranslateAverage(ShapedQueryExpression

var newSelector = selector == null
|| selector.Body == selector.Parameters[0]
? selectExpression.GetMappedProjection(new ProjectionMember())
? selectExpression.Projection.Count == 0
? selectExpression.GetMappedProjection(new ProjectionMember())
: null
: RemapLambdaBody(source, selector);

if (newSelector == null)
{
return null;
}

var projection = _sqlTranslator.TranslateAverage(newSelector);
return projection != null
? AggregateResultShaper(source, projection, throwWhenEmpty: true, resultType)
Expand Down Expand Up @@ -683,9 +690,16 @@ protected override ShapedQueryExpression TranslateMax(ShapedQueryExpression sour

var newSelector = selector == null
|| selector.Body == selector.Parameters[0]
? selectExpression.GetMappedProjection(new ProjectionMember())
? selectExpression.Projection.Count == 0
? selectExpression.GetMappedProjection(new ProjectionMember())
: null
: RemapLambdaBody(source, selector);

if (newSelector == null)
{
return null;
}

var projection = _sqlTranslator.TranslateMax(newSelector);

return AggregateResultShaper(source, projection, throwWhenEmpty: true, resultType);
Expand All @@ -701,9 +715,16 @@ protected override ShapedQueryExpression TranslateMin(ShapedQueryExpression sour

var newSelector = selector == null
|| selector.Body == selector.Parameters[0]
? selectExpression.GetMappedProjection(new ProjectionMember())
? selectExpression.Projection.Count == 0
? selectExpression.GetMappedProjection(new ProjectionMember())
: null
: RemapLambdaBody(source, selector);

if (newSelector == null)
{
return null;
}

var projection = _sqlTranslator.TranslateMin(newSelector);

return AggregateResultShaper(source, projection, throwWhenEmpty: true, resultType);
Expand Down Expand Up @@ -1028,11 +1049,19 @@ protected override ShapedQueryExpression TranslateSum(ShapedQueryExpression sour

var selectExpression = (SelectExpression)source.QueryExpression;
selectExpression.PrepareForAggregate();

var newSelector = selector == null
|| selector.Body == selector.Parameters[0]
? selectExpression.GetMappedProjection(new ProjectionMember())
? selectExpression.Projection.Count == 0
? selectExpression.GetMappedProjection(new ProjectionMember())
: null
: RemapLambdaBody(source, selector);

if (newSelector == null)
{
return null;
}

var projection = _sqlTranslator.TranslateSum(newSelector);
return projection != null
? AggregateResultShaper(source, projection, throwWhenEmpty: false, resultType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,11 @@ public override Task Enum_closure_typed_as_underlying_type_generates_correct_par
{
return base.Enum_closure_typed_as_underlying_type_generates_correct_parameter_type(async);
}

[ConditionalTheory(Skip = "issue #17386")]
public override Task Client_eval_followed_by_set_operation_throws_meaningful_exception(bool async)
{
return base.Client_eval_followed_by_set_operation_throws_meaningful_exception(async);
}
}
}
25 changes: 25 additions & 0 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7481,6 +7481,31 @@ public virtual Task Enum_array_contains(bool async)
.Where(w => w.SynergyWith != null && types.Contains(w.SynergyWith.AmmunitionType)));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Client_eval_followed_by_set_operation_throws_meaningful_exception(bool async)
{
await AssertTranslationFailed(
() => AssertSum(
async,
ss => ss.Set<Mission>().Select(m => m.Duration.Ticks)));

await AssertTranslationFailed(
() => AssertAverage(
async,
ss => ss.Set<Mission>().Select(m => m.Duration.Ticks)));

await AssertTranslationFailed(
() => AssertMin(
async,
ss => ss.Set<Mission>().Select(m => m.Duration.Ticks)));

await AssertTranslationFailed(
() => AssertMax(
async,
ss => ss.Set<Mission>().Select(m => m.Duration.Ticks)));
}

protected GearsOfWarContext CreateContext() => Fixture.CreateContext();

protected virtual void ClearLog()
Expand Down

0 comments on commit 21d92de

Please sign in to comment.