Skip to content

Exploratory work on custom aggregate methods#1531

Closed
roji wants to merge 1 commit intonpgsql:mainfrom
roji:Aggregates
Closed

Exploratory work on custom aggregate methods#1531
roji wants to merge 1 commit intonpgsql:mainfrom
roji:Aggregates

Conversation

@roji
Copy link
Member

@roji roji commented Oct 11, 2020

Following discussion in dotnet/efcore#13278, I attempted to translate string_agg, which is a PostgreSQL-specific aggregate function that reduces a set of values to a single string (concatenation). There are some other similar functions.

I did successfully managed to translate top-level (IQueryable) invocations, which is really cool. However, the current query pipeline doesn't seem to allow custom functions in the GroupBy aggregate case (e.g. GroupingElementExpression is private). Assuming we want to fully support arbitrary aggregate functions, we can use this to understand what's needed.

/cc @smitpatel @ajcvickers

return AggregateResultShaper(source, projection, throwWhenEmpty: true, typeof(string));
}

#region Copied from RelationalQueryableMethodTranslatingExpressionVisitor
Copy link
Member Author

Choose a reason for hiding this comment

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

Note code copied below, which seems like it would be necessary for anyone needing to add top-level aggregate function support. Ideally this duplication wouldn't be necessary.

@"@__p_1='|'
@__p_0='2'

SELECT string_agg(t.""ContactName"", @__p_1)
Copy link
Member Author

Choose a reason for hiding this comment

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

Subquery isn't needed, no?

var lambdaBody = ReplacingExpressionVisitor.Replace(
lambdaExpression.Parameters.Single(), shapedQueryExpression.ShaperExpression, lambdaExpression.Body);

// TODO: Can't duplicate WeakEntityExpandingExpressionVisitor (ColumnExpression ctors are internal)
Copy link
Member Author

Choose a reason for hiding this comment

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

Apparently not possible support weak entities

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.

1 participant