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.

Sign up for GitHub

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

Jump to bottom

Search using filters not working correctly #3178

Closed
3 tasks
roparz opened this issue Dec 1, 2022 · 6 comments · Fixed by #3202 or #3217
Closed
3 tasks

Search using filters not working correctly #3178

roparz opened this issue Dec 1, 2022 · 6 comments · Fixed by #3202 or #3217
Assignees
@loiclec
Labels
bug Something isn't working as expected milli Related to the milli workspace v0.30.2 PRs/issues solved in v0.30.2 released on 2022-12-14
Milestone
v0.30.2

Comments

@roparz
Copy link

roparz commented Dec 1, 2022 •
edited by curquiza

Describe the bug
I have an index of users with ~250k documents.
All documents are correctly imported (number of documents in /stats reflects the count in my DB).
When I search for users using filters, with different values, sometimes it returns the expected documents, sometimes an empty result.

To Reproduce
Steps to reproduce the behavior:

  1. With an index of ~250k documents looking like the following:
{
  "fullname":"xxxxxx",
  "email":"xxxxx",
  "company_number":"0123456789",
  "emails":["xxxxxx"],
  "id":246500,
  "firstname":"xxx",
  "lastname":"xxx",
  "fk_active_company":273, <= ~600 différent values
  "email_verified_at":null,
  "state":"guest",
  "status":"employee",
  "company":"xxx",
  "is_deleted":false
}
  1. GET a user using it's ID. It returns the document.
curl -X GET "http://localhost:7700/indexes/users/documents/246500"
  1. Search for users using fk_active_company filter. Result is empty.
curl \
  -H "Content-Type: application/json"\
  -X POST "http://localhost:7700/indexes/users/search" \
  --data-binary '{ "filter": [ "fk_active_company = 273" ] }'

{"hits":[],"query":"","processingTimeMs":0,"limit":20,"offset":0,"estimatedTotalHits":0}

Expected behavior
Search with filters should work at any time.

Meilisearch version: v0.30.0

Additional context
Infra: AWS Meilisearch AMI v0.30.0 on a c5.large instance.

TODO

  • Implement changes in Milli
  • Release a Milli version containing these changes
  • Bump this new Milli version in Meilisearch and merge it into main
@roparz roparz mentioned this issue Dec 1, 2022
Closed
@curquiza curquiza added the support Issues related to support questions label Dec 1, 2022
@ManyTheFish ManyTheFish added bug Something isn't working as expected and removed support Issues related to support questions labels Dec 1, 2022
@ManyTheFish
Copy link
Member

ManyTheFish commented Dec 1, 2022 •
edited

Elevate this issue from support to bug because @imkarthikk faced the same problem.

@imkarthikk, don't hesitate to give us additional information about your own issue.

related to several discord support discussions:

@curquiza curquiza added this to the v0.30.1 milestone Dec 1, 2022
@loiclec
Copy link
Contributor

loiclec commented Dec 5, 2022

I'll try to double-check today to be sure, but I think this bug is caused by the same "corrupted database" issue causing #3165 and fixed by meilisearch/milli#712

@oluademola oluademola mentioned this issue Dec 5, 2022
Closed
@curquiza curquiza added the milli Related to the milli workspace label Dec 6, 2022
@Kerollmops Kerollmops mentioned this issue Dec 6, 2022
Merged
@Kerollmops Kerollmops linked a pull request Dec 6, 2022 that will close this issue
Bump milli to v0.37.1 #3202
Merged
bors bot added a commit that referenced this issue Dec 6, 2022
@bors @Kerollmops @curquiza
Merge #3202 #3203
58cd5d2 
3202: Bump milli to v0.37.1 r=curquiza a=Kerollmops

This PR bumps milli to v0.37.1 and fixes #3167, #3178, #3165, and #3021.

3203: Update version for the next release (v0.30.1) in Cargo.toml files r=Kerollmops a=meili-bot

⚠️ This PR is automatically generated. Check the new version is the expected one before merging.

Co-authored-by: Kerollmops <clement@meilisearch.com>
Co-authored-by: curquiza <curquiza@users.noreply.github.com>
@curquiza
Copy link
Member

curquiza commented Dec 6, 2022

Fixed by #3202, will be released in v0.30.1 today!

@curquiza curquiza closed this as completed Dec 6, 2022
@imkarthikk
Copy link

@curquiza Incredible. Thanks for the effort guys 👏

@meili-bot meili-bot added the v0.30.1 PRs/issues solved in v0.30.1 released on 2022-12-08 label Dec 7, 2022
@loiclec
Copy link
Contributor

loiclec commented Dec 7, 2022

Hi everyone, I am very sorry, but the update to v0.30.1 did not completely fix this issue. Most of the missing filter results were indeed caused by a corrupted database, as I suspected. However, I then found another bug in the search algorithms that could cause a filter to miss some documents. We will quickly prepare a v0.30.2 (sigh...) containing a fix for this new bug as well. Thank you so much for the bug reports, we will try to do better in the future ❤️

@loiclec loiclec reopened this Dec 7, 2022
@loiclec loiclec added v0.30.0 PRs/issues solved in v0.30.0 released on 2022-11-28 v0.30.2 and removed v0.30.1 PRs/issues solved in v0.30.1 released on 2022-12-08 labels Dec 7, 2022
@loiclec loiclec self-assigned this Dec 7, 2022
@loiclec loiclec modified the milestones: v0.30.1, v0.30.2 Dec 7, 2022
@loiclec loiclec removed the v0.30.0 PRs/issues solved in v0.30.0 released on 2022-11-28 label Dec 7, 2022
@loiclec loiclec mentioned this issue Dec 7, 2022
Merged
@curquiza curquiza removed the v0.30.2 label Dec 7, 2022
bors bot added a commit to meilisearch/milli that referenced this issue Dec 7, 2022
@bors @loiclec
Merge #727
098c410 
727: Fix bug in filter search r=Kerollmops a=loiclec

# Pull Request

## Related issue
Fixes (partially, until merged into meilisearch) meilisearch/meilisearch#3178

## What does this PR do?
The most important change is this one:
```rust
    // in milli/src/search/facet/facet_range_search.rs, line 239
    let should_stop = {
        match self.right {
            Bound::Included(right) => right < previous_key.left_bound,
            Bound::Excluded(right) => right <= previous_key.left_bound,
            Bound::Unbounded => false,
        }
    };
```
where the operations `<` and `<=` between the two branches were switched. This caused (very few) documents to be missing from filter results.

The second change is a simplification of the algorithm for filters such as `field = value`, where we now perform a direct query into the "Level 0" of the facet db to retrieve the docids instead of invoking the full facet search algorithm. This change is done in `milli/src/search/facet/filter.rs`.

I have added yet more insta-snapshot tests, rechecked the content of the snapshots, and added some integration tests as well. 

This is purely a fix in the search algorithms. Based on this PR alone, a dump will not be necessary to switch from v0.30.1 (where this bug is present) to v0.30.2 (where this PR is merged).


Co-authored-by: Loïc Lecrenier <loic.lecrenier@me.com>
@curquiza curquiza linked a pull request Dec 8, 2022 that will close this issue
Bump milli to v0.37.2 #3217
Merged
@curquiza
Copy link
Member

curquiza commented Dec 8, 2022

Fixed by #3217, will be released today in v0.30.2

@curquiza curquiza closed this as completed Dec 8, 2022
@meili-bot meili-bot added the v0.30.2 PRs/issues solved in v0.30.2 released on 2022-12-14 label Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@loiclec loiclec

Labels
bug Something isn't working as expected milli Related to the milli workspace v0.30.2 PRs/issues solved in v0.30.2 released on 2022-12-14
Projects
None yet
Milestone
v0.30.2
Development

Successfully merging a pull request may close this issue.

Bump milli to v0.37.1 meilisearch/meilisearch
Bump milli to v0.37.2 meilisearch/meilisearch
6 participants
@roparz @imkarthikk @loiclec @ManyTheFish @curquiza @meili-bot