From 21d92de3fc7e05b47f6b7cbbf83cfbe29237a330 Mon Sep 17 00:00:00 2001 From: maumar Date: Tue, 12 May 2020 16:13:18 -0700 Subject: [PATCH] Fix to #20759 - Query: client eval followed by aggregate operation throws 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 --- ...yableMethodTranslatingExpressionVisitor.cs | 37 +++++++++++++++++-- .../Query/GearsOfWarQueryInMemoryTest.cs | 6 +++ .../Query/GearsOfWarQueryTestBase.cs | 25 +++++++++++++ 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs index 14be52c8e65..e71c2dd7adf 100644 --- a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs @@ -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) @@ -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); @@ -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); @@ -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) diff --git a/test/EFCore.InMemory.FunctionalTests/Query/GearsOfWarQueryInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/Query/GearsOfWarQueryInMemoryTest.cs index e763cb39903..cdca991be52 100644 --- a/test/EFCore.InMemory.FunctionalTests/Query/GearsOfWarQueryInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/Query/GearsOfWarQueryInMemoryTest.cs @@ -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); + } } } diff --git a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs index 54f723c541d..6e68b173a31 100644 --- a/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs @@ -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().Select(m => m.Duration.Ticks))); + + await AssertTranslationFailed( + () => AssertAverage( + async, + ss => ss.Set().Select(m => m.Duration.Ticks))); + + await AssertTranslationFailed( + () => AssertMin( + async, + ss => ss.Set().Select(m => m.Duration.Ticks))); + + await AssertTranslationFailed( + () => AssertMax( + async, + ss => ss.Set().Select(m => m.Duration.Ticks))); + } + protected GearsOfWarContext CreateContext() => Fixture.CreateContext(); protected virtual void ClearLog()