Skip to content

Translate COALESCE as ISNULL #34171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cincuranet merged 3 commits into from
Apr 16, 2025
Merged

Translate COALESCE as ISNULL #34171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c688af7
Implement test for type propagation in `ISNULL`
ranma42 Jul 6, 2024
78f0ee1
Translate `COALESCE` as `ISNULL`
ranma42 Jul 5, 2024
06d9bb3
Update baselines
ranma42 Jul 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,53 @@ protected override Expression VisitValues(ValuesExpression valuesExpression)
return valuesExpression;
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
protected override Expression VisitSqlFunction(SqlFunctionExpression sqlFunctionExpression)
{
if (sqlFunctionExpression is { IsBuiltIn: true, Arguments: not null }
&& string.Equals(sqlFunctionExpression.Name, "COALESCE", StringComparison.OrdinalIgnoreCase))
{
var type = sqlFunctionExpression.Type;
var typeMapping = sqlFunctionExpression.TypeMapping;
var defaultTypeMapping = _typeMappingSource.FindMapping(type);

// ISNULL always return a value having the same type as its first
// argument. Ideally we would convert the argument to have the
// desired type and type mapping, but currently EFCore has some
// trouble in computing types of non-homogeneous expressions
// (tracked in https://github.com/dotnet/efcore/issues/15586). To
// stay on the safe side we only use ISNULL if:
// - all sub-expressions have the same type as the expression
// - all sub-expressions have the same type mapping as the expression
// - the expression is using the default type mapping (combined
Copy link
Copy Markdown
Member

@roji roji Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one also seems a bit strict - if all sub-expressions have the same store type, shouldn't everything be fine and we can assume the overall expression has the same type? What's the scenario that could be problematic given a non-default store type?

Copy link
Copy Markdown
Contributor Author

@ranma42 ranma42 Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EFCore computes accurate store types only in some cases (column expressions, simple additions), but in many other cases it approximates them, for example a literal string is modeled as varchar instead of varchar(length-of-the-literal).

The idea was that if all of the the expressions are using the default type mapping, the C#-oriented type compatibility matches the SQL one as closely as possible... but now I wonder if some cases are actually unsafe even under this assumption 🤔 .
I think an example that would break with only same type and same type-mapping is:

(col1VarChar10 != 'foo' ? col1VarChar10 : 'a long literal') ??
(col2VarChar10 != 'foo' ? col2VarChar10 : 'a very long literal')

IIRC EFCore computes the type mapping of the LHS (and of the whole expression) as the same type mapping as col1VarChar10.

I think an example that would break with the current set of constraints is :

(col1VarChar10 == 'foo' ? 'a long literal' : col1VarChar10) ??
(col2VarChar10 == 'foo' ? 'a very long literal' : col2VarChar10)

I will try to check this ASAP.

Copy link
Copy Markdown
Member

@roji roji Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the general logic is that type mappings are configured on columns, and then inferred from them to other, non-column expression types (e.g. in b.MyCol == "some constant"), as well as bubbling up in the tree as necessary.

But I'm still a bit confused: if we know that all sub-expressions have the same store type (and therefore so should the SqlFunctionExpression for the COALESCE call on top), how could we possibly have an issue, regardless of whether that type mapping happens to be a default or not? Is this because you think that with a default type mapping there's less of chance that EF got the type mapping wrong (or similar)? Would be good to see a concrete case for that.

Copy link
Copy Markdown
Contributor Author

@ranma42 ranma42 Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coalesce_Correct_TypeMapping_Double and Coalesce_Correct_TypeMapping_String are a couple of examples in which EFCore computes the wrong (/inaccurate) type mapping for some of the expressions.
Assuming that the implementation in this PR is wrong, I will add another test case that shows that the check is not strict enough.

Conversely, you are right that checking that the type is the default one is not required; its main purpose was to trust C# for the type computation, under the (presumably wrong) assumption that when the C#-computed type and the EFCore-computed type matched, that also meant a match for the SQL type.

// with the two above, this implies that all of the expressions
// are using the default type mapping of the type)
Comment thread
cincuranet marked this conversation as resolved.
if (defaultTypeMapping == typeMapping
&& sqlFunctionExpression.Arguments.All(a => a.Type == type && a.TypeMapping == typeMapping)) {
Copy link
Copy Markdown
Member

@roji roji Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to the above comment on the CLR type, I think we could compare the store type instead of referencing-comparing the type mapping instances... There's no guarantee in EF that there's only one type mapping instance for a given type mapping, and unless I'm mistaken the only thing that matters here for ISNULL is the actual store type. So comparing the store type should be safe and possibly allow for some more scenarios.

Suggested change
&& sqlFunctionExpression.Arguments.All(a => a.Type == type && a.TypeMapping == typeMapping)) {
&& sqlFunctionExpression.Arguments.All(a => a.Type == type && a.TypeMapping?.StoreType == typeMapping?.StoreType)) {

Copy link
Copy Markdown
Contributor Author

@ranma42 ranma42 Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC this would not recognize cases in which the mismatch is in one of the parameters (such as varchar vs varchar(15)).
Maybe the right approach here would be to make typemappings IEquatable and compare them

Copy link
Copy Markdown
Member

@roji roji Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC this would not recognize cases in which the mismatch is in one of the parameters (such as varchar vs varchar(15)).

I think StoreType must always be the full store type - including any facets. If that's not the case, we most probably have bugs somewhere else as well. Do you have a specific case in mind where this doesn't work?

Maybe the right approach here would be to make typemappings IEquatable and compare them

I'm not sure... The point here is that in this specific context, the only thing we care about is the actual type in the database; in other words, various other details that distinguish type mappings from one another are irrelevant (value comparers, value converters...), since they affect EF client-side processing only. Assuming we have a 100% unambiguous representation of the database type - which is what StoreType is supposed to be - comparing those should be sufficient and allow for maximum coverage of this optimization, I think.

A general IEquatable would need to also compare e.g. value comparers (since it's possible that in other contexts that's relevant), and so would needlessly exclude some valid cases...


var head = sqlFunctionExpression.Arguments[0];
sqlFunctionExpression = (SqlFunctionExpression)sqlFunctionExpression
.Arguments
.Skip(1)
.Aggregate(head, (l, r) => new SqlFunctionExpression(
"ISNULL",
arguments: [l, r],
nullable: true,
Copy link
Copy Markdown
Member

@roji roji Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least for the top-most ISNULL, shouldn't we be taking the original coalesce expression's nullability? For example, in the very common case of b.Foo ?? "<unknown>", the entire expression should be non-nullable since the last sub-expression (the constant string) is non-nullable.

As this is done extremely late in the query pipeline (SQL generator), there's not going to be anything afterwards that cares about this (this node is immediately processed, generating SQL string data, and then discarded). But for correctness's sake (and in case this code is copy-pasted somewhere else), maybe we should do it?

Copy link
Copy Markdown
Contributor Author

@ranma42 ranma42 Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nullable: true is always safer than nullable: false, so no risk of incorrectness here ;)
We could do this for the top-most ISNULL invocation (which, if the COALESCE has been optimized properly, is the only one that can be non-nullable), but it would make the code a little more complicated for no apparent advantage.

Note that in this context we do not have access to the results of the nullability processor (the nullable value in the expression does not correspond to the results of the processing).

Copy link
Copy Markdown
Member

@roji roji Apr 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no risk of incorrectness here

Right, not incorrect in the sense of incorrect results - just potentially SQL that's more convoluted (and less efficient) than it needs to be.

Note that in this context we do not have access to the results of the nullability processor (the nullable value in the expression does not correspond to the results of the processing).

That's true, but the expression nullable does represent what it represents: it seems odd to basically lose that setting just because we're transforming one SQL function into another here.

But anyway, given where this code is, I agree it's theoretical. If we do any extra work on this, I'd add a comment making that clear.

Copy link
Copy Markdown
Contributor Author

@ranma42 ranma42 Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no risk of incorrectness here

Right, not incorrect in the sense of incorrect results - just potentially SQL that's more convoluted (and less efficient) than it needs to be.

Note that in this context we do not have access to the results of the nullability processor (the nullable value in the expression does not correspond to the results of the processing).

That's true, but the expression nullable does represent what it represents: it seems odd to basically lose that setting just because we're transforming one SQL function into another here.

I believe we are not losing any information here: COALESCE expressions are always constructed with nullable: true from this code.
They can be re-written in a few places but the IsNullable property is never modified (neither for COALESCE nor for other function expressions).

But anyway, given where this code is, I agree it's theoretical. If we do any extra work on this, I'd add a comment making that clear.

Copy link
Copy Markdown
Member

@roji roji Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I wasn't aware of that... That's presumably because we have special handling of COALESCE in any case in SqlNullabilityProcessor, so IsNullable is maybe effectively unused?

I'd prefer it IsNullable were accurate here (across the board, regardless of COALESCE vs. ISNULL) - you never know when someone might look at it - but I guess it's not super important at this point.

Copy link
Copy Markdown
Contributor Author

@ranma42 ranma42 Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I wasn't aware of that... That's presumably because we have special handling of COALESCE in any case in SqlNullabilityProcessor, so IsNullable is maybe effectively unused?

The nullability processor relies on IsNullable as input to decide whether to assume that the function is guaranteed to return a non-null value or whether its result should be considered (non)nullable depending on (some of) its arguments.
Currently it is accurate in the perspective of function definition and not from the point of view of the expression (even less of the expression in a given context).
IIRC the same holds true for other expressions as well: during the nullability computation for myNullableColumn is null ? 1 : myNullableColumn + 2 the second myNullableColumn expression is evaluated as non-nullable, but it is not rewritten to have IsNullable = false.

I'd prefer it IsNullable were accurate here (across the board, regardless of COALESCE vs. ISNULL) - you never know when someone might look at it - but I guess it's not super important at this point.

Your suggestion goes in the direction of tracking nullability in the expression, which might indeed prove very useful, for example when lowering SqlServer boolean expressions/predicates 😫, but I think is not specifically related to the change COALESCE->ISNULL in this PR

argumentsPropagateNullability: [false, false],
sqlFunctionExpression.Type,
sqlFunctionExpression.TypeMapping
));
}
}

return base.VisitSqlFunction(sqlFunctionExpression);
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down
45 changes: 44 additions & 1 deletion test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3229,6 +3229,49 @@ public override async Task Entity_equality_orderby_descending_subquery_composite
AssertSql();
}

public override Task Coalesce_Correct_Multiple_Same_TypeMapping(bool async)
=> Fixture.NoSyncTest(
async, async a =>
{
await base.Coalesce_Correct_Multiple_Same_TypeMapping(async);

AssertSql(
"""
SELECT VALUE
{
"ReportsTo" : c["ReportsTo"],
"c" : 1,
"c0" : 2,
"c1" : 3
}
FROM root c
ORDER BY c["EmployeeID"]
""");
});

public override Task Coalesce_Correct_TypeMapping_Double(bool async)
=> Fixture.NoSyncTest(
async, async a =>
{
await base.Coalesce_Correct_TypeMapping_Double(async);

AssertSql();
});

public override Task Coalesce_Correct_TypeMapping_String(bool async)
=> Fixture.NoSyncTest(
async, async a =>
{
await base.Coalesce_Correct_TypeMapping_String(async);

AssertSql(
"""
SELECT VALUE ((c["Region"] != null) ? c["Region"] : "no region specified")
FROM root c
ORDER BY c["id"]
""");
});

public override async Task Null_Coalesce_Short_Circuit(bool async)
{
// Cosmos client evaluation. Issue #17246.
Expand Down Expand Up @@ -3347,7 +3390,7 @@ public override async Task SelectMany_primitive_select_subquery(bool async)
// Cosmos client evaluation. Issue #17246.
Assert.Equal(
CoreStrings.ExpressionParameterizationExceptionSensitive(
"value(Microsoft.EntityFrameworkCore.Query.NorthwindMiscellaneousQueryTestBase`1+<>c__DisplayClass172_0[Microsoft.EntityFrameworkCore.Query.NorthwindQueryCosmosFixture`1[Microsoft.EntityFrameworkCore.TestUtilities.NoopModelCustomizer]]).ss.Set().Any()"),
"value(Microsoft.EntityFrameworkCore.Query.NorthwindMiscellaneousQueryTestBase`1+<>c__DisplayClass175_0[Microsoft.EntityFrameworkCore.Query.NorthwindQueryCosmosFixture`1[Microsoft.EntityFrameworkCore.TestUtilities.NoopModelCustomizer]]).ss.Set().Any()"),
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.SelectMany_primitive_select_subquery(async))).Message);

Expand Down
25 changes: 25 additions & 0 deletions test/EFCore.Specification.Tests/Query/NorthwindMiscellaneousQueryTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,31 @@ public virtual Task Ternary_should_not_evaluate_both_sides_with_parameter(bool a
}));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Coalesce_Correct_Multiple_Same_TypeMapping(bool async)
=> AssertQuery(
async,
ss => ss.Set<Employee>().OrderBy(e => e.EmployeeID)
.Select(e => (e.ReportsTo + 1L) ?? (e.ReportsTo + 2L) ?? (e.ReportsTo + 3L)),
assertOrder: true);

[ConditionalTheory(Skip = "issue #15586")]
[MemberData(nameof(IsAsyncData))]
public virtual Task Coalesce_Correct_TypeMapping_Double(bool async)
=> AssertQuery(
async,
ss => ss.Set<Employee>().OrderBy(e => e.EmployeeID).Select(e => e.ReportsTo ?? 2.25),
assertOrder: true);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Coalesce_Correct_TypeMapping_String(bool async)
=> AssertQuery(
async,
ss => ss.Set<Customer>().OrderBy(c => c.CustomerID).Select(c => c.Region ?? "no region specified"),
assertOrder: true);

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task Null_Coalesce_Short_Circuit(bool async)
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServer160Test.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3980,7 +3980,7 @@ public override async Task Nested_object_constructed_from_group_key_properties(b

AssertSql(
"""
SELECT [l].[Id], [l].[Name], [l].[Date], [l0].[Id], [l1].[Name], [l0].[Date], [l0].[Level1_Optional_Id], [l0].[Level1_Required_Id], COALESCE(SUM(CAST(LEN([l].[Name]) AS int)), 0) AS [Aggregate]
SELECT [l].[Id], [l].[Name], [l].[Date], [l0].[Id], [l1].[Name], [l0].[Date], [l0].[Level1_Optional_Id], [l0].[Level1_Required_Id], ISNULL(SUM(CAST(LEN([l].[Name]) AS int)), 0) AS [Aggregate]
FROM [LevelOne] AS [l]
LEFT JOIN [LevelTwo] AS [l0] ON [l].[Id] = [l0].[Level1_Optional_Id]
LEFT JOIN [LevelTwo] AS [l1] ON [l].[Id] = [l1].[Level1_Required_Id]
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3980,7 +3980,7 @@ public override async Task Nested_object_constructed_from_group_key_properties(b

AssertSql(
"""
SELECT [l].[Id], [l].[Name], [l].[Date], [l0].[Id], [l1].[Name], [l0].[Date], [l0].[Level1_Optional_Id], [l0].[Level1_Required_Id], COALESCE(SUM(CAST(LEN([l].[Name]) AS int)), 0) AS [Aggregate]
SELECT [l].[Id], [l].[Name], [l].[Date], [l0].[Id], [l1].[Name], [l0].[Date], [l0].[Level1_Optional_Id], [l0].[Level1_Required_Id], ISNULL(SUM(CAST(LEN([l].[Name]) AS int)), 0) AS [Aggregate]
FROM [LevelOne] AS [l]
LEFT JOIN [LevelTwo] AS [l0] ON [l].[Id] = [l0].[Level1_Optional_Id]
LEFT JOIN [LevelTwo] AS [l1] ON [l].[Id] = [l1].[Level1_Required_Id]
Expand Down
2 changes: 1 addition & 1 deletion ...Core.SqlServer.FunctionalTests/Query/ComplexNavigationsSharedTypeQuerySqlServer160Test.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,7 @@ public override async Task Nested_object_constructed_from_group_key_properties(b

AssertSql(
"""
SELECT [s].[Id], [s].[Name], [s].[Date], [s].[InnerId] AS [Id], [s].[Level2_Name0] AS [Name], [s].[OneToOne_Required_PK_Date] AS [Date], [s].[Level1_Optional_Id], [s].[Level1_Required_Id], COALESCE(SUM(CAST(LEN([s].[Name]) AS int)), 0) AS [Aggregate]
SELECT [s].[Id], [s].[Name], [s].[Date], [s].[InnerId] AS [Id], [s].[Level2_Name0] AS [Name], [s].[OneToOne_Required_PK_Date] AS [Date], [s].[Level1_Optional_Id], [s].[Level1_Required_Id], ISNULL(SUM(CAST(LEN([s].[Name]) AS int)), 0) AS [Aggregate]
FROM (
SELECT [l].[Id], [l].[Date], [l].[Name], [l1].[OneToOne_Required_PK_Date], [l1].[Level1_Optional_Id], [l1].[Level1_Required_Id], [l3].[Level2_Name] AS [Level2_Name0], CASE
WHEN [l1].[OneToOne_Required_PK_Date] IS NOT NULL AND [l1].[Level1_Required_Id] IS NOT NULL AND [l1].[OneToMany_Required_Inverse2Id] IS NOT NULL THEN [l1].[Id]
Expand Down
2 changes: 1 addition & 1 deletion .../EFCore.SqlServer.FunctionalTests/Query/ComplexNavigationsSharedTypeQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ public override async Task Nested_object_constructed_from_group_key_properties(b

AssertSql(
"""
SELECT [s].[Id], [s].[Name], [s].[Date], [s].[InnerId] AS [Id], [s].[Level2_Name0] AS [Name], [s].[OneToOne_Required_PK_Date] AS [Date], [s].[Level1_Optional_Id], [s].[Level1_Required_Id], COALESCE(SUM(CAST(LEN([s].[Name]) AS int)), 0) AS [Aggregate]
SELECT [s].[Id], [s].[Name], [s].[Date], [s].[InnerId] AS [Id], [s].[Level2_Name0] AS [Name], [s].[OneToOne_Required_PK_Date] AS [Date], [s].[Level1_Optional_Id], [s].[Level1_Required_Id], ISNULL(SUM(CAST(LEN([s].[Name]) AS int)), 0) AS [Aggregate]
FROM (
SELECT [l].[Id], [l].[Date], [l].[Name], [l1].[OneToOne_Required_PK_Date], [l1].[Level1_Optional_Id], [l1].[Level1_Required_Id], [l3].[Level2_Name] AS [Level2_Name0], CASE
WHEN [l1].[OneToOne_Required_PK_Date] IS NOT NULL AND [l1].[Level1_Required_Id] IS NOT NULL AND [l1].[OneToMany_Required_Inverse2Id] IS NOT NULL THEN [l1].[Id]
Expand Down
18 changes: 9 additions & 9 deletions test/EFCore.SqlServer.FunctionalTests/Query/GearsOfWarQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3475,7 +3475,7 @@ public override async Task ToString_nullable_enum_property_projection(bool async
SELECT CASE [w].[AmmunitionType]
WHEN 1 THEN N'Cartridge'
WHEN 2 THEN N'Shell'
ELSE COALESCE(CAST([w].[AmmunitionType] AS nvarchar(max)), N'')
ELSE ISNULL(CAST([w].[AmmunitionType] AS nvarchar(max)), N'')
END
FROM [Weapons] AS [w]
""");
Expand Down Expand Up @@ -3504,7 +3504,7 @@ FROM [Weapons] AS [w]
WHERE CASE [w].[AmmunitionType]
WHEN 1 THEN N'Cartridge'
WHEN 2 THEN N'Shell'
ELSE COALESCE(CAST([w].[AmmunitionType] AS nvarchar(max)), N'')
ELSE ISNULL(CAST([w].[AmmunitionType] AS nvarchar(max)), N'')
END LIKE N'%Cart%'
""");
}
Expand Down Expand Up @@ -4883,7 +4883,7 @@ public override async Task Select_subquery_projecting_single_constant_int(bool a

AssertSql(
"""
SELECT [s].[Name], COALESCE((
SELECT [s].[Name], ISNULL((
SELECT TOP(1) 42
FROM [Gears] AS [g]
WHERE [s].[Id] = [g].[SquadId] AND [g].[HasSoulPatch] = CAST(1 AS bit)), 0) AS [Gear]
Expand Down Expand Up @@ -4911,7 +4911,7 @@ public override async Task Select_subquery_projecting_single_constant_bool(bool

AssertSql(
"""
SELECT [s].[Name], COALESCE((
SELECT [s].[Name], ISNULL((
SELECT TOP(1) CAST(1 AS bit)
FROM [Gears] AS [g]
WHERE [s].[Id] = [g].[SquadId] AND [g].[HasSoulPatch] = CAST(1 AS bit)), CAST(0 AS bit)) AS [Gear]
Expand Down Expand Up @@ -5546,7 +5546,7 @@ public override async Task String_concat_on_various_types(bool async)

AssertSql(
"""
SELECT N'HasSoulPatch ' + CAST([g].[HasSoulPatch] AS nvarchar(max)) + N' HasSoulPatch' AS [HasSoulPatch], N'Rank ' + CAST([g].[Rank] AS nvarchar(max)) + N' Rank' AS [Rank], N'SquadId ' + CAST([g].[SquadId] AS nvarchar(max)) + N' SquadId' AS [SquadId], N'Rating ' + COALESCE(CAST([m].[Rating] AS nvarchar(max)), N'') + N' Rating' AS [Rating], N'Timeline ' + CAST([m].[Timeline] AS nvarchar(max)) + N' Timeline' AS [Timeline]
SELECT N'HasSoulPatch ' + CAST([g].[HasSoulPatch] AS nvarchar(max)) + N' HasSoulPatch' AS [HasSoulPatch], N'Rank ' + CAST([g].[Rank] AS nvarchar(max)) + N' Rank' AS [Rank], N'SquadId ' + CAST([g].[SquadId] AS nvarchar(max)) + N' SquadId' AS [SquadId], N'Rating ' + ISNULL(CAST([m].[Rating] AS nvarchar(max)), N'') + N' Rating' AS [Rating], N'Timeline ' + CAST([m].[Timeline] AS nvarchar(max)) + N' Timeline' AS [Timeline]
FROM [Gears] AS [g]
CROSS JOIN [Missions] AS [m]
ORDER BY [g].[Nickname], [m].[Id]
Expand Down Expand Up @@ -6616,7 +6616,7 @@ public override async Task Complex_GroupBy_after_set_operator(bool async)

AssertSql(
"""
SELECT [u].[Name], [u].[Count], COALESCE(SUM([u].[Count]), 0) AS [Sum]
SELECT [u].[Name], [u].[Count], ISNULL(SUM([u].[Count]), 0) AS [Sum]
FROM (
SELECT [c].[Name], (
SELECT COUNT(*)
Expand All @@ -6642,7 +6642,7 @@ public override async Task Complex_GroupBy_after_set_operator_using_result_selec

AssertSql(
"""
SELECT [u].[Name], [u].[Count], COALESCE(SUM([u].[Count]), 0) AS [Sum]
SELECT [u].[Name], [u].[Count], ISNULL(SUM([u].[Count]), 0) AS [Sum]
FROM (
SELECT [c].[Name], (
SELECT COUNT(*)
Expand Down Expand Up @@ -9062,7 +9062,7 @@ public override async Task Set_operator_with_navigation_in_projection_groupby_ag
AssertSql(
"""
SELECT [s].[Name], (
SELECT COALESCE(SUM(CAST(LEN([c].[Location]) AS int)), 0)
SELECT ISNULL(SUM(CAST(LEN([c].[Location]) AS int)), 0)
FROM [Gears] AS [g2]
INNER JOIN [Squads] AS [s0] ON [g2].[SquadId] = [s0].[Id]
INNER JOIN [Cities] AS [c] ON [g2].[CityOfBirthName] = [c].[Name]
Expand Down Expand Up @@ -9146,7 +9146,7 @@ LEFT JOIN (
SELECT [w].[Id], [w].[AmmunitionType], [w].[IsAutomatic], [w].[Name], [w].[OwnerFullName], [w].[SynergyWithId], ROW_NUMBER() OVER(PARTITION BY [w].[OwnerFullName] ORDER BY [w].[Id]) AS [row]
FROM [Weapons] AS [w]
) AS [w0]
WHERE [w0].[row] <= COALESCE((
WHERE [w0].[row] <= ISNULL((
SELECT [n].[value]
FROM OPENJSON(@numbers) WITH ([value] int '$') AS [n]
ORDER BY [n].[value]
Expand Down
6 changes: 3 additions & 3 deletions test/EFCore.SqlServer.FunctionalTests/Query/NorthwindAggregateOperatorsQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@ public override async Task Sum_over_uncorrelated_subquery(bool async)
// #34256: rewrite query to avoid "Cannot perform an aggregate function on an expression containing an aggregate or a subquery"
AssertSql(
"""
SELECT COALESCE(SUM([s].[value]), 0)
SELECT ISNULL(SUM([s].[value]), 0)
FROM [Customers] AS [c]
CROSS JOIN (
SELECT COUNT(*) AS [value]
Expand Down Expand Up @@ -2721,7 +2721,7 @@ public override async Task Project_constant_Sum(bool async)

AssertSql(
"""
SELECT COALESCE(SUM(1), 0)
SELECT ISNULL(SUM(1), 0)
FROM [Employees] AS [e]
""");
}
Expand Down Expand Up @@ -3081,7 +3081,7 @@ public override async Task Contains_inside_Sum_without_GroupBy(bool async)
"""
@cities='["London","Berlin"]' (Size = 4000)

SELECT COALESCE(SUM([s].[value]), 0)
SELECT ISNULL(SUM([s].[value]), 0)
FROM [Customers] AS [c]
OUTER APPLY (
SELECT CASE
Expand Down
6 changes: 3 additions & 3 deletions test/EFCore.SqlServer.FunctionalTests/Query/NorthwindGroupByQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ public override async Task GroupBy_aggregate_projecting_conditional_expression(b
"""
SELECT [o].[OrderDate] AS [Key], CASE
WHEN COUNT(*) = 0 THEN 1
ELSE COALESCE(SUM(CASE
ELSE ISNULL(SUM(CASE
WHEN [o].[OrderID] % 2 = 0 THEN 1
ELSE 0
END), 0) / COUNT(*)
Expand Down Expand Up @@ -1939,7 +1939,7 @@ public override async Task GroupBy_Sum_constant(bool async)

AssertSql(
"""
SELECT COALESCE(SUM(1), 0)
SELECT ISNULL(SUM(1), 0)
FROM [Orders] AS [o]
GROUP BY [o].[CustomerID]
""");
Expand Down Expand Up @@ -2699,7 +2699,7 @@ public override async Task GroupBy_aggregate_followed_by_another_GroupBy_aggrega

AssertSql(
"""
SELECT [o1].[Key0] AS [Key], COALESCE(SUM([o1].[Count]), 0) AS [Count]
SELECT [o1].[Key0] AS [Key], ISNULL(SUM([o1].[Count]), 0) AS [Count]
FROM (
SELECT [o0].[Count], 1 AS [Key0]
FROM (
Expand Down
12 changes: 6 additions & 6 deletions test/EFCore.SqlServer.FunctionalTests/Query/NorthwindJoinQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,9 +596,9 @@ public override async Task GroupJoin_aggregate_anonymous_key_selectors(bool asyn
await base.GroupJoin_aggregate_anonymous_key_selectors(async);

AssertSql(
"""
"""
SELECT [c].[CustomerID], (
SELECT COALESCE(SUM(CAST(LEN([o].[CustomerID]) AS int)), 0)
SELECT ISNULL(SUM(CAST(LEN([o].[CustomerID]) AS int)), 0)
FROM [Orders] AS [o]
WHERE [c].[City] IS NOT NULL AND [c].[CustomerID] = [o].[CustomerID] AND [c].[City] = N'London') AS [Sum]
FROM [Customers] AS [c]
Expand All @@ -610,9 +610,9 @@ public override async Task GroupJoin_aggregate_anonymous_key_selectors2(bool asy
await base.GroupJoin_aggregate_anonymous_key_selectors2(async);

AssertSql(
"""
"""
SELECT [c].[CustomerID], (
SELECT COALESCE(SUM(CAST(LEN([o].[CustomerID]) AS int)), 0)
SELECT ISNULL(SUM(CAST(LEN([o].[CustomerID]) AS int)), 0)
FROM [Orders] AS [o]
WHERE [c].[CustomerID] = [o].[CustomerID] AND 1996 = DATEPART(year, [o].[OrderDate])) AS [Sum]
FROM [Customers] AS [c]
Expand All @@ -624,9 +624,9 @@ public override async Task GroupJoin_aggregate_anonymous_key_selectors_one_argum
await base.GroupJoin_aggregate_anonymous_key_selectors_one_argument(async);

AssertSql(
"""
"""
SELECT [c].[CustomerID], (
SELECT COALESCE(SUM(CAST(LEN([o].[CustomerID]) AS int)), 0)
SELECT ISNULL(SUM(CAST(LEN([o].[CustomerID]) AS int)), 0)
FROM [Orders] AS [o]
WHERE [c].[CustomerID] = [o].[CustomerID]) AS [Sum]
FROM [Customers] AS [c]
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.SqlServer.FunctionalTests/Query/NorthwindKeylessEntitiesQuerySqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public override async Task KeylessEntity_groupby(bool async)

AssertSql(
"""
SELECT [m].[City] AS [Key], COUNT(*) AS [Count], COALESCE(SUM(CAST(LEN([m].[Address]) AS int)), 0) AS [Sum]
SELECT [m].[City] AS [Key], COUNT(*) AS [Count], ISNULL(SUM(CAST(LEN([m].[Address]) AS int)), 0) AS [Sum]
FROM (
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]
) AS [m]
Expand Down
Loading