backport #29990 This PR uses `db.ListOptions` instead of `Paginor` to make the code simpler. And it also fixed the performance problem when viewing /pulls or /issues. Before the counting in fact will also do the search. Co-authored-by: Jason Song <i@wolfogre.com> Co-authored-by: silverwind <me@silverwind.io>tags/v1.21.11
@@ -21,7 +21,7 @@ import ( | |||
// IssuesOptions represents options of an issue. | |||
type IssuesOptions struct { //nolint | |||
db.Paginator | |||
Paginator *db.ListOptions | |||
RepoIDs []int64 // overwrites RepoCond if the length is not 0 | |||
RepoCond builder.Cond | |||
AssigneeID int64 | |||
@@ -103,23 +103,11 @@ func applyLimit(sess *xorm.Session, opts *IssuesOptions) *xorm.Session { | |||
return sess | |||
} | |||
// Warning: Do not use GetSkipTake() for *db.ListOptions | |||
// Its implementation could reset the page size with setting.API.MaxResponseItems | |||
if listOptions, ok := opts.Paginator.(*db.ListOptions); ok { | |||
if listOptions.Page >= 0 && listOptions.PageSize > 0 { | |||
var start int | |||
if listOptions.Page == 0 { | |||
start = 0 | |||
} else { | |||
start = (listOptions.Page - 1) * listOptions.PageSize | |||
} | |||
sess.Limit(listOptions.PageSize, start) | |||
} | |||
return sess | |||
start := 0 | |||
if opts.Paginator.Page > 1 { | |||
start = (opts.Paginator.Page - 1) * opts.Paginator.PageSize | |||
} | |||
start, limit := opts.Paginator.GetSkipTake() | |||
sess.Limit(limit, start) | |||
sess.Limit(opts.Paginator.PageSize, start) | |||
return sess | |||
} |
@@ -69,13 +69,17 @@ func CountIssuesByRepo(ctx context.Context, opts *IssuesOptions) (map[int64]int6 | |||
} | |||
// CountIssues number return of issues by given conditions. | |||
func CountIssues(ctx context.Context, opts *IssuesOptions) (int64, error) { | |||
func CountIssues(ctx context.Context, opts *IssuesOptions, otherConds ...builder.Cond) (int64, error) { | |||
sess := db.GetEngine(ctx). | |||
Select("COUNT(issue.id) AS count"). | |||
Table("issue"). | |||
Join("INNER", "repository", "`issue`.repo_id = `repository`.id") | |||
applyConditions(sess, opts) | |||
for _, cond := range otherConds { | |||
sess.And(cond) | |||
} | |||
return sess.Count() | |||
} | |||
@@ -10,7 +10,7 @@ import ( | |||
) | |||
// ParsePaginator parses a db.Paginator into a skip and limit | |||
func ParsePaginator(paginator db.Paginator, max ...int) (int, int) { | |||
func ParsePaginator(paginator *db.ListOptions, max ...int) (int, int) { | |||
// Use a very large number to indicate no limit | |||
unlimited := math.MaxInt32 | |||
if len(max) > 0 { | |||
@@ -19,22 +19,15 @@ func ParsePaginator(paginator db.Paginator, max ...int) (int, int) { | |||
} | |||
if paginator == nil || paginator.IsListAll() { | |||
// It shouldn't happen. In actual usage scenarios, there should not be requests to search all. | |||
// But if it does happen, respect it and return "unlimited". | |||
// And it's also useful for testing. | |||
return 0, unlimited | |||
} | |||
// Warning: Do not use GetSkipTake() for *db.ListOptions | |||
// Its implementation could reset the page size with setting.API.MaxResponseItems | |||
if listOptions, ok := paginator.(*db.ListOptions); ok { | |||
if listOptions.Page >= 0 && listOptions.PageSize > 0 { | |||
var start int | |||
if listOptions.Page == 0 { | |||
start = 0 | |||
} else { | |||
start = (listOptions.Page - 1) * listOptions.PageSize | |||
} | |||
return start, listOptions.PageSize | |||
} | |||
return 0, unlimited | |||
if paginator.PageSize == 0 { | |||
// Do not return any results when searching, it's used to get the total count only. | |||
return 0, 0 | |||
} | |||
return paginator.GetSkipTake() |
@@ -78,6 +78,17 @@ func (i *Indexer) Search(ctx context.Context, options *internal.SearchOptions) ( | |||
return nil, err | |||
} | |||
// If pagesize == 0, return total count only. It's a special case for search count. | |||
if options.Paginator != nil && options.Paginator.PageSize == 0 { | |||
total, err := issue_model.CountIssues(ctx, opt, cond) | |||
if err != nil { | |||
return nil, err | |||
} | |||
return &internal.SearchResult{ | |||
Total: total, | |||
}, nil | |||
} | |||
ids, total, err := issue_model.IssueIDs(ctx, opt, cond) | |||
if err != nil { | |||
return nil, err |
@@ -309,7 +309,7 @@ func SearchIssues(ctx context.Context, opts *SearchOptions) ([]int64, int64, err | |||
// CountIssues counts issues by options. It is a shortcut of SearchIssues(ctx, opts) but only returns the total count. | |||
func CountIssues(ctx context.Context, opts *SearchOptions) (int64, error) { | |||
opts = opts.Copy(func(options *SearchOptions) { opts.Paginator = &db_model.ListOptions{PageSize: 0} }) | |||
opts = opts.Copy(func(options *SearchOptions) { options.Paginator = &db_model.ListOptions{PageSize: 0} }) | |||
_, total, err := SearchIssues(ctx, opts) | |||
return total, err |
@@ -104,7 +104,7 @@ type SearchOptions struct { | |||
UpdatedAfterUnix *int64 | |||
UpdatedBeforeUnix *int64 | |||
db.Paginator | |||
Paginator *db.ListOptions | |||
SortBy SortBy // sort by field | |||
} |
@@ -77,6 +77,13 @@ func TestIndexer(t *testing.T, indexer internal.Indexer) { | |||
assert.Equal(t, c.ExpectedIDs, ids) | |||
assert.Equal(t, c.ExpectedTotal, result.Total) | |||
} | |||
// test counting | |||
c.SearchOptions.Paginator = &db.ListOptions{PageSize: 0} | |||
countResult, err := indexer.Search(context.Background(), c.SearchOptions) | |||
require.NoError(t, err) | |||
assert.Empty(t, countResult.Hits) | |||
assert.Equal(t, result.Total, countResult.Total) | |||
}) | |||
} | |||
} |
@@ -211,6 +211,14 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) ( | |||
skip, limit := indexer_internal.ParsePaginator(options.Paginator, maxTotalHits) | |||
counting := limit == 0 | |||
if counting { | |||
// If set limit to 0, it will be 20 by default, and -1 is not allowed. | |||
// See https://www.meilisearch.com/docs/reference/api/search#limit | |||
// So set limit to 1 to make the cost as low as possible, then clear the result before returning. | |||
limit = 1 | |||
} | |||
// to make it non fuzzy ("typo tolerance" in meilisearch terms), we have to quote the keyword(s) | |||
// https://www.meilisearch.com/docs/reference/api/search#phrase-search | |||
keyword := doubleQuoteKeyword(options.Keyword) | |||
@@ -226,6 +234,10 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) ( | |||
return nil, err | |||
} | |||
if counting { | |||
searchRes.Hits = nil | |||
} | |||
hits := make([]internal.Match, 0, len(searchRes.Hits)) | |||
for _, hit := range searchRes.Hits { | |||
hits = append(hits, internal.Match{ |