Introduce an IQueryable ToDictionary method (to align with async) and translate it #16730
Comments
|
@kccsf do you expect the aggregate function (COUNT) to be evaluated in memory or on the server? |
|
@divega - Thanks for the response. I would expect it to be evaluated server side. |
|
@kccsf Do you have the same expectation for |
|
@ajcvickers - Yes; IMHO it is less confusing if async/non async functions perform in the same manner (server side or client side) unless there is a compelling reason that they shouldn't? My preference of server side is purely down to not wishing to retrieve a potentially large amount of records when all I need is a count for a high level summary. |
|
@kccsf It is important for you to know that the lambda expressions passed to either This is because For There is an ongoing discussion on what to do about this in the long term. See #16838 for one possible way to address it. |
|
In the meantime, the correct way to write this query is: var test = await this.dbcontext.MyEntity
.GroupBy(x => x.Status)
.Select(x => new {Name = x.Key.Name, Count = x.Count()})
.ToDictionaryAsync(p => p.Name, q => q.Count);(Note that I haven't tried this on a real application, so you could still run into other issues). |
|
@divega - Thanks for the explanation and the corrected query. An analyzer such as that mentioned in the thread to which you linked could indeed be helpful. Unfortunately, the rewritten query still suffers from the original issue - EF.Property called with wrong property name. var test = await this.dbcontext.MyEntity
.GroupBy(x => x.Status)
.Select(x => new { Name = x.Key.Name, Count = x.Count() })
.ToDictionaryAsync(x => x.Name, y => y.Count);Slightly different stack trace: |
|
Is |
|
@smitpatel - it is indeed. I'll keep an eye on that issue. Thanks |
|
@smitpatel wouldn't it be possible to workaround #15249 by explicitly grouping on the key of Status? |
|
Yes, that works. If you project scalar property out of navigation (or even anonymous type if more than one property it would work. |
|
@divega - Do you mean like so? var test = await this.dbcontext.MyEntity
.GroupBy(x => x.Status.Id)
............If so that does not help as I need the non grouped property x.Key.Name which would then not be available. |
|
|
@kccsf, @smitpatel understood, thanks for the additional details. I am guessing that if the Name of Status is guaranteed somehow to be unique, you can use it directly as the key of the groups, simplifying the query to: var test = await this.dbcontext.MyEntity
.GroupBy(x => x.Status.Name)
.Select(x => new { Name = x.Key, Count = x.Count() })
.ToDictionaryAsync(x => x.Name, y => y.Count);You may need to create additional database indexes for this query to be evaluated more efficiently on the server. I hope this helps. |
|
@divega / @smitpatel - Very kind but I already have a workaround in the form of FromSqlRaw, so unless you wish to have something here for others who hit this issue please don't waste your precious time on it! @divega - Thanks but name is not unique. @smitpatel - Thanks but fails in the same manner (EF.Property called with wrong property name): var test = await this.dbcontext.MyEntity
.GroupBy(x => x.Status.Id)
.Select(g => new { g.Key, Count = g.Count() })
.Join(
this.dbcontext.MyEntity.Select(x => x.Status),
x => x.Key,
y => y.Id,
(x, y) => new { y.Name, x.Count })
.ToListAsync();It is still the GroupBy failing (EF.Property called with wrong property name): var test = await this.dbcontext.MyEntity
.GroupBy(x => x.Status.Id)
.Select(x => new { Id = x.Key, Count = x.Count() })
.ToListAsync(); |
|
Missed that part. GroupBy does not expand navigation so you would have to generate manual join. |
ajcvickers
changed the title
|
I'd like to see If, on the other hand, Here is a working sample of an enhanced ToDictionaryAsync function: public static class EnhancedExtensions
{
public static async Task<Dictionary<TKey, TElement>> EnhancedToDictionaryAsync<TSource, TKey, TElement>(this IQueryable<TSource> source, Expression<Func<TSource, TKey>> keySelector, Expression<Func<TSource, TElement>> elementSelector, CancellationToken cancellationToken = default)
{
if (source == null) throw new ArgumentNullException(nameof(source));
if (keySelector == null) throw new ArgumentNullException(nameof(keySelector));
if (elementSelector == null) throw new ArgumentNullException(nameof(elementSelector));
var param = keySelector.Parameters[0];
var elementSelectorBody = elementSelector.Body.Replace(elementSelector.Parameters[0], param);
var type = typeof(ValueTuple<TKey, TElement>);
var item1member = type.GetMember(nameof(ValueTuple<TKey, TElement>.Item1))[0];
var item2member = type.GetMember(nameof(ValueTuple<TKey, TElement>.Item2))[0];
var memberInit = Expression.MemberInit(
Expression.New(typeof(ValueTuple<TKey, TElement>)),
Expression.Bind(item1member, keySelector.Body),
Expression.Bind(item2member, elementSelectorBody));
var expr = Expression.Lambda<Func<TSource, ValueTuple<TKey, TElement>>>(
memberInit,
param);
var results = await source.Select(expr).ToListAsync(cancellationToken);
return results.ToDictionary(x => x.Item1, x => x.Item2);
}
private static Expression Replace(this Expression expression, ParameterExpression oldParameter, Expression newBody)
{
if (expression == null) throw new ArgumentNullException(nameof(expression));
if (oldParameter == null) throw new ArgumentNullException(nameof(oldParameter));
if (newBody == null) throw new ArgumentNullException(nameof(newBody));
if (expression is LambdaExpression) throw new InvalidOperationException("The search & replace operation must be performed on the body of the lambda.");
return (new ParameterReplacerVisitor(oldParameter, newBody)).Visit(expression);
}
private class ParameterReplacerVisitor : ExpressionVisitor
{
private ParameterExpression _source;
private Expression _target;
public ParameterReplacerVisitor(ParameterExpression source, Expression target)
{
_source = source;
_target = target;
}
protected override Expression VisitParameter(ParameterExpression node)
{
// Replace the source with the target, visit other params as usual.
return node.Equals(_source) ? _target : base.VisitParameter(node);
}
}
}I also ran a few tests on one of my production Azure databases which has 66,000 records and a bunch of columns: var a = (await _db.Parts.ToListAsync(this.HttpContext.RequestAborted)).ToDictionary(x => x.PartId, x => x.PartNumber);
var b = (await _db.Parts.ToDictionaryAsync(x => x.PartId, x => x.PartNumber, this.HttpContext.RequestAborted));
var c = (await _db.Parts.EnhancedToDictionaryAsync(x => x.PartId, x => x.PartNumber, this.HttpContext.RequestAborted));As you can imagine, 'a' and 'b' took 3.5 to 4.5 seconds each, while 'c' took 60 to 300ms. It might be even faster if the PartNumber field was indexed. For bonus points, I'd like to see ToHashSet and ToHashSetAsync added as well. |
|
See #20076 for possibly introducing an IQueryable ToList/ToArray as well. |
The following query is not working:
If you are seeing an exception, include the full exceptions details (message and stack trace).
Steps to reproduce
Further technical details
EF Core version: 3.0.0-preview8.19374.2
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 10
IDE: 16.2.0 Preview 4.0
The text was updated successfully, but these errors were encountered: