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

Added Postgresql specific aggregation functions. #1745

Closed
wants to merge 24 commits into from

Conversation

nathannau
Copy link

No description provided.

roji and others added 23 commits November 15, 2020 21:17
(cherry picked from commit 9f222de)
@nathannau nathannau requested a review from roji as a code owner March 5, 2021 01:22
Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

@nathannau thanks for submitting this.

Fully supporting provider-specific aggregate functions is a goal for EF Core 6.0 (see dotnet/efcore#22957) - this should fix some of the issues (e.g. copy out the private GroupingElementExpression, support for using aggregate methods on queryable/top-level). I've done some exploratory work in #1531.

Once things are properly implemented upstream in EF Core, the actual implementation of the PG aggregate methods would likely look different (e.g. I doubt we'll be adding Translate* methods for each and every method on NpgsqlSqlTranslationExpressionVisitor - we'd likely have method translators). So I think it's better to wait until things are probably done in EF Core before doing this in the PG provider.

@roji
Copy link
Member

roji commented Dec 29, 2021

As written in the above review, proper support for PG-specific aggregate functions depends on dotnet/efcore#22957. I'll go ahead and close this until PR until that issue is implemented on the EF Core side.

@roji roji closed this Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants