meilisearch / milli

Search engine library for Meilisearch ⚡️
MIT License
464 stars 81 forks source link

Add all_obkv_to_json function #707

Closed GregoryConrad closed 1 year ago

GregoryConrad commented 1 year ago

What does this PR do?

When embedding milli in an application (other than Meilisearch), it often makes sense to not use the displayed_attributes functionality and instead just use milli as a full document store. Thus, this PR adds a function, all_obkv_to_json, to supplement the already exposed milli::obkv_to_json so that those embedding milli do not need to deal with displayed_attributes if they don't need to.

~This PR also introduces a slight breaking change: obkv_to_json now accepts a reference to obkv::KvReaderU16 instead of taking ownership of it. As far as I can tell, this seems like a change for the better (obkv_to_json only acts upon obkv rather than consuming it), but I can change it back if you so desire.~ (reverted in 935a724)

PR checklist

Please check if your PR fulfills the following requirements:

Thank you so much for contributing to Meilisearch!

Kerollmops commented 1 year ago

Hey @GregoryConrad, Would it be possible to quickly fix the Clippy CI in a dedicated commit, please?

bors[bot] commented 1 year ago

Build succeeded:

GregoryConrad commented 1 year ago

Hey @GregoryConrad, Would it be possible to quickly fix the Clippy CI in a dedicated commit, please?

Hi @Kerollmops, cargo clippy runs fine on my machine, and it looks ok in CI based on that bors message? Am I overlooking something?

Kerollmops commented 1 year ago

Hum... strange, it looks like the issue was in the filter-parser crate. Are you sure you use the latest version of Rust? i.e. rustup update.

GregoryConrad commented 1 year ago

@Kerollmops I still can't replicate:

Screenshot 2022-11-28 at 11 08 31 AM

Would you mind copy-pasting the clippy output you have from filter-parser so I can fix it? Perhaps it is on an outdated commit (before I reverted the pass-by-reference changes I made)?

Kerollmops commented 1 year ago

Hey @GregoryConrad, sorry for the late reply. I just tried to trigger the Clippy error again and could not output it again 🎉