Translate PostgreSQL custom aggregates #2383
Conversation
| .Select(g => new | ||
| { | ||
| City = g.Key, | ||
| FaxNumbers = EF.Functions.ArrayAgg(g.Select(c => c.Fax).OrderBy(id => id)) |
There was a problem hiding this comment.
This "aggregates" a column into a PG array. This notably can allow loading related entities in a single query without the cartesian explosion (all posts of a blog are simply downloaded as an array).
| MethodInfo method, | ||
| EnumerableExpression source, | ||
| IReadOnlyList<SqlExpression> arguments, | ||
| IDiagnosticsLogger<DbLoggerCategory.Query> logger) |
There was a problem hiding this comment.
You mean why do I need to "replace" NpgsqlQueryableAggregateMethodTranslator.cs? There are two translations which actually need to be customized for PG (see comments on Count and Sum below), but it's also to support the PG-specific predicate and ordering clauses (which are handled by _sqlExpressionFactory.AggregateFunction)...
Makes sense?
There was a problem hiding this comment.
Yes. I didn't notice the subtle difference that translation is calling into AggregateFunction method. Fine to override if you are generating a different translation. I thought it was same code as relational. In the absence of provider translating these methods, relational would provide it.
There was a problem hiding this comment.
Yep, the new relational implementation makes sense as the default thing.
| { | ||
| // These methods accept two enumerable (column) arguments; this is represented in LINQ as a projection from the grouping | ||
| // to a tuple of the two columns. Since we generally translate tuples to PostgresRowValueExpression, we take it apart here. | ||
| if (source.Selector is not PostgresRowValueExpression rowValueExpression) |
There was a problem hiding this comment.
@smitpatel here's a translation of an aggregate function with multiple enumerable arguments. See test below for the C# side - as discussed the grouping needs to be projected to a tuple containing the two columns. It looks a bit weird/complex, but it prevents applying different predicate/orderings/distincts which would have been possible had the function accepted two enumerables instead.
What do you think, how does it look?
Funnily enough, since the PG provider translates ValueTuple to PostgresRowValueExpression, that's what we get here. That's also a bit weird, but seems OK.
There was a problem hiding this comment.
I do have some idea, let's discuss next time we are in office.
There was a problem hiding this comment.
BTW @smitpatel at some point we can discuss this too
| g => new | ||
| { | ||
| City = g.Key, | ||
| Companies = EF.Functions.JsonbObjectAgg<string, string, Dictionary<string, string>>( |
There was a problem hiding this comment.
And probably my last aggregate work for now 😌
We now support the PG jsonb_object_agg function, using it to project out a Dictionary<string,string> out of two arbitrary columns in a table. Can also get a string JSON document (see test above).
(@ajcvickers this goes in the direction of projecting raw JSON data so we can send it directly over HTTP without entity dematerialization).
roji
changed the title
|
Rebased on the merged aggregate infra support (#2393), cleaned up and split spatial aggregates out to #2398 (because it relies on upstream tests in dotnet/efcore#28104 (comment), which hasn't yet been merged). |
roji
force-pushed
the
CustomAggregates
branch
2 times, most recently
from
d4a9594 to
a22027b
Compare
src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlQueryableAggregateMethodTranslator.cs
Outdated
Show resolved
Hide resolved
* string_agg (string.Join) * array_agg * json_agg/jsonb_agg * json_object_agg/jsonb_object_agg * range_agg, range_intersect_agg * Aggregate statistics functions Closes npgsql#2395 Closes npgsql#532
Here's some nice WIP based off of @smitpatel's great work in dotnet/efcore#28092.
This implements the PG-specific aggregate operators (FILTER and ORDER BY), and provides a first implementation of string_agg and array_agg. Really impressive we can do this!
/cc @dotnet/efteam
Closes #2384