From ca0a8b00ec27bd4519aa5e2db7628b87cddb674f Mon Sep 17 00:00:00 2001
From: WithoutPants <53250216+WithoutPants@users.noreply.github.com>
Date: Fri, 1 Oct 2021 16:24:58 +1000
Subject: [PATCH] Only group by if using having clauses (#1750)

* Only group by if using having clauses
* Change tag sorting SQL
* Add sorting unit tests
---
 pkg/sqlite/repository.go                      |  2 +-
 pkg/sqlite/tag.go                             | 15 +++-----
 pkg/sqlite/tag_test.go                        | 35 +++++++++++++++++++
 .../components/Changelog/versions/v0100.md    |  1 +
 4 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/pkg/sqlite/repository.go b/pkg/sqlite/repository.go
index ffe5b78ad..b6b107151 100644
--- a/pkg/sqlite/repository.go
+++ b/pkg/sqlite/repository.go
@@ -238,8 +238,8 @@ func (r *repository) buildQueryBody(body string, whereClauses []string, havingCl
 	if len(whereClauses) > 0 {
 		body = body + " WHERE " + strings.Join(whereClauses, " AND ") // TODO handle AND or OR
 	}
-	body = body + " GROUP BY " + r.tableName + ".id "
 	if len(havingClauses) > 0 {
+		body = body + " GROUP BY " + r.tableName + ".id "
 		body = body + " HAVING " + strings.Join(havingClauses, " AND ") // TODO handle AND or OR
 	}
 
diff --git a/pkg/sqlite/tag.go b/pkg/sqlite/tag.go
index 5d76fddf5..0acf748c0 100644
--- a/pkg/sqlite/tag.go
+++ b/pkg/sqlite/tag.go
@@ -543,20 +543,15 @@ func (qb *tagQueryBuilder) getTagSort(query *queryBuilder, findFilter *models.Fi
 	if findFilter.Sort != nil {
 		switch *findFilter.Sort {
 		case "scenes_count":
-			query.join("scenes_tags", "", "scenes_tags.tag_id = tags.id")
-			return " ORDER BY COUNT(distinct scenes_tags.scene_id) " + direction
+			return getCountSort(tagTable, scenesTagsTable, tagIDColumn, direction)
 		case "scene_markers_count":
-			query.join("scene_markers_tags", "", "scene_markers_tags.tag_id = tags.id")
-			return " ORDER BY COUNT(distinct scene_markers_tags.scene_marker_id) " + direction
+			return getCountSort(tagTable, "scene_markers_tags", tagIDColumn, direction)
 		case "images_count":
-			query.join("images_tags", "", "images_tags.tag_id = tags.id")
-			return " ORDER BY COUNT(distinct images_tags.image_id) " + direction
+			return getCountSort(tagTable, imagesTagsTable, tagIDColumn, direction)
 		case "galleries_count":
-			query.join("galleries_tags", "", "galleries_tags.tag_id = tags.id")
-			return " ORDER BY COUNT(distinct galleries_tags.gallery_id) " + direction
+			return getCountSort(tagTable, galleriesTagsTable, tagIDColumn, direction)
 		case "performers_count":
-			query.join("performers_tags", "", "performers_tags.tag_id = tags.id")
-			return " ORDER BY COUNT(distinct performers_tags.performer_id) " + direction
+			return getCountSort(tagTable, performersTagsTable, tagIDColumn, direction)
 		}
 	}
 
diff --git a/pkg/sqlite/tag_test.go b/pkg/sqlite/tag_test.go
index 49f5e7643..d91325280 100644
--- a/pkg/sqlite/tag_test.go
+++ b/pkg/sqlite/tag_test.go
@@ -151,6 +151,41 @@ func TestTagFindByNames(t *testing.T) {
 	})
 }
 
+func TestTagQuerySort(t *testing.T) {
+	withTxn(func(r models.Repository) error {
+		sqb := r.Tag()
+
+		sortBy := "scenes_count"
+		dir := models.SortDirectionEnumDesc
+		findFilter := &models.FindFilterType{
+			Sort:      &sortBy,
+			Direction: &dir,
+		}
+
+		tags := queryTags(t, sqb, nil, findFilter)
+		assert := assert.New(t)
+		assert.Equal(tagIDs[tagIdxWithScene], tags[0].ID)
+
+		sortBy = "scene_markers_count"
+		tags = queryTags(t, sqb, nil, findFilter)
+		assert.Equal(tagIDs[tagIdxWithMarker], tags[0].ID)
+
+		sortBy = "images_count"
+		tags = queryTags(t, sqb, nil, findFilter)
+		assert.Equal(tagIDs[tagIdxWithImage], tags[0].ID)
+
+		sortBy = "galleries_count"
+		tags = queryTags(t, sqb, nil, findFilter)
+		assert.Equal(tagIDs[tagIdxWithGallery], tags[0].ID)
+
+		sortBy = "performers_count"
+		tags = queryTags(t, sqb, nil, findFilter)
+		assert.Equal(tagIDs[tagIdxWithPerformer], tags[0].ID)
+
+		return nil
+	})
+}
+
 func TestTagQueryName(t *testing.T) {
 	const tagIdx = 1
 	tagName := getSceneStringValue(tagIdx, "Name")
diff --git a/ui/v2.5/src/components/Changelog/versions/v0100.md b/ui/v2.5/src/components/Changelog/versions/v0100.md
index f09e7d335..f152fa6fa 100644
--- a/ui/v2.5/src/components/Changelog/versions/v0100.md
+++ b/ui/v2.5/src/components/Changelog/versions/v0100.md
@@ -14,6 +14,7 @@
 * Support filtering Movies by Performers. ([#1675](https://github.com/stashapp/stash/pull/1675))
 
 ### 🎨 Improvements
+* Improved image query performance. ([#1750](https://github.com/stashapp/stash/pull/1750))
 * Added movie count to performer and studio cards. ([#1760](https://github.com/stashapp/stash/pull/1760))
 * Added date and details to Movie card, and move scene count to icon. ([#1758](https://github.com/stashapp/stash/pull/1758))
 * Added date and details to Gallery card, and move image count to icon. ([#1763](https://github.com/stashapp/stash/pull/1763))