loiclec commented 1 year ago

Pull Request

Related issue

Fixes (partially, until merged into meilisearch) https://github.com/meilisearch/meilisearch/issues/3178

What does this PR do?

The most important change is this one:

    // 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).

Kerollmops commented 1 year ago

bors merge

bors[bot] commented 1 year ago

Build succeeded:

meilisearch / milli

Fix bug in filter search #727

Pull Request

Related issue

What does this PR do?