Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix stats upon searching issues (#17566) #17578

Merged

Conversation

Gusted
Copy link
Contributor

@Gusted Gusted commented Nov 7, 2021

Backport #17566

  • Fixes a issue whereby the given chunk of issueIDs wasn't respected and
    thus the returned results where not the correct results. More specifically when the amount of returned results are above maxQueryParameters.

To reproduce this issue easily:

  • Set the constant maxQueryParameters within the code to a value of 5.
  • Create a new repo.
  • Create more than 5 issues(I'm using 7) with the same description(e.g. Hello).
  • Go to the issues tab and search for that description: Hello.
  • See that amount of open issues is inaccurate and way above the correct amount.

^ This PR fixes that issue.

Their is a interesting comment above the relevant code:

gitea/models/issue.go

Lines 1502 to 1505 in bd613c7

// If too long a list of IDs is provided, we get the statistics in
// smaller chunks and get accumulates. Note: this could potentially
// get us invalid results. The alternative is to insert the list of
// ids in a temporary table and join from them.

I do think that the person writing that comment was hitting into this bug whereby the wrong issuesIDs were used in the database action and I do think it can be removed, but I'm not sure.

Screenshots(The repo had 7 issues with the description of Helo):

Before:
image

After:
image

@delvh delvh added the type/bug label Nov 7, 2021
@delvh delvh added this to the 1.15.7 milestone Nov 7, 2021
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Nov 8, 2021
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 8, 2021
@zeripath zeripath changed the title backport(1.15): Fix stats upon searching issues Fix stats upon searching issues (#17566) Nov 8, 2021
@lafriks lafriks merged commit 1cb1101 into go-gitea:release/v1.15 Nov 8, 2021
@Gusted Gusted deleted the backkport1.15-fix-search-cunk branch November 24, 2021 08:26
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants