Optimization of labels handling in issue_search (#26460)

This PR enhances the labels handling in issue_search by optimizing the
SQL query and de-duplicate the IDs when generating the query string.

---------

Co-authored-by: techknowlogick <techknowlogick@gitea.com>
This commit is contained in:
Chl 2024-06-25 19:09:13 +02:00 committed by GitHub
parent 72c66bd479
commit b5326a431f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 59 additions and 9 deletions

View File

@ -6,6 +6,7 @@ package issues
import ( import (
"context" "context"
"fmt" "fmt"
"strconv"
"strings" "strings"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
@ -13,6 +14,7 @@ import (
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/container"
"code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/optional"
"xorm.io/builder" "xorm.io/builder"
@ -116,14 +118,30 @@ func applyLabelsCondition(sess *xorm.Session, opts *IssuesOptions) {
if opts.LabelIDs[0] == 0 { if opts.LabelIDs[0] == 0 {
sess.Where("issue.id NOT IN (SELECT issue_id FROM issue_label)") sess.Where("issue.id NOT IN (SELECT issue_id FROM issue_label)")
} else { } else {
for i, labelID := range opts.LabelIDs { // We sort and deduplicate the labels' ids
IncludedLabelIDs := make(container.Set[int64])
ExcludedLabelIDs := make(container.Set[int64])
for _, labelID := range opts.LabelIDs {
if labelID > 0 { if labelID > 0 {
sess.Join("INNER", fmt.Sprintf("issue_label il%d", i), IncludedLabelIDs.Add(labelID)
fmt.Sprintf("issue.id = il%[1]d.issue_id AND il%[1]d.label_id = %[2]d", i, labelID))
} else if labelID < 0 { // 0 is not supported here, so just ignore it } else if labelID < 0 { // 0 is not supported here, so just ignore it
sess.Where("issue.id not in (select issue_id from issue_label where label_id = ?)", -labelID) ExcludedLabelIDs.Add(-labelID)
} }
} }
// ... and use them in a subquery of the form :
// where (select count(*) from issue_label where issue_id=issue.id and label_id in (2, 4, 6)) = 3
// This equality is guaranteed thanks to unique index (issue_id,label_id) on table issue_label.
if len(IncludedLabelIDs) > 0 {
subquery := builder.Select("count(*)").From("issue_label").Where(builder.Expr("issue_id = issue.id")).
And(builder.In("label_id", IncludedLabelIDs.Values()))
sess.Where(builder.Eq{strconv.Itoa(len(IncludedLabelIDs)): subquery})
}
// or (select count(*)...) = 0 for excluded labels
if len(ExcludedLabelIDs) > 0 {
subquery := builder.Select("count(*)").From("issue_label").Where(builder.Expr("issue_id = issue.id")).
And(builder.In("label_id", ExcludedLabelIDs.Values()))
sess.Where(builder.Eq{"0": subquery})
}
} }
} }

View File

@ -7,6 +7,7 @@ package issues
import ( import (
"context" "context"
"fmt" "fmt"
"slices"
"strconv" "strconv"
"strings" "strings"
@ -142,9 +143,8 @@ func (l *Label) CalOpenOrgIssues(ctx context.Context, repoID, labelID int64) {
// LoadSelectedLabelsAfterClick calculates the set of selected labels when a label is clicked // LoadSelectedLabelsAfterClick calculates the set of selected labels when a label is clicked
func (l *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64, currentSelectedExclusiveScopes []string) { func (l *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64, currentSelectedExclusiveScopes []string) {
var labelQuerySlice []string labelQuerySlice := []int64{}
labelSelected := false labelSelected := false
labelID := strconv.FormatInt(l.ID, 10)
labelScope := l.ExclusiveScope() labelScope := l.ExclusiveScope()
for i, s := range currentSelectedLabels { for i, s := range currentSelectedLabels {
if s == l.ID { if s == l.ID {
@ -155,15 +155,26 @@ func (l *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64, curr
} else if s != 0 { } else if s != 0 {
// Exclude other labels in the same scope from selection // Exclude other labels in the same scope from selection
if s < 0 || labelScope == "" || labelScope != currentSelectedExclusiveScopes[i] { if s < 0 || labelScope == "" || labelScope != currentSelectedExclusiveScopes[i] {
labelQuerySlice = append(labelQuerySlice, strconv.FormatInt(s, 10)) labelQuerySlice = append(labelQuerySlice, s)
} }
} }
} }
if !labelSelected { if !labelSelected {
labelQuerySlice = append(labelQuerySlice, labelID) labelQuerySlice = append(labelQuerySlice, l.ID)
} }
l.IsSelected = labelSelected l.IsSelected = labelSelected
l.QueryString = strings.Join(labelQuerySlice, ",")
// Sort and deduplicate the ids to avoid the crawlers asking for the
// same thing with simply a different order of parameters
slices.Sort(labelQuerySlice)
labelQuerySlice = slices.Compact(labelQuerySlice)
// Quick conversion (strings.Join() doesn't accept slices of Int64)
labelQuerySliceStrings := make([]string, len(labelQuerySlice))
for i, x := range labelQuerySlice {
labelQuerySliceStrings[i] = strconv.FormatInt(x, 10)
}
l.QueryString = strings.Join(labelQuerySliceStrings, ",")
} }
// BelongsToOrg returns true if label is an organization label // BelongsToOrg returns true if label is an organization label

View File

@ -23,6 +23,27 @@ func TestLabel_CalOpenIssues(t *testing.T) {
assert.EqualValues(t, 2, label.NumOpenIssues) assert.EqualValues(t, 2, label.NumOpenIssues)
} }
func TestLabel_LoadSelectedLabelsAfterClick(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
// Loading the label id:8 which have a scope and an exclusivity
label := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 8})
// First test : with negative and scope
label.LoadSelectedLabelsAfterClick([]int64{1, -8}, []string{"", "scope"})
assert.Equal(t, "1", label.QueryString)
assert.Equal(t, true, label.IsSelected)
// Second test : with duplicates
label.LoadSelectedLabelsAfterClick([]int64{1, 7, 1, 7, 7}, []string{"", "scope", "", "scope", "scope"})
assert.Equal(t, "1,8", label.QueryString)
assert.Equal(t, false, label.IsSelected)
// Third test : empty set
label.LoadSelectedLabelsAfterClick([]int64{}, []string{})
assert.False(t, label.IsSelected)
assert.Equal(t, "8", label.QueryString)
}
func TestLabel_ExclusiveScope(t *testing.T) { func TestLabel_ExclusiveScope(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase()) assert.NoError(t, unittest.PrepareTestDatabase())
label := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 7}) label := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 7})