Return null when the type is nullable for Cosmos Max/Min/Average

Fixes #35094

This was a regression resulting from the major Cosmos query refactoring that happened in EF9. In EF8, the functions Min, Max, and Average would return null if the return type was nullable or was cast to a nullable when the collection is empty. In EF9, this started throwing, which is correct for non-nullable types, but a regression for nullable types.

Author: ajcvickers

src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs

@@ -444,7 +444,25 @@ private ShapedQueryExpression CreateShapedQueryExpression(SelectExpression selec
     /// </summary>
     protected override ShapedQueryExpression? TranslateAverage(ShapedQueryExpression source, LambdaExpression? selector, Type resultType)
     {
-        var selectExpression = (SelectExpression)source.QueryExpression;
+        var updatedSource = TranslateAggregateCommon(source, selector, resultType, out var selectExpression);
+        if (updatedSource == null)
+        {
+            return null;
+        }
+
+        var projection = (SqlExpression)selectExpression.GetMappedProjection(new ProjectionMember());
+        projection = _sqlExpressionFactory.Function("AVG", new[] { projection }, resultType, _typeMappingSource.FindMapping(resultType));
+
+        return AggregateResultShaper(updatedSource, projection, resultType);
+    }
+
+    private ShapedQueryExpression? TranslateAggregateCommon(
+        ShapedQueryExpression source,
+        LambdaExpression? selector,
+        Type resultType,
+        out SelectExpression selectExpression)
+    {
+        selectExpression = (SelectExpression)source.QueryExpression;
         if (selectExpression.IsDistinct
             || selectExpression.Limit != null
             || selectExpression.Offset != null)
@@ -457,10 +475,13 @@ private ShapedQueryExpression CreateShapedQueryExpression(SelectExpression selec
             source = TranslateSelect(source, selector);
         }
 
-        var projection = (SqlExpression)selectExpression.GetMappedProjection(new ProjectionMember());
-        projection = _sqlExpressionFactory.Function("AVG", new[] { projection }, resultType, _typeMappingSource.FindMapping(resultType));
+        if (resultType.IsNullableType())
+        {
+            // For nullable types, we want to return null from Max, Min, and Average, rather than throwing. See Issue #35094.
+            source = source.UpdateResultCardinality(ResultCardinality.SingleOrDefault);
+        }
 
-        return AggregateResultShaper(source, projection, throwOnNullResult: true, resultType);
+        return source;
     }
 
     /// <summary>
@@ -842,24 +863,17 @@ protected override ShapedQueryExpression TranslateCast(ShapedQueryExpression sou
     /// </summary>
     protected override ShapedQueryExpression? TranslateMax(ShapedQueryExpression source, LambdaExpression? selector, Type resultType)
     {
-        var selectExpression = (SelectExpression)source.QueryExpression;
-        if (selectExpression.IsDistinct
-            || selectExpression.Limit != null
-            || selectExpression.Offset != null)
+        var updatedSource = TranslateAggregateCommon(source, selector, resultType, out var selectExpression);
+        if (updatedSource == null)
         {
             return null;
         }
 
-        if (selector != null)
-        {
-            source = TranslateSelect(source, selector);
-        }
-
         var projection = (SqlExpression)selectExpression.GetMappedProjection(new ProjectionMember());
 
         projection = _sqlExpressionFactory.Function("MAX", new[] { projection }, resultType, projection.TypeMapping);
 
-        return AggregateResultShaper(source, projection, throwOnNullResult: true, resultType);
+        return AggregateResultShaper(updatedSource, projection, resultType);
     }
 
     /// <summary>
@@ -870,24 +884,17 @@ protected override ShapedQueryExpression TranslateCast(ShapedQueryExpression sou
     /// </summary>
     protected override ShapedQueryExpression? TranslateMin(ShapedQueryExpression source, LambdaExpression? selector, Type resultType)
     {
-        var selectExpression = (SelectExpression)source.QueryExpression;
-        if (selectExpression.IsDistinct
-            || selectExpression.Limit != null
-            || selectExpression.Offset != null)
+        var updatedSource = TranslateAggregateCommon(source, selector, resultType, out var selectExpression);
+        if (updatedSource == null)
         {
             return null;
         }
 
-        if (selector != null)
-        {
-            source = TranslateSelect(source, selector);
-        }
-
         var projection = (SqlExpression)selectExpression.GetMappedProjection(new ProjectionMember());
 
         projection = _sqlExpressionFactory.Function("MIN", new[] { projection }, resultType, projection.TypeMapping);
 
-        return AggregateResultShaper(source, projection, throwOnNullResult: true, resultType);
+        return AggregateResultShaper(updatedSource, projection, resultType);
     }
 
     /// <summary>
@@ -1241,7 +1248,7 @@ protected override ShapedQueryExpression TranslateSelect(ShapedQueryExpression s
 
         projection = _sqlExpressionFactory.Function("SUM", new[] { projection }, serverOutputType, projection.TypeMapping);
 
-        return AggregateResultShaper(source, projection, throwOnNullResult: false, resultType);
+        return AggregateResultShaper(source, projection, resultType);
     }
 
     /// <summary>
@@ -1695,7 +1702,6 @@ private Expression RemapLambdaBody(ShapedQueryExpression shapedQueryExpression,
     private static ShapedQueryExpression AggregateResultShaper(
         ShapedQueryExpression source,
         Expression projection,
-        bool throwOnNullResult,
         Type resultType)
     {
         var selectExpression = (SelectExpression)source.QueryExpression;
@@ -1706,29 +1712,7 @@ private static ShapedQueryExpression AggregateResultShaper(
         var nullableResultType = resultType.MakeNullable();
         Expression shaper = new ProjectionBindingExpression(source.QueryExpression, new ProjectionMember(), nullableResultType);
 
-        if (throwOnNullResult)
-        {
-            var resultVariable = Expression.Variable(nullableResultType, "result");
-            var returnValueForNull = resultType.IsNullableType()
-                ? (Expression)Expression.Constant(null, resultType)
-                : Expression.Throw(
-                    Expression.New(
-                        typeof(InvalidOperationException).GetConstructors()
-                            .Single(ci => ci.GetParameters().Length == 1),
-                        Expression.Constant(CoreStrings.SequenceContainsNoElements)),
-                    resultType);
-
-            shaper = Expression.Block(
-                new[] { resultVariable },
-                Expression.Assign(resultVariable, shaper),
-                Expression.Condition(
-                    Expression.Equal(resultVariable, Expression.Default(nullableResultType)),
-                    returnValueForNull,
-                    resultType != resultVariable.Type
-                        ? Expression.Convert(resultVariable, resultType)
-                        : resultVariable));
-        }
-        else if (resultType != shaper.Type)
+        if (resultType != shaper.Type)
         {
             shaper = Expression.Convert(shaper, resultType);
         }

test/EFCore.Cosmos.FunctionalTests/Query/NorthwindAggregateOperatorsQueryCosmosTest.cs

@@ -555,49 +555,33 @@ FROM root c
         }
     }
 
-    public override async Task Average_no_data_nullable(bool async)
-    {
-        // Sync always throws before getting to exception being tested.
-        if (async)
-        {
-            await Fixture.NoSyncTest(
-                async, async a =>
-                {
-                    Assert.Equal(
-                        CoreStrings.SequenceContainsNoElements,
-                        (await Assert.ThrowsAsync<InvalidOperationException>(() => base.Average_no_data_nullable(a))).Message);
+    public override Task Average_no_data_nullable(bool async)
+        => Fixture.NoSyncTest(
+            async, async a =>
+            {
+                await base.Average_no_data_nullable(a);
 
-                    AssertSql(
-                        """
+                AssertSql(
+                    """
 SELECT VALUE AVG(c["SupplierID"])
 FROM root c
 WHERE ((c["$type"] = "Product") AND (c["SupplierID"] = -1))
 """);
-                });
-        }
-    }
+            });
 
-    public override async Task Average_no_data_cast_to_nullable(bool async)
-    {
-        // Sync always throws before getting to exception being tested.
-        if (async)
-        {
-            await Fixture.NoSyncTest(
-                async, async a =>
-                {
-                    Assert.Equal(
-                        CoreStrings.SequenceContainsNoElements,
-                        (await Assert.ThrowsAsync<InvalidOperationException>(() => base.Average_no_data_cast_to_nullable(a))).Message);
+    public override Task Average_no_data_cast_to_nullable(bool async)
+        => Fixture.NoSyncTest(
+            async, async a =>
+            {
+                await base.Average_no_data_cast_to_nullable(a);
 
-                    AssertSql(
-                        """
+                AssertSql(
+                    """
 SELECT VALUE AVG(c["OrderID"])
 FROM root c
 WHERE ((c["$type"] = "Order") AND (c["OrderID"] = -1))
 """);
-                });
-        }
-    }
+            });
 
     public override async Task Min_no_data(bool async)
     {
@@ -647,49 +631,33 @@ public override async Task Max_no_data_subquery(bool async)
         AssertSql();
     }
 
-    public override async Task Max_no_data_nullable(bool async)
-    {
-        // Sync always throws before getting to exception being tested.
-        if (async)
-        {
-            await Fixture.NoSyncTest(
-                async, async a =>
-                {
-                    Assert.Equal(
-                        CoreStrings.SequenceContainsNoElements,
-                        (await Assert.ThrowsAsync<InvalidOperationException>(() => base.Max_no_data_nullable(a))).Message);
+    public override Task Max_no_data_nullable(bool async)
+        => Fixture.NoSyncTest(
+            async, async a =>
+            {
+                await base.Max_no_data_nullable(a);
 
-                    AssertSql(
-                        """
+                AssertSql(
+                    """
 SELECT VALUE MAX(c["SupplierID"])
 FROM root c
 WHERE ((c["$type"] = "Product") AND (c["SupplierID"] = -1))
 """);
-                });
-        }
-    }
+            });
 
-    public override async Task Max_no_data_cast_to_nullable(bool async)
-    {
-        // Sync always throws before getting to exception being tested.
-        if (async)
-        {
-            await Fixture.NoSyncTest(
-                async, async a =>
-                {
-                    Assert.Equal(
-                        CoreStrings.SequenceContainsNoElements,
-                        (await Assert.ThrowsAsync<InvalidOperationException>(() => base.Max_no_data_cast_to_nullable(a))).Message);
+    public override Task Max_no_data_cast_to_nullable(bool async)
+        => Fixture.NoSyncTest(
+            async, async a =>
+            {
+                await base.Max_no_data_cast_to_nullable(a);
 
-                    AssertSql(
-                        """
+                AssertSql(
+                    """
 SELECT VALUE MAX(c["OrderID"])
 FROM root c
 WHERE ((c["$type"] = "Order") AND (c["OrderID"] = -1))
 """);
-                });
-        }
-    }
+            });
 
     public override async Task Min_no_data_subquery(bool async)
     {
@@ -868,49 +836,33 @@ FROM root c
 """);
             });
 
-    public override async Task Min_no_data_nullable(bool async)
-    {
-        // Sync always throws before getting to exception being tested.
-        if (async)
-        {
-            await Fixture.NoSyncTest(
-                async, async a =>
-                {
-                    Assert.Equal(
-                        CoreStrings.SequenceContainsNoElements,
-                        (await Assert.ThrowsAsync<InvalidOperationException>(() => base.Min_no_data_nullable(a))).Message);
+    public override  Task Min_no_data_nullable(bool async)
+        => Fixture.NoSyncTest(
+            async, async a =>
+            {
+                await base.Min_no_data_nullable(a);
 
-                    AssertSql(
-                        """
+                AssertSql(
+                    """
 SELECT VALUE MIN(c["SupplierID"])
 FROM root c
 WHERE ((c["$type"] = "Product") AND (c["SupplierID"] = -1))
 """);
-                });
-        }
-    }
+            });
 
-    public override async Task Min_no_data_cast_to_nullable(bool async)
-    {
-        // Sync always throws before getting to exception being tested.
-        if (async)
-        {
-            await Fixture.NoSyncTest(
-                async, async a =>
-                {
-                    Assert.Equal(
-                        CoreStrings.SequenceContainsNoElements,
-                        (await Assert.ThrowsAsync<InvalidOperationException>(() => base.Min_no_data_cast_to_nullable(a))).Message);
+    public override Task Min_no_data_cast_to_nullable(bool async)
+        => Fixture.NoSyncTest(
+            async, async a =>
+            {
+                await base.Min_no_data_cast_to_nullable(a);
 
-                    AssertSql(
-                        """
+                AssertSql(
+                    """
 SELECT VALUE MIN(c["OrderID"])
 FROM root c
 WHERE ((c["$type"] = "Order") AND (c["OrderID"] = -1))
 """);
-                });
-        }
-    }
+            });
 
     public override Task Min_with_coalesce(bool async)
         => Fixture.NoSyncTest(