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 return null if we don't find a mapping we are looking for. This then gets propagated into (more user friendly) translation failed exception.
  • Loading branch information
maumar committed May 12, 2020
1 parent 35f8fc6 commit bf4c52c
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,11 @@ protected override ShapedQueryExpression TranslateAverage(ShapedQueryExpression
? selectExpression.GetMappedProjection(new ProjectionMember())
: 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 @@ -686,6 +691,11 @@ protected override ShapedQueryExpression TranslateMax(ShapedQueryExpression sour
? selectExpression.GetMappedProjection(new ProjectionMember())
: RemapLambdaBody(source, selector);

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

var projection = _sqlTranslator.TranslateMax(newSelector);

return AggregateResultShaper(source, projection, throwWhenEmpty: true, resultType);
Expand All @@ -704,6 +714,11 @@ protected override ShapedQueryExpression TranslateMin(ShapedQueryExpression sour
? selectExpression.GetMappedProjection(new ProjectionMember())
: RemapLambdaBody(source, selector);

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

var projection = _sqlTranslator.TranslateMin(newSelector);

return AggregateResultShaper(source, projection, throwWhenEmpty: true, resultType);
Expand Down Expand Up @@ -1033,6 +1048,11 @@ protected override ShapedQueryExpression TranslateSum(ShapedQueryExpression sour
? selectExpression.GetMappedProjection(new ProjectionMember())
: 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 @@ -222,7 +222,9 @@ public Expression GetMappedProjection([NotNull] ProjectionMember projectionMembe
{
Check.NotNull(projectionMember, nameof(projectionMember));

return _projectionMapping[projectionMember];
return _projectionMapping.TryGetValue(projectionMember, out var result)
? result
: null;
}

/// <summary>
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 bf4c52c

Please sign in to comment.