Skip to content
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.

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

Implement sum and avg PG interval #2339

Closed
Tracked by #727
ponderingexistence opened this issue Apr 22, 2022 · 7 comments · Fixed by #2423
Closed
Tracked by #727

Implement sum and avg PG interval #2339

ponderingexistence opened this issue Apr 22, 2022 · 7 comments · Fixed by #2423
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@ponderingexistence
Copy link

ponderingexistence commented Apr 22, 2022

Hey there, thank you immensely for your phenomenal work, it's truly invaluable.

It's not unthinkable to have entity classes that contain properties of type TimeSpan, which are by default mapped to PostgreSQL's interval type.
Now, Postgres supports doing sum on interval columns, like so:

SELECT sum(l.duration)
FROM lessons AS l
WHERE l.course_id = 1000

Nice and simple.
However, currently there seems to be no way to get EF Core and Npgsql to generate a SQL like that for TimeSpan properties.

Consider the following model:

public class Lesson
{
    public int Id { get; set; }
    public string Title { get; set; }
    public TimeSpan Duration { get; set; }
}

If you do

var totalDuration = new TimeSpan(db.Lessons.Sum(l => l.Duration.Ticks));

The following exception will be raised:

Unhandled exception. System.InvalidOperationException: The LINQ expression 'DbSet<Lesson>()
    .Sum(l => l.Duration.Ticks)' could not be translated. Either rewrite the query in a form that can be translated, or switch to client evaluation explicitly by inserting a call to 'AsEnumerable', 'AsAsyncEnumerable', 'ToList', or 'ToListAsync'. See https://go.microsoft.com/fwlink/?linkid=2101038 for more information.
   at Microsoft.EntityFrameworkCore.Query.QueryableMethodTranslatingExpressionVisitor.<VisitMethodCall>g__CheckTranslated|15_0(ShapedQueryExpression translated, <>c__DisplayClass15_0& )

I think it's reasonable to say that there should be a way (at least maybe with a special function of some sort, similar to the ones in DbFunctions — e.g. NpgqlFunctions.SumInterval(TimeSpan t) or something, I don't know) to trigger a sum on an interval column. Currently, the only workaround I can think of is something like this:

var totalDuration = new TimeSpan(db.Lessons.Select(l => l.Duration).ToList().Sum(t => t.Ticks));

which isn't nice, as it obviously first retrieves the durations of all the lessons, in this example, and then does the calculation on the client-side, as opposed to taking advantage of PostgreSQL's native sum function.

@roji
Copy link
Member

roji commented Apr 22, 2022

@Aaron2321d what you say make total sense; however, this would be a provider-specific aggregate function, and right now EF Core doesn't really support that. Support is tracked by dotnet/efcore#22957, and the good news is that it's currently planned for 7.0. So I'll put this in the 7.0 milestone here, and once EF Core support is complete I'll definiltely implement this.

@roji roji added the enhancement New feature or request label Apr 22, 2022
@roji roji added this to the 7.0.0 milestone Apr 22, 2022
@roji
Copy link
Member

roji commented Apr 22, 2022

In the meantime you may be able to work around this by first projecting the inteval to some int/long value (e.g. number of seconds), and then perform a sum over that.

@ponderingexistence
Copy link
Author

ponderingexistence commented Apr 22, 2022

what you say make total sense; however, this would be a provider-specific aggregate function, and right now EF Core doesn't really support that. Support is tracked by dotnet/efcore#22957, and the good news is that it's currently planned for 7.0. So I'll put this in the 7.0 milestone here, and once EF Core support is complete I'll definiltely implement this.

@roji Great to hear. Thanks a lot.

In the meantime you may be able to work around this by first projecting the inteval to some int/long value (e.g. number of seconds), and then perform a sum over that.

Is this what you mean?

var totalDuration = TimeSpan.FromSeconds(db.Lessons.Sum(l => l.Duration.TotalSeconds));

It works, to my surprise, the resulting SQL will be:

SELECT COALESCE(SUM(date_part('epoch', l.duration)), 0.0)
FROM lessons AS l

Clearly better than the client-side calculation workaround I mentioned in my original comment.

Since .Sum(l => l.Duration.Ticks) threw a "...could not be translated" exception, I (mistakenly) assumed .Sum(l => l.Duration.TotalSeconds) would throw as well.

Good enough for a workaround I think, thank you!

@roji
Copy link
Member

roji commented Apr 22, 2022

Yep, that's the kind of workaround I was referring to.

@roji roji self-assigned this Jun 4, 2022
@roji roji changed the title Using sum on TimeSpans Implement aggregate methods over PG interval Jun 4, 2022
@roji roji changed the title Implement aggregate methods over PG interval Implement aggregate methods over PG date/time types Jun 4, 2022
@roji roji changed the title Implement aggregate methods over PG date/time types Implement sum and avg PG interval Jun 4, 2022
roji added a commit to roji/efcore.pg that referenced this issue Jun 4, 2022
@roji roji added the blocked label Jun 4, 2022
@roji
Copy link
Member

roji commented Jun 4, 2022

roji added a commit to roji/efcore.pg that referenced this issue Jul 9, 2022
@roji
Copy link
Member

roji commented Jul 9, 2022

This should ideally make use of C# 11 generic math; in other words, LINQ would receive a generic Queryable.Sum method, and we'd just translate that over TimeSpan/Duration/Period.

However, that's unlikely to make it in for 7.0, and since the work is mostly done I'll do this via custom EF.Functions methods. When a generic Sum method appears in .NET, we can obsolete these.

@roji
Copy link
Member

roji commented Jul 9, 2022

Implemented a hacky workaround around dotnet/efcore#28158 to unblock this. Split the future generic Sum/Average out to #2424.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants