Query optimization based om empty collections
yecli opened this issue · 6 comments
Need to optimize GetByIds methods by skipping redundant requests in case of empty collections like in Cart module.
Places where need to do optimization:
GetCatalogsByIds
GetItemByIds
GetPropertiesByIds
GetAllCatalogProperties
GetPropertyDictionaryItemsByIds
As proposed by @asvishnyakov and @artem-dudarev, we could add IQueryable<> extension method to platform that implements this logic and use it in this cases.
I would like to have extension method and use method chain instead of if-wrapping, i.e.
result = Cart.NotNull.Where(...).ToArray();
Not clear how to do this, as our goal here is not just to ensure that IEnumerable<> is not null or empty, but to skip queries execution in that case. Could execution of query be skipped in extension method?
I do it this way:
public class MyRepository
{
public virtual IList<CarouselBannerEntity> GetCarouselBannersByIds(IList<string> ids)
{
return CarouselBanners
.Where(x => ids.Contains(x.Id))
.LoadWithRelatedEntities(ids, LoadCarouselBannerRelatedEntities);
}
public virtual IList<CarouselBannerEntity> GetCarouselBannersByObjectIds(IList<string> objectIds, string objectType)
{
return CarouselBanners
.Where(x => x.ObjectType == objectType && objectIds.Contains(x.ObjectId))
.LoadWithRelatedEntities(objectIds, LoadCarouselBannerRelatedEntities);
}
protected virtual void LoadCarouselBannerRelatedEntities(IList<string> ids)
{
CarouselBannerStores.Where(x => ids.Contains(x.CarouselBannerId)).Load();
CarouselBannerUserGroups.Where(x => ids.Contains(x.CarouselBannerId)).Load();
}
}
public static class QueryableExtensions
{
public static IList<T> LoadWithRelatedEntities<T>(this IQueryable<T> query, IEnumerable<string> keys, Action<IList<string>> loadRelatedEntitiesAction)
where T : Entity
{
IList<T> result;
if (keys?.Any() == true)
{
result = query.ToList();
if (result.Any())
{
var ids = result.Select(x => x.Id).ToList();
loadRelatedEntitiesAction(ids);
}
}
else
{
result = new T[0];
}
return result;
}
}
pretty nice! but Array.Empty() instead of T[0] looks better for me, last one may confuse (access to element with zero index)
Yes, wanted to change that too.
After discussion with @tatarincev decided to not use extension method and handle references loading in place.