Removed slow subquery from albums view#283
Conversation
| 'conditions' => $subQuery, | ||
| 'fields' => array('MIN(Song.id)', 'Song.band', 'Song.album', 'Song.cover'), | ||
| 'order' => 'Song.created DESC', | ||
| 'group' => 'Song.album', |
There was a problem hiding this comment.
Could you try grouping also by Song.band to allow showing albums with the same name from different artists ? ( See #236 which was reverted due to poor performances apparently)
There was a problem hiding this comment.
I amended the commit to group by both columns. Tested ok in the GUI :)
There was a problem hiding this comment.
@gs11 great ! It should also be the same for the grouping on the "latest" albums (line 251).
There was a problem hiding this comment.
If repo owners don't oppose it, I can include that change too in this pull req.
There was a problem hiding this comment.
I reconsidered and will place that fix in a separate pull request. Not sure how I create a new pull request with just that commit though. Fix here: https://github.com/gs11/sonerezh/commit/af91a03a17c2e6acde383fe756b89af27f1dea26
There was a problem hiding this comment.
Create a new branch (based on development or your actual branch, depending on the base code you depend on), cherry-pick gs11@af91a03 into this branch. Make a PR from this branch to dev.
In this actual branch, I guess git rebase -i 57d1531 and there you remove commit af91a03. Force push, and that should do it.
There was a problem hiding this comment.
It seems I can't split them since the second commit does changes on what was introduced in the first commit? (getting conflicts) I guess the options then are 1) either remove my last commit, get the pull request merged by the owner and then re-commit and create the new pull request for the last commit 2) or let both commits stay in the same pull request.
There was a problem hiding this comment.
I would advice 1), even if it's the longest path, because with 2) the devs might refused your PR because they don't like the second commit.
It's a bit more painful, but I think it's safer.
There was a problem hiding this comment.
Thanks, last push rolled-back making this pull request only include one commit.
|
Closing PR to use another source branch rather than development. |
Fix for issue #279