Further improvements on null protection/propagation

Fixed cases involving member pushdown. We now remove all converts from nullable to non-nullable (even ones explicitly added by users) so that nullability can propagate to the very top.

src/EFCore.InMemory/Query/Internal/InMemoryExpressionTranslatingExpressionVisitor.cs

@@ -588,6 +588,12 @@ protected override Expression VisitUnary(UnaryExpression unaryExpression)
                 return newOperand;
             }
 
+            if (unaryExpression.NodeType == ExpressionType.Convert
+                && IsConvertedToNullable(newOperand, unaryExpression))
+            {
+                return newOperand;
+            }
+
             var result = (Expression)Expression.MakeUnary(unaryExpression.NodeType, newOperand, unaryExpression.Type);
             if (result is UnaryExpression outerUnary
                 && outerUnary.NodeType == ExpressionType.Convert

src/EFCore.InMemory/Query/Internal/InMemoryProjectionBindingExpressionVisitor.cs

@@ -136,7 +136,7 @@ public override Expression Visit(Expression expression)
 
                     if (translation.Type != expression.Type)
                     {
-                        translation = Expression.Convert(translation, expression.Type);
+                        translation = NullSafeConvert(translation, expression.Type);
                     }
 
                     return new ProjectionBindingExpression(_queryExpression, _queryExpression.AddToProjection(translation), expression.Type);
@@ -151,19 +151,23 @@ public override Expression Visit(Expression expression)
 
                     if (translation.Type != expression.Type)
                     {
-                        translation = Expression.Convert(translation, expression.Type);
+                        translation = NullSafeConvert(translation, expression.Type);
                     }
 
                     _projectionMapping[_projectionMembers.Peek()] = translation;
 
                     return new ProjectionBindingExpression(_queryExpression, _projectionMembers.Peek(), expression.Type);
                 }
-
             }
 
             return base.Visit(expression);
         }
 
+        private Expression NullSafeConvert(Expression expression, Type convertTo)
+            => expression.Type.IsNullableType() && !convertTo.IsNullableType() && expression.Type.UnwrapNullableType() == convertTo
+                ? (Expression)Expression.Coalesce(expression, Expression.Default(convertTo))
+                : Expression.Convert(expression, convertTo);
+
         private CollectionShaperExpression AddCollectionProjection(
             ShapedQueryExpression subquery, INavigation navigation, Type elementType)
             => new CollectionShaperExpression(

test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.Select.cs

@@ -857,5 +857,11 @@ public override Task Client_method_in_projection_requiring_materialization_2(boo
         {
             return base.Client_method_in_projection_requiring_materialization_2(isAsync);
         }
+
+        [ConditionalTheory(Skip = "Issue#17246")]
+        public override Task Project_non_nullable_value_after_FirstOrDefault_on_empty_collection(bool isAsync)
+        {
+            return base.Project_non_nullable_value_after_FirstOrDefault_on_empty_collection(isAsync);
+        }
     }
 }

test/EFCore.Cosmos.FunctionalTests/Query/SimpleQueryCosmosTest.Where.cs

@@ -1882,5 +1882,11 @@ public override Task Where_is_conditional(bool isAsync)
         {
             return base.Where_is_conditional(isAsync);
         }
+
+        [ConditionalTheory(Skip = "Issue#17246")]
+        public override Task Filter_non_nullable_value_after_FirstOrDefault_on_empty_collection(bool isAsync)
+        {
+            return base.Filter_non_nullable_value_after_FirstOrDefault_on_empty_collection(isAsync);
+        }
     }
 }

test/EFCore.InMemory.FunctionalTests/Query/ComplexNavigationsQueryInMemoryTest.cs

@@ -15,19 +15,19 @@ public ComplexNavigationsQueryInMemoryTest(ComplexNavigationsQueryInMemoryFixtur
             //TestLoggerFactory.TestOutputHelper = testOutputHelper;
         }
 
-        [ConditionalTheory(Skip = "issue #16963 TooManyResults")]
+        [ConditionalTheory(Skip = "issue #17531")]
         public override Task SelectMany_with_nested_navigations_and_explicit_DefaultIfEmpty_followed_by_Select_required_navigation_using_different_navs(bool isAsync)
         {
             return base.SelectMany_with_nested_navigations_and_explicit_DefaultIfEmpty_followed_by_Select_required_navigation_using_different_navs(isAsync);
         }
 
-        [ConditionalTheory(Skip = "issue #16963 TooManyResults")]
+        [ConditionalTheory(Skip = "issue #17531")]
         public override Task SelectMany_with_nested_navigation_and_explicit_DefaultIfEmpty(bool isAsync)
         {
             return base.SelectMany_with_nested_navigation_and_explicit_DefaultIfEmpty(isAsync);
         }
 
-        [ConditionalTheory(Skip = "issue #16963 TooManyResults")]
+        [ConditionalTheory(Skip = "issue #17531")]
         public override Task SelectMany_with_nested_navigation_filter_and_explicit_DefaultIfEmpty(bool isAsync)
         {
             return base.SelectMany_with_nested_navigation_filter_and_explicit_DefaultIfEmpty(isAsync);
@@ -39,37 +39,37 @@ public override Task Complex_query_with_optional_navigations_and_client_side_eva
             return base.Complex_query_with_optional_navigations_and_client_side_evaluation(isAsync);
         }
 
-        [ConditionalTheory(Skip = "issue #16963 TooManyResults")]
+        [ConditionalTheory(Skip = "issue #17531")]
         public override Task Project_collection_navigation_nested(bool isAsync)
         {
             return base.Project_collection_navigation_nested(isAsync);
         }
 
-        [ConditionalTheory(Skip = "issue #16963 TooManyResults")]
+        [ConditionalTheory(Skip = "issue #17531")]
         public override Task Project_collection_navigation_nested_anonymous(bool isAsync)
         {
             return base.Project_collection_navigation_nested_anonymous(isAsync);
         }
 
-        [ConditionalTheory(Skip = "issue #16963 TooManyResults")]
+        [ConditionalTheory(Skip = "issue #17531")]
         public override Task Project_collection_navigation_using_ef_property(bool isAsync)
         {
             return base.Project_collection_navigation_using_ef_property(isAsync);
         }
 
-        [ConditionalTheory(Skip = "issue #16963 TooManyResults")]
+        [ConditionalTheory(Skip = "issue #17531")]
         public override Task Project_navigation_and_collection(bool isAsync)
         {
             return base.Project_navigation_and_collection(isAsync);
         }
 
-        [ConditionalTheory(Skip = "issue #16963 TooManyResults")]
+        [ConditionalTheory(Skip = "issue #17531")]
         public override Task SelectMany_nested_navigation_property_optional_and_projection(bool isAsync)
         {
             return base.SelectMany_nested_navigation_property_optional_and_projection(isAsync);
         }
 
-        [ConditionalTheory(Skip = "issue #16963 TooManyResults")]
+        [ConditionalTheory(Skip = "issue #17531")]
         public override Task SelectMany_nested_navigation_property_required(bool isAsync)
         {
             return base.SelectMany_nested_navigation_property_required(isAsync);

test/EFCore.InMemory.FunctionalTests/Query/SimpleQueryInMemoryTest.cs

@@ -2,10 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Dynamic;
-using System.Linq;
-using System.Runtime.InteropServices.WindowsRuntime;
-using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 using Microsoft.EntityFrameworkCore.TestUtilities;
 using Xunit;
@@ -164,16 +160,6 @@ public override Task Projection_when_arithmetic_mixed_subqueries(bool isAsync)
 
         #region SelectMany
 
-        public override Task SelectMany_Joined_DefaultIfEmpty(bool isAsync)
-        {
-            return Task.CompletedTask;
-        }
-
-        public override Task SelectMany_Joined_DefaultIfEmpty2(bool isAsync)
-        {
-            return Task.CompletedTask;
-        }
-
         public override Task SelectMany_correlated_with_outer_3(bool isAsync)
         {
             return Task.CompletedTask;
@@ -186,25 +172,12 @@ public override Task SelectMany_correlated_with_outer_4(bool isAsync)
 
         #endregion
 
-        #region NullableError
-
-        public override Task Project_single_element_from_collection_with_OrderBy_Distinct_and_FirstOrDefault_followed_by_projecting_length(bool isAsync)
-        {
-            return Task.CompletedTask;
-        }
-
+        [ConditionalTheory(Skip = "Issue #17531")]
         public override Task DefaultIfEmpty_in_subquery_nested(bool isAsync)
         {
-            return Task.CompletedTask;
-        }
-
-        public override Task Default_if_empty_top_level_projection(bool isAsync)
-        {
-            return Task.CompletedTask;
+            return base.DefaultIfEmpty_in_subquery_nested(isAsync);
         }
 
-        #endregion
-
         [ConditionalTheory(Skip = "issue #17386")]
         public override Task Where_equals_on_null_nullable_int_types(bool isAsync)
         {

test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs

@@ -4762,6 +4762,29 @@ select Maybe(l1.OneToOne_Optional_FK1, () => l1.OneToOne_Optional_FK1.OneToMany_
                 });
         }
 
+        [ConditionalTheory(Skip = "issue #17531")]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Project_collection_navigation_nested_with_take(bool isAsync)
+        {
+            return AssertQuery<Level1>(
+                isAsync,
+                l1s => from l1 in l1s
+                       select l1.OneToOne_Optional_FK1.OneToMany_Optional2.Take(50),
+                l1s => from l1 in l1s
+                       select Maybe(l1.OneToOne_Optional_FK1, () => l1.OneToOne_Optional_FK1.OneToMany_Optional2.Take(50)),
+                elementSorter: e => e != null ? e.Count : 0,
+                elementAsserter: (e, a) =>
+                {
+                    var actualCollection = new List<Level3>();
+                    foreach (var actualElement in a)
+                    {
+                        actualCollection.Add(actualElement);
+                    }
+
+                    Assert.Equal(((IEnumerable<Level3>)e)?.Count() ?? 0, actualCollection.Count);
+                });
+        }
+
         [ConditionalTheory]
         [MemberData(nameof(IsAsyncData))]
         public virtual Task Project_collection_navigation_using_ef_property(bool isAsync)

test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs

@@ -6694,6 +6694,26 @@ public virtual Task Select_subquery_boolean_empty_with_pushdown(bool isAsync)
                 gs => gs.Select(g => (bool?)null));
         }
 
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Select_subquery_boolean_empty_with_pushdown_without_convert_to_nullable1(bool isAsync)
+        {
+            return AssertQueryScalar<Gear>(
+                isAsync,
+                gs => gs.Select(g => g.Weapons.Where(w => w.Name == "BFG").OrderBy(w => w.Id).FirstOrDefault().IsAutomatic),
+                gs => gs.Select(g => false));
+        }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Select_subquery_boolean_empty_with_pushdown_without_convert_to_nullable2(bool isAsync)
+        {
+            return AssertQueryScalar<Gear>(
+                isAsync,
+                gs => gs.Select(g => g.Weapons.Where(w => w.Name == "BFG").OrderBy(w => w.Id).FirstOrDefault().Id),
+                gs => gs.Select(g => 0));
+        }
+
         [ConditionalTheory]
         [MemberData(nameof(IsAsyncData))]
         public virtual Task Select_subquery_distinct_singleordefault_boolean1(bool isAsync)

test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.Select.cs

@@ -1299,5 +1299,15 @@ from o in os.Where(o => c.CustomerID == o.CustomerID).OrderBy(o => c.City).Take(
                             },
                 entryCount: 268);
         }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Project_non_nullable_value_after_FirstOrDefault_on_empty_collection(bool isAsync)
+        {
+            return AssertQueryScalar<Customer, Order, int>(
+                isAsync,
+                (cs, os) => cs.Select(c => os.Where(o => o.CustomerID == "John Doe").Select(o => o.CustomerID).FirstOrDefault().Length),
+                (cs, os) => cs.Select(c => 0));
+        }
     }
 }

test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.Where.cs

@@ -2098,5 +2098,15 @@ public virtual Task Enclosing_class_const_member_does_not_generate_parameter(boo
         private int SettableProperty { get; set; }
         private int ReadOnlyProperty => 5;
         private const int ConstantProperty = 1;
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Filter_non_nullable_value_after_FirstOrDefault_on_empty_collection(bool isAsync)
+        {
+            return AssertQuery<Customer, Order>(
+                isAsync,
+                (cs, os) => cs.Where(c => os.Where(o => o.CustomerID == "John Doe").Select(o => o.CustomerID).FirstOrDefault().Length == 0),
+                (cs, os) => cs.Where(c => false));
+        }
     }
 }

test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs

@@ -6038,6 +6038,34 @@ FROM [Gears] AS [g]
 WHERE [g].[Discriminator] IN (N'Gear', N'Officer')");
         }
 
+        public override async Task Select_subquery_boolean_empty_with_pushdown_without_convert_to_nullable1(bool isAsync)
+        {
+            await base.Select_subquery_boolean_empty_with_pushdown_without_convert_to_nullable1(isAsync);
+
+            AssertSql(
+                @"SELECT (
+    SELECT TOP(1) [w].[IsAutomatic]
+    FROM [Weapons] AS [w]
+    WHERE (([g].[FullName] = [w].[OwnerFullName]) AND [w].[OwnerFullName] IS NOT NULL) AND (([w].[Name] = N'BFG') AND [w].[Name] IS NOT NULL)
+    ORDER BY [w].[Id])
+FROM [Gears] AS [g]
+WHERE [g].[Discriminator] IN (N'Gear', N'Officer')");
+        }
+
+        public override async Task Select_subquery_boolean_empty_with_pushdown_without_convert_to_nullable2(bool isAsync)
+        {
+            await base.Select_subquery_boolean_empty_with_pushdown_without_convert_to_nullable2(isAsync);
+
+            AssertSql(
+                @"SELECT (
+    SELECT TOP(1) [w].[Id]
+    FROM [Weapons] AS [w]
+    WHERE (([g].[FullName] = [w].[OwnerFullName]) AND [w].[OwnerFullName] IS NOT NULL) AND (([w].[Name] = N'BFG') AND [w].[Name] IS NOT NULL)
+    ORDER BY [w].[Id])
+FROM [Gears] AS [g]
+WHERE [g].[Discriminator] IN (N'Gear', N'Officer')");
+        }
+
         public override async Task Select_subquery_distinct_singleordefault_boolean1(bool isAsync)
         {
             await base.Select_subquery_distinct_singleordefault_boolean1(isAsync);

test/EFCore.SqlServer.FunctionalTests/Query/SimpleQuerySqlServerTest.Where.cs

@@ -1738,5 +1738,33 @@ public override async Task Enclosing_class_const_member_does_not_generate_parame
 FROM [Orders] AS [o]
 WHERE [o].[OrderID] = 1");
         }
+
+        public override async Task Project_non_nullable_value_after_FirstOrDefault_on_empty_collection(bool isAsync)
+        {
+            await base.Project_non_nullable_value_after_FirstOrDefault_on_empty_collection(isAsync);
+
+            AssertSql(
+                @"SELECT (
+    SELECT TOP(1) CAST(LEN([o].[CustomerID]) AS int)
+    FROM [Orders] AS [o]
+    WHERE ([o].[CustomerID] = N'John Doe') AND [o].[CustomerID] IS NOT NULL)
+FROM [Customers] AS [c]");
+        }
+
+        public override async Task Filter_non_nullable_value_after_FirstOrDefault_on_empty_collection(bool isAsync)
+        {
+            await base.Filter_non_nullable_value_after_FirstOrDefault_on_empty_collection(isAsync);
+
+            AssertSql(
+                @"SELECT [c].[CustomerID], [c].[Address], [c].[City], [c].[CompanyName], [c].[ContactName], [c].[ContactTitle], [c].[Country], [c].[Fax], [c].[Phone], [c].[PostalCode], [c].[Region]
+FROM [Customers] AS [c]
+WHERE ((
+    SELECT TOP(1) CAST(LEN([o].[CustomerID]) AS int)
+    FROM [Orders] AS [o]
+    WHERE ([o].[CustomerID] = N'John Doe') AND [o].[CustomerID] IS NOT NULL) = 0) AND (
+    SELECT TOP(1) CAST(LEN([o].[CustomerID]) AS int)
+    FROM [Orders] AS [o]
+    WHERE ([o].[CustomerID] = N'John Doe') AND [o].[CustomerID] IS NOT NULL) IS NOT NULL");
+        }
     }
 }