Related PR

This PR is for #673

What does this PR do?

• ~~add Run Clippy job to CI (rust.yml)~~
• apply cargo clippy --fix command
• fix some cargo clippy error manually (but warnings still remain on tests)

PR checklist

Please check if your PR fulfills the following requirements:

• [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• [x] Have you read the contributing guidelines?
• [x] Have you made sure that the title is accurate and descriptive of the changes?

Pull Request

Related issue

Fixes #3167 (https://github.com/meilisearch/meilisearch/issues/3167)

What does this PR do?

• allows Geosearch with zero radius to return the specified location when the coordinates match perfectly (instead of returning nothing). See link for more details.

PR checklist

Please check if your PR fulfills the following requirements:

• [ X ] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• [ X ] Have you read the contributing guidelines?
• [ X ] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

Note: this draft PR is based on https://github.com/meilisearch/milli/pull/601 , for no particular reason.

What does this PR do?

Make a series of changes with the goal of speeding up debug builds:

1. Add an all_languages feature which compiles charabia with its default features activated. The all_languages feature is activated by default. But running:

cargo build --no-default-features

on milli is now much faster.

2. Reduce the debug optimisation level from 3 to 0, except for a few critical dependencies.

3. Compile the build dependencies quicker as well. Previously, all build dependencies were compiled with opt-level = 3. Now, only the critical build dependencies are compiled with optimisations.

4. Reduce the amount of code generated by the documents! macro

5. Make the "progress update" closure provided to indexing functions a trait object instead of a generic parameter. This avoids monomorphising the indexing code multiple times needlessly.

Results

Initial build times on my computer before and after these changes: | | cargo check | cargo check --no-default-features | cargo test | cargo test --lib | cargo test --no-default-features | cargo test --lib --no-default-features | |--------|-------------|-----------------------------------|------------|------------------|----------------------------------|----------------------------------------| | before | 1m05s | 1m05s | 2m06s | 1m47s | 2m06 | 1m47s | | after | 28.9s | 13.1s | 40s | 38s | 23s | 21s |

We reuse the same ~/.cargo and ./target directory between each run on the same OS and rust toolchain. The key to decide if we can use the cache or not is: $OS_NAME-$RUST_TOOLCHAIN-$HASH(Cargo.toml)

We also removed the Cargo.lock from this repository. Indeed, milli is a library and should not track the Cargo.lock

And finally, we enabled the tests on windows-latest. Since lmdb has been updated, this is now possible.

Pull Request

Related issue

Fixes https://github.com/meilisearch/meilisearch/issues/3004

What does this PR do?

• Improves correct error msg in response

PR checklist

Please check if your PR fulfills the following requirements:

• [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• [x] Have you read the contributing guidelines?
• [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

Pull Request

Related issue

Fixes #646

What does this PR do?

• [x] create a new database

  • [x] implementation

• [x] update this database during indexing

  • [x] implementation
  • [x] tests

• [x] update this database during deletion

  • [x] implementation
  • [x] tests

PR checklist

Please check if your PR fulfills the following requirements:

• [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• [x] Have you read the contributing guidelines?
• [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

fix https://github.com/meilisearch/meilisearch/issues/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.

Pull Request

This a WIP draft PR I wanted to create to let other potential contributors know that I'm working on this issue. I'll be completing this in a few hours from opening this.

Related issue

Fixes #661 and towards fixing meilisearch/meilisearch#2905

What does this PR do?

• [x] Change Phrase Operation to use a Vec<Option<String>> instead of Vec<String> where None corresponds to a stop word
• [x] Update all other uses of phrase operation
• [x] Update resolve_phrase
• [x] Update create_primitive_query?
• [x] Add test

PR checklist

Please check if your PR fulfills the following requirements:

• [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• [x] Have you read the contributing guidelines?
• [x] Have you made sure that the title is accurate and descriptive of the changes?

This pr adds support for document batches in milli. It changes the API of the IndexDocuments builder by adding a add_documents method. The API of the updates is changed a little, with the UpdateBuilder being renamed to IndexerConfig and being passed to the update builders. This makes it easier to pass around structs that need to access the indexer config, rather that extracting the fields each time. This change impacts many function signatures and simplify them.

The change in not thorough, and may require another PR to propagate to the whole codebase. I restricted to the necessary for this PR.

Related issue

No issue fixed. Simply cleaning up some code for clippy on the march towards a clean build when #659 is merged.

What does this PR do?

Most of these are calling clone when the struct supports Copy.

Many are using & and &mut on self when the function they are called from already has an immutable or mutable borrow so this isn't needed.

I tried to stay away from actual changes or places where I'd have to name fresh variables.

PR checklist

Please check if your PR fulfills the following requirements:

• [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• [x] Have you read the contributing guidelines?
• [x] Have you made sure that the title is accurate and descriptive of the changes?

I cherry-picked the commits from these PRs

• https://github.com/meilisearch/milli/pull/518
• https://github.com/meilisearch/milli/pull/522
• https://github.com/meilisearch/milli/pull/520

release-v0.26.4 has been started from the tag v0.26.3 and not from main

The release tag v0.26.4 will be done on the branch release-v0.26.4 once this PR is merge

Hi! Please take a look at this PR first. It will need to be merged before this PR can be merged.

In summary, this PR adds support for running milli on iOS and macOS when using App Sandbox, which is required when submitting to the App Store (and required for all iOS apps; macOS apps can get away without needing the App Sandbox but it's recommended).

Under the hood, this is done by adding a cargo feature to use LMDB's POSIX semaphores instead of SysV semaphores (which is LMDB's default). POSIX semaphores comply with Apple's App Sandbox and actually might bring slight performance improvements on *nix platforms.

Refactor and extend the search benchmarks.

1. Use the new search parameter to test the set-based, iterative, and dynamic implementations independently. This allows us to surface performance improvements or regressions that could be hidden in the past. e.g. if there's a regression in the iterative implementation, but the benchmark just happened to use the set-based version all the time, we wouldn't detect the regression.
2. Avoid reindexing the dataset between groups of searches. Instead, the dataset is indexed only once at the beginning, and then the settings are changed between each group of searches if needed.
3. Avoid collecting and analysing the time that each individual search query takes. Instead, collect the total time that the whole group of search requests took. This will speed up the benchmarks and make it easier to read their results. Note that we should then try to group searches by similarity.
4. Add new search requests that are known to have performance problems. Previously, the benchmarks only tested relatively easy search requests.
5. Limit the number of samples used by the benchmarking tool to 10. This will also speed up the benchmarks, at the cost of adding some noise. However, I don't think noise will be a big problem, since we mostly want to detect large changes in performance. And since the benchmarks are now fast, we can also easily re-run them.

The ranking rules proximity, sort, and attribute have two different implementation strategies. The first one (set-based) queries milli's databases and performs set operations on roaring bitmaps to find buckets of document ids. The second one (iterative) iterates on each candidate document and analyse their contents in order to sort them.

Currently, we switch between the set-based and iterative implementation strategy based on the number of candidate documents that need to be sorted. In the proximity criterion, this is done with this constant:

/// Threshold on the number of candidates that will make
/// the system choose between one algorithm or another.
const CANDIDATES_THRESHOLD: u64 = 1000;

There are however, a few problems with this approach:

1. The CANDIDATES_THRESHOLD will always be arbitrary and suboptimal depending on the kind of data that was indexed. Maybe a value of 1000 is the best choice for small documents containing just a few dozen words, but for people with documents that weigh >500 kB, we may opt into the iterative approach too soon and take a heavy performance penalty.

2. We have to maintain two different implementations and update them both whenever we make a change to the behaviour of a ranking rule, which is difficult. It is also difficult to ensure that both implementations are equivalent. In fact, some ranking rules already behave differently depending on the implementation strategy that was chosen. For example, in proximity a difference occurs but only in some specific cases (e.g. when we have documents/queries with consecutive identical words), which is okay. But for attribute, it appears that there is a large difference between the two implementations.

3. It is harder to benchmark search requests correctly. We might make a change in the iterative or set-based version of the algorithm, and then misjudged the impact of the change because the alternative implementation is used instead. (This is partly fixed by https://github.com/meilisearch/milli/pull/742 ).

4. It is also harder to detect bugs in the implementation of the ranking rules, for the same reason as in (3)

Ideally, when we refactor the search algorithms, we should aim to make the set-based strategy fast enough such that it is reasonable to use it even when sorting only two candidate documents. It would allow us to reduce the size of the code base and make performance/correctness problems more visible.

Additionally, if we remove the iterative versions of the proximity and attribute ranking rules, we can also remove the docid_word_positions database, which will reduce the size of the index.

Pull Request

Related issue

Related to https://github.com/meilisearch/meilisearch/issues/3233

What does this PR do?

User PoV

• Change primary key inference to only consider a value as a candidate when it ends with "id", rather than when it simply contains "id".
• Change primary key inference to always fail when there are multiple candidates.
• Replace UserError::MissingPrimaryKey with UserError::NoPrimaryKeyCandidateFound and UserError::MultiplePrimaryKeyCandidatesFound

Implementation-wise

• Remove uses of UserError::MissingPrimaryKey not pertaining to inference. This introduces a possible panicking path.

PR checklist

Please check if your PR fulfills the following requirements:

• [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• [x] Have you read the contributing guidelines?
• [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

related to https://github.com/meilisearch/milli/issues/682

TODO

• [x] Extract detected Language during indexing and store it in a dedicated database
• [ ] at search time: Use this database to create an allow_list and pass it to the TokenizerBuilder of Charabia
• [ ] ensure that https://github.com/meilisearch/meilisearch/issues/2403 is fixed

Pull Request

Related issue

Fixes (when merged into meilisearch) https://github.com/meilisearch/meilisearch/issues/3222

Implementation

The design is described well in the related issue. For details of how different relative positions are grouped together, see the test bucketed_position.

Basically, we no longer store the exact position of words that appear far into an attribute, but instead group relative positions together in buckets whose size grows exponentially with the original position. This is done to improve the relevancy and the performance of the attribute ranking rule.

This is a draft until https://github.com/meilisearch/milli/pull/742 is merged and the results of the benchmarks are available.

EDIT: I also realised just now that the iterative version of the algorithm needs to be updated as well!