[2.1] Query: Null checking with anonymous projection produces invalid…

… SQL

Issue:
The issue here is, we had projection of kind `nav.Id == null ? null : new { nav.Property }`
When it is single property access, we can remove null check since SQL is null safe.
Same is not true for anonymous type since it has no SQL equivalent.
Anonyous type produces Expression[] in sql translation and we tried to print it out in SQL causing exception.

Fix:
Fix is to skip over NewExpression/MemberInitExpression so that if we encounter those, we don't go inside of them.

Resolves #12412

src/EFCore.Relational/Query/ExpressionVisitors/SqlTranslatingExpressionVisitor.cs

@@ -436,6 +436,12 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
                 return base.VisitMethodCall(node);
             }
 
+            // We skip these nodes because test ? null : new { ... } cannot remove null check
+            protected override Expression VisitNew(NewExpression newExpression) => newExpression;
+
+            protected override Expression VisitMemberInit(MemberInitExpression memberInitExpression)
+                => memberInitExpression;
+
             protected override Expression VisitBinary(BinaryExpression node)
             {
                 // not safe to make the optimization due to null semantics

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

@@ -4033,5 +4033,50 @@ public virtual void SelectMany_subquery_with_custom_projection()
             AssertQuery<Level1>(
                 l1s => l1s.OrderBy(l1 => l1.Id).SelectMany(l1 => l1.OneToMany_Optional.Select(l2 => new { l2.Name })).Take(1));
         }
+
+        [ConditionalFact]
+        public virtual void Null_check_in_anonymous_type_projection_should_not_be_removed()
+        {
+            AssertQuery<Level1>(
+                l1s => l1s.OrderBy(l1 => l1.Id).Select(l1 => new
+                {
+                    Level2s = l1.OneToMany_Optional.Select(l2 => new
+                    {
+                        Level3 = l2.OneToOne_Required_FK == null
+                            ? null
+                            : new { l2.OneToOne_Required_FK.Name }
+                    }).ToList()
+                }),
+                assertOrder: true,
+                elementAsserter: (e, a) => {
+                    CollectionAsserter<dynamic>(
+                        elementAsserter: (e1, a1) => Assert.Equal(e1.Level3.Name, a1.Level3.Name))(e.Level2s, a.Level2s);
+                });
+        }
+
+        [ConditionalFact]
+        public virtual void Null_check_in_Dto_projection_should_not_be_removed()
+        {
+            AssertQuery<Level1>(
+                l1s => l1s.OrderBy(l1 => l1.Id).Select(l1 => new
+                {
+                    Level2s = l1.OneToMany_Optional.Select(l2 => new
+                    {
+                        Level3 = l2.OneToOne_Required_FK == null
+                            ? null
+                            : new ProjectedDto<string> { Value = l2.OneToOne_Required_FK.Name }
+                    }).ToList()
+                }),
+                assertOrder: true,
+                elementAsserter: (e, a) => {
+                    CollectionAsserter<dynamic>(
+                        elementAsserter: (e1, a1) => Assert.Equal(e1.Level3.Value, a1.Level3.Value))(e.Level2s, a.Level2s);
+                });
+        }
+
+        private class ProjectedDto<T>
+        {
+            public T Value { get; set; }
+        }
     }
 }

src/EFCore.Specification.Tests/Query/ComplexNavigationsWeakQueryTestBase.cs

@@ -433,6 +433,14 @@ public override void SelectMany_subquery_with_custom_projection()
         {
         }
 
+        public override void Null_check_in_anonymous_type_projection_should_not_be_removed()
+        {
+        }
+
+        public override void Null_check_in_Dto_projection_should_not_be_removed()
+        {
+        }
+
         #endregion
     }
 }

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

@@ -3697,6 +3697,50 @@ FROM [LevelTwo] AS [l2]
 ORDER BY [l1].[Id]");
         }
 
+        public override void Null_check_in_anonymous_type_projection_should_not_be_removed()
+        {
+            base.Null_check_in_anonymous_type_projection_should_not_be_removed();
+
+            AssertSql(
+                @"SELECT [l1].[Id]
+FROM [LevelOne] AS [l1]
+ORDER BY [l1].[Id]",
+                //
+                @"SELECT [t].[Id], CASE
+    WHEN [l2.OneToOne_Required_FK].[Id] IS NULL
+    THEN CAST(1 AS BIT) ELSE CAST(0 AS BIT)
+END, [l2.OneToOne_Required_FK].[Name], [l1.OneToMany_Optional].[OneToMany_Optional_InverseId]
+FROM [LevelTwo] AS [l1.OneToMany_Optional]
+LEFT JOIN [LevelThree] AS [l2.OneToOne_Required_FK] ON [l1.OneToMany_Optional].[Id] = [l2.OneToOne_Required_FK].[Level2_Required_Id]
+INNER JOIN (
+    SELECT [l10].[Id]
+    FROM [LevelOne] AS [l10]
+) AS [t] ON [l1.OneToMany_Optional].[OneToMany_Optional_InverseId] = [t].[Id]
+ORDER BY [t].[Id]");
+        }
+
+        public override void Null_check_in_Dto_projection_should_not_be_removed()
+        {
+            base.Null_check_in_Dto_projection_should_not_be_removed();
+
+            AssertSql(
+                @"SELECT [l1].[Id]
+FROM [LevelOne] AS [l1]
+ORDER BY [l1].[Id]",
+                //
+                @"SELECT [t].[Id], CASE
+    WHEN [l2.OneToOne_Required_FK].[Id] IS NULL
+    THEN CAST(1 AS BIT) ELSE CAST(0 AS BIT)
+END, [l2.OneToOne_Required_FK].[Name] AS [Value], [l1.OneToMany_Optional].[OneToMany_Optional_InverseId]
+FROM [LevelTwo] AS [l1.OneToMany_Optional]
+LEFT JOIN [LevelThree] AS [l2.OneToOne_Required_FK] ON [l1.OneToMany_Optional].[Id] = [l2.OneToOne_Required_FK].[Level2_Required_Id]
+INNER JOIN (
+    SELECT [l10].[Id]
+    FROM [LevelOne] AS [l10]
+) AS [t] ON [l1.OneToMany_Optional].[OneToMany_Optional_InverseId] = [t].[Id]
+ORDER BY [t].[Id]");
+        }
+
         private void AssertSql(params string[] expected)
         {
             Fixture.TestSqlLoggerFactory.AssertBaseline(expected);