Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

fix distinct count bug #489

Merged
merged 3 commits into from
Apr 13, 2022
Merged

fix distinct count bug #489

merged 3 commits into from
Apr 13, 2022

Conversation

MarinPostma
Copy link
Contributor

fix meilisearch/meilisearch#2152

I think the issue was that we didn't take off the excluded candidates from the initial candidates when returning the candidates with the search result.

@MarinPostma MarinPostma force-pushed the fix-distinct-count-bug branch 2 times, most recently from 37cafbc to 7b15a00 Compare April 9, 2022 12:56
@curquiza
Copy link
Member

curquiza commented Apr 9, 2022

#490 should be merged before this PR since a new check is Required to merge the milli PRs: Specify breaking

ManyTheFish
ManyTheFish previously approved these changes Apr 11, 2022
Copy link
Member

@ManyTheFish ManyTheFish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you @MarinPostma.

Bors merge

bors bot added a commit that referenced this pull request Apr 11, 2022
489: fix distinct count bug r=ManyTheFish a=MarinPostma

fix meilisearch/meilisearch#2152

I think the issue was that we didn't take off the excluded candidates from the initial candidates when returning the candidates with the search result.


Co-authored-by: ad hoc <postma.marin@protonmail.com>
@ManyTheFish ManyTheFish added the no breaking The related changes are not breaking (DB nor API) label Apr 11, 2022
@bors
Copy link
Contributor

bors bot commented Apr 11, 2022

This PR was included in a batch that successfully built, but then failed to merge into main. It will not be retried.

Additional information:

{"message":"Required status check \"Specify breaking\" is expected.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@curquiza
Copy link
Member

bors merge

bors bot added a commit that referenced this pull request Apr 11, 2022
489: fix distinct count bug r=curquiza a=MarinPostma

fix meilisearch/meilisearch#2152

I think the issue was that we didn't take off the excluded candidates from the initial candidates when returning the candidates with the search result.


Co-authored-by: ad hoc <postma.marin@protonmail.com>
@bors
Copy link
Contributor

bors bot commented Apr 11, 2022

This PR was included in a batch that successfully built, but then failed to merge into main. It will not be retried.

Additional information:

{"message":"Required status check \"Specify breaking\" is expected.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@curquiza curquiza added no breaking The related changes are not breaking (DB nor API) and removed no breaking The related changes are not breaking (DB nor API) labels Apr 11, 2022
@curquiza
Copy link
Member

curquiza commented Apr 11, 2022

This PR #492 should fix the bors issue

curquiza
curquiza previously approved these changes Apr 11, 2022
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve since I only rebased the branch

@curquiza
Copy link
Member

bors merge

bors bot added a commit that referenced this pull request Apr 11, 2022
489: fix distinct count bug r=curquiza a=MarinPostma

fix meilisearch/meilisearch#2152

I think the issue was that we didn't take off the excluded candidates from the initial candidates when returning the candidates with the search result.


Co-authored-by: ad hoc <postma.marin@protonmail.com>
@bors
Copy link
Contributor

bors bot commented Apr 11, 2022

This PR was included in a batch that successfully built, but then failed to merge into main. It will not be retried.

Additional information:

{"message":"Required status check \"Specify breaking\" is expected.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

bors bot added a commit that referenced this pull request Apr 11, 2022
492: Add the new `Specify breaking` check to bors.toml r=MarinPostma a=curquiza

Should prevent this problem: #489 (comment)

Co-authored-by: Clémentine Urquizar - curqui <clementine@meilisearch.com>
bors bot added a commit that referenced this pull request Apr 12, 2022
492: Add the new `Specify breaking` check to bors.toml r=curquiza a=curquiza

Should prevent this problem: #489 (comment)

Co-authored-by: Clémentine Urquizar - curqui <clementine@meilisearch.com>
@curquiza
Copy link
Member

bors merge-

bors bot added a commit that referenced this pull request Apr 12, 2022
492: Add the new `Specify breaking` check to bors.toml r=curquiza a=curquiza

Should prevent this problem: #489 (comment)

Co-authored-by: Clémentine Urquizar - curqui <clementine@meilisearch.com>
bors bot added a commit that referenced this pull request Apr 12, 2022
492: Add the new `Specify breaking` check to bors.toml r=curquiza a=curquiza

Should prevent this problem: #489 (comment)

Co-authored-by: Clémentine Urquizar - curqui <clementine@meilisearch.com>
bors bot added a commit that referenced this pull request Apr 13, 2022
492: Add the new `Specify breaking` check to bors.toml r=curquiza a=curquiza

Should prevent this problem: #489 (comment)

Co-authored-by: Clémentine Urquizar - curqui <clementine@meilisearch.com>
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors merge

@bors
Copy link
Contributor

bors bot commented Apr 13, 2022

🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@curquiza
Copy link
Member

bors merge

@bors
Copy link
Contributor

bors bot commented Apr 13, 2022

Already running a review

@bors
Copy link
Contributor

bors bot commented Apr 13, 2022

@bors bors bot merged commit 3828635 into main Apr 13, 2022
@bors bors bot deleted the fix-distinct-count-bug branch April 13, 2022 10:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
no breaking The related changes are not breaking (DB nor API)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong nbHits when using distinct attribute
3 participants