Fix to #26344 - SqlNullabilityProcessor and COALESCE with more than two arguments

src/EFCore.Relational/Query/ISqlExpressionFactory.cs

@@ -238,12 +238,12 @@ SqlBinaryExpression Or(
 
         // Other
         /// <summary>
-        ///     Creates a <see cref="SqlBinaryExpression" /> which represents a bitwise OR operation.
+        ///     Creates a <see cref="SqlFunctionExpression" /> which represents a COALESCE operation.
         /// </summary>
         /// <param name="left">The left operand.</param>
         /// <param name="right">The right operand.</param>
         /// <param name="typeMapping">A type mapping to be assigned to the created expression.</param>
-        /// <returns>An expression representing a SQL bitwise OR operation.</returns>
+        /// <returns>An expression representing a SQL COALESCE operation.</returns>
         SqlFunctionExpression Coalesce(
             SqlExpression left,
             SqlExpression right,

src/EFCore.Relational/Query/Internal/SqlExpressionSimplifyingExpressionVisitor.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
@@ -67,7 +68,44 @@ protected override Expression VisitExtension(Expression extensionExpression)
                 return SimplifySqlBinary(sqlBinaryExpression);
             }
 
+            if (extensionExpression is SqlFunctionExpression sqlFunctionExpression
+                && IsCoalesce(sqlFunctionExpression))
+            {
+                var arguments = new List<SqlExpression>();
+                foreach (var argument in sqlFunctionExpression.Arguments!)
+                {
+                    var newArgument = (SqlExpression)Visit(argument);
+                    if (IsCoalesce(newArgument))
+                    {
+                        arguments.AddRange(((SqlFunctionExpression)newArgument).Arguments!);
+                    }
+                    else
+                    {
+                        arguments.Add(newArgument);
+                    }
+                }
+
+                var distinctArguments = arguments.Distinct().ToList();
+
+                return distinctArguments.Count > 1
+                    ? new SqlFunctionExpression(
+                        sqlFunctionExpression.Name,
+                        distinctArguments,
+                        sqlFunctionExpression.IsNullable,
+                        argumentsPropagateNullability: distinctArguments.Select(a => false).ToArray(),
+                        sqlFunctionExpression.Type,
+                        sqlFunctionExpression.TypeMapping)
+                    : distinctArguments[0];
+            }
+
             return base.VisitExtension(extensionExpression);
+
+            static bool IsCoalesce(SqlExpression sqlExpression)
+                => sqlExpression is SqlFunctionExpression sqlFunctionExpression
+                    && sqlFunctionExpression.IsBuiltIn
+                    && sqlFunctionExpression.Instance == null
+                    && string.Equals(sqlFunctionExpression.Name, "COALESCE", StringComparison.OrdinalIgnoreCase)
+                    && sqlFunctionExpression.Arguments?.Count > 1;
         }
 
         private bool IsCompareTo([NotNullWhen(true)] CaseExpression? caseExpression)

src/EFCore.Relational/Query/SqlExpressionFactory.cs

@@ -447,7 +447,7 @@ public virtual SqlFunctionExpression Coalesce(SqlExpression left, SqlExpression
                 "COALESCE",
                 typeMappedArguments,
                 nullable: true,
-                // COALESCE is handled separately since it's only nullable if *both* arguments are null
+                // COALESCE is handled separately since it's only nullable if *all* arguments are null
                 argumentsPropagateNullability: new[] { false, false },
                 resultType,
                 inferredTypeMapping);

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs

@@ -1036,12 +1036,17 @@ protected virtual SqlExpression VisitSqlFunction(
                 && sqlFunctionExpression.Arguments != null
                 && string.Equals(sqlFunctionExpression.Name, "COALESCE", StringComparison.OrdinalIgnoreCase))
             {
-                var left = Visit(sqlFunctionExpression.Arguments[0], out var leftNullable);
-                var right = Visit(sqlFunctionExpression.Arguments[1], out var rightNullable);
+                var coalesceArguments = new List<SqlExpression>();
+                var coalesceNullable = true;
+                foreach (var argument in sqlFunctionExpression.Arguments)
+                {
+                    coalesceArguments.Add(Visit(argument, out var argumentNullable));
+                    coalesceNullable = coalesceNullable && argumentNullable;
+                }
 
-                nullable = leftNullable && rightNullable;
+                nullable = coalesceNullable;
 
-                return sqlFunctionExpression.Update(sqlFunctionExpression.Instance, new[] { left, right });
+                return sqlFunctionExpression.Update(sqlFunctionExpression.Instance, coalesceArguments);
             }
 
             var instance = Visit(sqlFunctionExpression.Instance, out _);
@@ -1734,35 +1739,28 @@ private SqlExpression ProcessNullNotNull(SqlUnaryExpression sqlUnaryExpression,
                 case SqlFunctionExpression sqlFunctionExpression:
                 {
                     if (sqlFunctionExpression.IsBuiltIn
-                        && string.Equals("COALESCE", sqlFunctionExpression.Name, StringComparison.OrdinalIgnoreCase))
+                        && string.Equals("COALESCE", sqlFunctionExpression.Name, StringComparison.OrdinalIgnoreCase)
+                        && sqlFunctionExpression.Arguments != null)
                     {
-                        // for coalesce:
-                        // (a ?? b) == null -> a == null && b == null
-                        // (a ?? b) != null -> a != null || b != null
-                        var left = ProcessNullNotNull(
-                            _sqlExpressionFactory.MakeUnary(
-                                sqlUnaryExpression.OperatorType,
-                                sqlFunctionExpression.Arguments![0],
-                                typeof(bool),
-                                sqlUnaryExpression.TypeMapping)!,
-                            operandNullable);
-
-                        var right = ProcessNullNotNull(
-                            _sqlExpressionFactory.MakeUnary(
-                                sqlUnaryExpression.OperatorType,
-                                sqlFunctionExpression.Arguments[1],
-                                typeof(bool),
-                                sqlUnaryExpression.TypeMapping)!,
-                            operandNullable);
-
-                        return SimplifyLogicalSqlBinaryExpression(
+                        // for coalesce
+                        // (a ?? b ?? c) == null -> a == null && b == null && c == null
+                        // (a ?? b ?? c) != null -> a != null || b != null || c != null
+                        return sqlFunctionExpression.Arguments
+                            .Select(a => ProcessNullNotNull(
+                                _sqlExpressionFactory.MakeUnary(
+                                    sqlUnaryExpression.OperatorType,
+                                    a,
+                                    typeof(bool),
+                                    sqlUnaryExpression.TypeMapping)!,
+                                operandNullable))
+                            .Aggregate((l, r) => SimplifyLogicalSqlBinaryExpression(
                             _sqlExpressionFactory.MakeBinary(
                                 sqlUnaryExpression.OperatorType == ExpressionType.Equal
                                     ? ExpressionType.AndAlso
                                     : ExpressionType.OrElse,
-                                left,
-                                right,
-                                sqlUnaryExpression.TypeMapping)!);
+                                l,
+                                r,
+                                sqlUnaryExpression.TypeMapping)!));
                     }
 
                     if (!sqlFunctionExpression.IsNullable)

test/EFCore.Relational.Specification.Tests/Query/NullSemanticsQueryTestBase.cs

@@ -929,9 +929,9 @@ public virtual Task Projecting_nullable_bool_with_coalesce(bool async)
 
         [ConditionalTheory]
         [MemberData(nameof(IsAsyncData))]
-        public virtual Task Projecting_nullable_bool_with_coalesce_nested(bool async)
+        public virtual async Task Projecting_nullable_bool_with_coalesce_nested(bool async)
         {
-            return AssertQuery(
+            await AssertQuery(
                 async,
                 ss => ss.Set<NullSemanticsEntity1>().Select(e => new { e.Id, Coalesce = e.NullableBoolA ?? (e.NullableBoolB ?? false) }),
                 elementSorter: e => e.Id,
@@ -940,6 +940,16 @@ public virtual Task Projecting_nullable_bool_with_coalesce_nested(bool async)
                     Assert.Equal(e.Id, a.Id);
                     Assert.Equal(e.Coalesce, a.Coalesce);
                 });
+
+            await AssertQuery(
+                async,
+                ss => ss.Set<NullSemanticsEntity1>().Select(e => new { e.Id, Coalesce = (e.NullableBoolA ?? e.NullableBoolB) ?? false }),
+                elementSorter: e => e.Id,
+                elementAsserter: (e, a) =>
+                {
+                    Assert.Equal(e.Id, a.Id);
+                    Assert.Equal(e.Coalesce, a.Coalesce);
+                });
         }
 
         [ConditionalTheory]
@@ -1863,6 +1873,48 @@ public virtual Task Sum_function_is_always_considered_non_nullable(bool async)
                 ss => ss.Set<NullSemanticsEntity1>().GroupBy(e => e.NullableIntA).Select(g => new { g.Key, Sum = g.Sum(x => x.IntA) != g.Key }));
         }
 
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Nullability_is_computed_correctly_for_chained_coalesce(bool async)
+        {
+            return AssertQuery(
+                async,
+                ss => ss.Set<NullSemanticsEntity1>().Where(e => (e.NullableIntA ?? e.NullableIntB ?? e.IntC) != e.NullableIntC));
+        }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual async Task Nullability_check_is_computed_correctly_for_chained_coalesce(bool async)
+        {
+            await AssertQueryScalar(
+                async,
+                ss => ss.Set<NullSemanticsEntity1>().Where(e => (e.NullableIntA ?? e.NullableIntB ?? e.NullableIntC) == null).Select(e => e.Id));
+
+            await AssertQueryScalar(
+                async,
+                ss => ss.Set<NullSemanticsEntity1>().Where(e => (e.NullableIntA ?? e.NullableIntB ?? e.NullableIntC) != null).Select(e => e.Id));
+        }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Coalesce_on_self_gets_simplified(bool async)
+        {
+            return AssertQuery(
+                async,
+                ss => ss.Set<NullSemanticsEntity1>().Select(e => e.NullableStringA ?? e.NullableStringA));
+        }
+
+        [ConditionalTheory]
+        [MemberData(nameof(IsAsyncData))]
+        public virtual Task Coalesce_deeply_nested(bool async)
+        {
+            return AssertQueryScalar(
+                async,
+                ss => from e1 in ss.Set<NullSemanticsEntity1>()
+                      join e2 in ss.Set<NullSemanticsEntity2>() on e1.Id equals e2.Id
+                      select (e1.NullableIntA ?? (e1.NullableIntB ?? (e2.NullableIntC ?? e2.NullableIntB))) ?? e1.NullableIntC ?? (e2.NullableIntA ?? e2.NullableIntC ?? e1.NullableIntA));
+        }
+
         private string NormalizeDelimitersInRawString(string sql)
             => Fixture.TestStore.NormalizeDelimitersInRawString(sql);
 

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

@@ -5151,11 +5151,11 @@ public override async Task Select_subquery_int_with_outside_cast_and_coalesce(bo
             await base.Select_subquery_int_with_outside_cast_and_coalesce(async);
 
             AssertSql(
-                @"SELECT COALESCE(COALESCE((
+                @"SELECT COALESCE((
     SELECT TOP(1) [w].[Id]
     FROM [Weapons] AS [w]
     WHERE [g].[FullName] = [w].[OwnerFullName]
-    ORDER BY [w].[Id]), 0), 42)
+    ORDER BY [w].[Id]), 0, 42)
 FROM [Gears] AS [g]");
         }
 

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

@@ -1237,7 +1237,10 @@ public override async Task Projecting_nullable_bool_with_coalesce_nested(bool as
             await base.Projecting_nullable_bool_with_coalesce_nested(async);
 
             AssertSql(
-                @"SELECT [e].[Id], COALESCE([e].[NullableBoolA], COALESCE([e].[NullableBoolB], CAST(0 AS bit))) AS [Coalesce]
+                @"SELECT [e].[Id], COALESCE([e].[NullableBoolA], [e].[NullableBoolB], CAST(0 AS bit)) AS [Coalesce]
+FROM [Entities1] AS [e]",
+                //
+                @"SELECT [e].[Id], COALESCE([e].[NullableBoolA], [e].[NullableBoolB], CAST(0 AS bit)) AS [Coalesce]
 FROM [Entities1] AS [e]");
         }
 
@@ -2276,6 +2279,49 @@ FROM [Entities1] AS [e]
 GROUP BY [e].[NullableIntA]");
         }
 
+        public override async Task Nullability_is_computed_correctly_for_chained_coalesce(bool async)
+        {
+            await base.Nullability_is_computed_correctly_for_chained_coalesce(async);
+
+            AssertSql(
+                @"SELECT [e].[Id], [e].[BoolA], [e].[BoolB], [e].[BoolC], [e].[IntA], [e].[IntB], [e].[IntC], [e].[NullableBoolA], [e].[NullableBoolB], [e].[NullableBoolC], [e].[NullableIntA], [e].[NullableIntB], [e].[NullableIntC], [e].[NullableStringA], [e].[NullableStringB], [e].[NullableStringC], [e].[StringA], [e].[StringB], [e].[StringC]
+FROM [Entities1] AS [e]
+WHERE (COALESCE([e].[NullableIntA], [e].[NullableIntB], [e].[IntC]) <> [e].[NullableIntC]) OR [e].[NullableIntC] IS NULL");
+        }
+
+        public override async Task Nullability_check_is_computed_correctly_for_chained_coalesce(bool async)
+        {
+            await base.Nullability_check_is_computed_correctly_for_chained_coalesce(async);
+
+            AssertSql(
+                @"SELECT [e].[Id]
+FROM [Entities1] AS [e]
+WHERE ([e].[NullableIntA] IS NULL AND [e].[NullableIntB] IS NULL) AND [e].[NullableIntC] IS NULL",
+                //
+                @"SELECT [e].[Id]
+FROM [Entities1] AS [e]
+WHERE ([e].[NullableIntA] IS NOT NULL OR [e].[NullableIntB] IS NOT NULL) OR [e].[NullableIntC] IS NOT NULL");
+        }
+
+        public override async Task Coalesce_on_self_gets_simplified(bool async)
+        {
+            await base.Coalesce_on_self_gets_simplified(async);
+
+            AssertSql(
+                @"SELECT [e].[NullableStringA]
+FROM [Entities1] AS [e]");
+        }
+
+        public override async Task Coalesce_deeply_nested(bool async)
+        {
+            await base.Coalesce_deeply_nested(async);
+
+            AssertSql(
+                @"SELECT COALESCE([e].[NullableIntA], [e].[NullableIntB], [e0].[NullableIntC], [e0].[NullableIntB], [e].[NullableIntC], [e0].[NullableIntA])
+FROM [Entities1] AS [e]
+INNER JOIN [Entities2] AS [e0] ON [e].[Id] = [e0].[Id]");
+        }
+
         private void AssertSql(params string[] expected)
             => Fixture.TestSqlLoggerFactory.AssertBaseline(expected);
 

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

@@ -5641,14 +5641,14 @@ FROM [CompetitionSeasons] AS [c1]
                     @"SELECT [a].[Id], [a].[ActivityTypeId], [a].[DateTime], [a].[Points], (
     SELECT TOP(1) [c].[Id]
     FROM [CompetitionSeasons] AS [c]
-    WHERE ([c].[StartDate] <= [a].[DateTime]) AND ([a].[DateTime] < [c].[EndDate])) AS [CompetitionSeasonId], COALESCE([a].[Points], COALESCE((
+    WHERE ([c].[StartDate] <= [a].[DateTime]) AND ([a].[DateTime] < [c].[EndDate])) AS [CompetitionSeasonId], COALESCE([a].[Points], (
     SELECT TOP(1) [a1].[Points]
     FROM [ActivityTypePoints12456] AS [a1]
     INNER JOIN [CompetitionSeasons] AS [c0] ON [a1].[CompetitionSeasonId] = [c0].[Id]
     WHERE ([a0].[Id] = [a1].[ActivityTypeId]) AND ([c0].[Id] = (
         SELECT TOP(1) [c1].[Id]
         FROM [CompetitionSeasons] AS [c1]
-        WHERE ([c1].[StartDate] <= [a].[DateTime]) AND ([a].[DateTime] < [c1].[EndDate])))), 0)) AS [Points]
+        WHERE ([c1].[StartDate] <= [a].[DateTime]) AND ([a].[DateTime] < [c1].[EndDate])))), 0) AS [Points]
 FROM [Activities] AS [a]
 INNER JOIN [ActivityType12456] AS [a0] ON [a].[ActivityTypeId] = [a0].[Id]");
             }

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

@@ -6093,11 +6093,11 @@ public override async Task Select_subquery_int_with_outside_cast_and_coalesce(bo
             await base.Select_subquery_int_with_outside_cast_and_coalesce(async);
 
             AssertSql(
-                @"SELECT COALESCE(COALESCE((
+                @"SELECT COALESCE((
     SELECT TOP(1) [w].[Id]
     FROM [Weapons] AS [w]
     WHERE [g].[FullName] = [w].[OwnerFullName]
-    ORDER BY [w].[Id]), 0), 42)
+    ORDER BY [w].[Id]), 0, 42)
 FROM [Gears] AS [g]");
         }