0
\$\begingroup\$

The code is returning reviews based on users like status order. It seems it executes a lot of SQL queries, so do you think it can be simplified and can it be a headache to server performance?

ret = _dbEntitySet.AsNoTracking()
           .Where(p => p.PostId == postId && p.IsPublished)
           .Include(p => p.UserLikes)
           .OrderByDescending(p => p.UserLikes.Select(f => f.LikeStatus == LikeStatus.Like).Count() - p.UserLikes.Select(f => f.LikeStatus == LikeStatus.Dislike).Count())
           .Skip((reviewPageIndex - 1) * reviewPageSize)
           .Take(reviewPageSize)
           .Select(p => new PostModalReviewDisplay()
           {
               CommentCount = p.Comments.Count(),
               DislikeCount = p.UserLikes.Where(q => q.LikeStatus == LikeStatus.Dislike).Count(),
               LikeCount = p.UserLikes.Where(q => q.LikeStatus == LikeStatus.Like).Count(),
               IsCurrentUserDisliked = p.UserLikes.Any(w => w.LikeStatus == LikeStatus.Dislike && w.UserInfoId == currUserId),
               IsCurrentUserLiked = p.UserLikes.Any(w => w.LikeStatus == LikeStatus.Like && w.UserInfoId == currUserId),

               Content = p.Content,
               Id = p.Id,
               UserInfo = new UserInfoExtraSmall()
               {
                   AppUserId=p.UserInfo.AppUserId,
                   Name=p.UserInfo.Name,
                   Surname=p.UserInfo.Surname,
                   ProfileImage=p.UserInfo.ProfilePicture.SmallPath
               }
           })
\$\endgroup\$
  • 1
    \$\begingroup\$ Nothing better than to check generated SQL then they query plan and then measure... \$\endgroup\$ – Adriano Repetti Oct 26 '17 at 12:47
  • \$\begingroup\$ My guess is that ef-core isn't smart enough yet to translate the nested counts into one SQL statement and auto-switches to client-side evaluation, i.e. n+1 queries. \$\endgroup\$ – Gert Arnold Oct 26 '17 at 12:50
  • \$\begingroup\$ Have you tried to transform OrderByDescending(p => p.UserLikes.Select(f => f.LikeStatus == LikeStatus.Like).Count() - p.UserLikes.Select(f => f.LikeStatus == LikeStatus.Dislike).Count()) to Select(p => new { Obj = p, CountProperty = p.UserLikes.Select(f => f.LikeStatus == LikeStatus.Like).Count() - p.UserLikes.Select(f => f.LikeStatus == LikeStatus.Dislike).Count() }).OrderBy(p => p.CountProperty)? \$\endgroup\$ – Gentian Kasa Oct 26 '17 at 14:15
  • \$\begingroup\$ @GentianKasa I did not try it, but I dont think it is going to work. \$\endgroup\$ – Okan Aslankan Oct 26 '17 at 17:45
  • \$\begingroup\$ @GertArnold so do you think this query can be simplified by using raw SQL or stored procedure or smth.? \$\endgroup\$ – Okan Aslankan Oct 26 '17 at 17:46
1
\$\begingroup\$

As mentioned in a comment of mine, it's possible that there's a bug in the code. The OrderByDescending(p => p.UserLikes.Select(f => f.LikeStatus == LikeStatus.Like).Count() - p.UserLikes.Select(f => f.LikeStatus == LikeStatus.Dislike).Count()) call should always sort on a single value, 0. The call should be equal to OrderByDescending(p => p.UserLikes.Count() - p.UserLikes.Count()) because the Select method does not filter values.

That being said, I'll consider it as OrderByDescending(p => p.UserLikes.Where(f => f.LikeStatus == LikeStatus.Like).Count() - p.UserLikes.Where(f => f.LikeStatus == LikeStatus.Dislike).Count()) from here on out. If this is not the case let me know and I'll delete this answer.

I see that you count likes and dislikes a couple of times in your queries, in the OrderByDescending method and in the LikeCount and DislikeCount properties in the final Select. Keeping in mind this consideration I'd change the query in the following:

ret = _dbEntitySet.AsNoTracking()
       .Where(p => p.PostId == postId && p.IsPublished)
       .Include(p => p.UserLikes)
       .Select(p => new { DbEntity = p, LikeCount = p.UserLikes.Where(f => f.LikeStatus == LikeStatus.Like).Count(), DislikeCount = p.UserLikes.Where(f => f.LikeStatus == LikeStatus.Dislike).Count() })
       .OrderByDescending(p => p.LikeCount - p.DislikeCount)
       .Skip((reviewPageIndex - 1) * reviewPageSize)
       .Take(reviewPageSize)
       .Select(p => new PostModalReviewDisplay()
       {
           CommentCount = p.DbEntity.Comments.Count(),
           DislikeCount = p.DislikeCount,
           LikeCount = p.LikeCount,
           IsCurrentUserDisliked = p.DbEntity.UserLikes.Any(w => w.LikeStatus == LikeStatus.Dislike && w.UserInfoId == currUserId),
           IsCurrentUserLiked = p.DbEntity.UserLikes.Any(w => w.LikeStatus == LikeStatus.Like && w.UserInfoId == currUserId),

           Content = p.DbEntity.Content,
           Id = p.DbEntity.Id,
           UserInfo = new UserInfoExtraSmall()
           {
               AppUserId=p.DbEntity.UserInfo.AppUserId,
               Name=p.DbEntity.UserInfo.Name,
               Surname=p.DbEntity.UserInfo.Surname,
               ProfileImage=p.DbEntity.UserInfo.ProfilePicture.SmallPath
           }
       })

One more thing that comes to mind is to switch the comparison clauses in the IsCurrentUserDisliked and IsCurrentUserLiked properties. Note that the two versions could be the same in terms of performance, depending on the execution plan that the query optimizer generates, but personally I prefer to put the most selective comparison clause first.

So, the final version would be the following:

ret = _dbEntitySet.AsNoTracking()
       .Where(p => p.PostId == postId && p.IsPublished)
       .Include(p => p.UserLikes)
       .Select(p => new { DbEntity = p, LikeCount = p.UserLikes.Where(f => f.LikeStatus == LikeStatus.Like).Count(), DislikeCount = p.UserLikes.Where(f => f.LikeStatus == LikeStatus.Dislike).Count() })
       .OrderByDescending(p => p.LikeCount - p.DislikeCount)
       .Skip((reviewPageIndex - 1) * reviewPageSize)
       .Take(reviewPageSize)
       .Select(p => new PostModalReviewDisplay()
       {
           CommentCount = p.DbEntity.Comments.Count(),
           DislikeCount = p.DislikeCount,
           LikeCount = p.LikeCount,
           IsCurrentUserDisliked = p.DbEntity.UserLikes.Any(w => w.UserInfoId == currUserId && w.LikeStatus == LikeStatus.Dislike),
           IsCurrentUserLiked = p.DbEntity.UserLikes.Any(w => w.UserInfoId == currUserId && w.LikeStatus == LikeStatus.Like),

           Content = p.DbEntity.Content,
           Id = p.DbEntity.Id,
           UserInfo = new UserInfoExtraSmall()
           {
               AppUserId=p.DbEntity.UserInfo.AppUserId,
               Name=p.DbEntity.UserInfo.Name,
               Surname=p.DbEntity.UserInfo.Surname,
               ProfileImage=p.DbEntity.UserInfo.ProfilePicture.SmallPath
           }
       })

As a final thought, I agree with @AdrianoRepetti's comment:

Nothing better than to check generated SQL then they query plan and then measure...

Check the generated query plan for different versions and use the query that best suits your needs.

\$\endgroup\$
  • \$\begingroup\$ Hmm. I've implemented your final version but it is throwing an error which I did not understand. Query source (from UserReviewLike f in value(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable"1[fso.Core.Domains.MMEntities.UserReviewLike])) has already been associated with an expression.' do you know why this may be throwing error. \$\endgroup\$ – Okan Aslankan Oct 27 '17 at 15:05
  • \$\begingroup\$ It works like charm, I was getting error from other action. Thanks. \$\endgroup\$ – Okan Aslankan Oct 27 '17 at 15:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.