dotnet / dotnet/efcore

Special-case coalescing of SUM in SqlNullability is causing issues

Open
#28,158 3 comments 2 reactions 0 assignees View on GitHub
area-query customer-reported
Dominant language
C#
Stars
14.8k
Forks
3.4k
PR merge metrics
PR metrics pending

Description

In my eternal quest to translate all aggregate functions on the planet (and beyond), I'm adding support for Sum and Average over TimeSpan for PostgreSQL, where that's supported (https://github.com/npgsql/efcore.pg/issues/2339).

For sum, I ran into trouble because of [the following code in SqlNullabilityProcessor](https://github.com/dotnet/efcore/blob/main/src/EFCore.Relational/Query/SqlNullabilityProcessor.cs#L1044):

```c#
if (sqlFunctionExpression.IsBuiltIn
&& string.Equals(sqlFunctionExpression.Name, "SUM", StringComparison.OrdinalIgnoreCase))
{
nullable = false;

return _sqlExpressionFactory.Coalesce(
sqlFunctionExpression.Update(instance, arguments),
_sqlExpressionFactory.Constant(0, sqlFunctionExpression.TypeMapping),
sqlFunctionExpression.TypeMapping);
}
```

This fails because it attempts to coalesce TimeSpan (non-numeric) to zero.

Replacing the 0 with `Activator.CreateInstance(sqlFunctionExpression.Type)` makes the problem go away. However, since I'm translating EF.Functions.Sum, the return type of that function is nullable (`TimeSpan?`), since that's how SQL sum behaves (compare with #26634). So the COALESCE introduced here is unwanted.

It generally seems better to handle this kind of thing in the method translator (but I don't have the context) - is that possible?

/cc @maumar @smitpatel

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.