From b1bf6722e8d3cc91ead999e8badd3d086e6a3d83 Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Mon, 11 Jul 2022 17:27:07 +0200 Subject: [PATCH 01/18] Update API to fit the proto needs --- .../src/analytics/segment_analytics.rs | 16 +++- meilisearch-http/src/routes/indexes/search.rs | 12 ++- meilisearch-lib/src/index/mod.rs | 5 +- meilisearch-lib/src/index/search.rs | 76 ++++++++++++++++--- 4 files changed, 88 insertions(+), 21 deletions(-) diff --git a/meilisearch-http/src/analytics/segment_analytics.rs b/meilisearch-http/src/analytics/segment_analytics.rs index e4dfac217..2c56530ae 100644 --- a/meilisearch-http/src/analytics/segment_analytics.rs +++ b/meilisearch-http/src/analytics/segment_analytics.rs @@ -10,7 +10,7 @@ use http::header::CONTENT_TYPE; use meilisearch_auth::SearchRules; use meilisearch_lib::index::{ SearchQuery, SearchResult, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, - DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, + DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_SEARCH_LIMIT, }; use meilisearch_lib::index_controller::Stats; use meilisearch_lib::MeiliSearch; @@ -373,6 +373,7 @@ pub struct SearchAggregator { // pagination max_limit: usize, max_offset: usize, + finite_pagination: bool, // formatting highlight_pre_tag: bool, @@ -427,12 +428,19 @@ impl SearchAggregator { ret.max_terms_number = q.split_whitespace().count(); } + if query.limit.is_none() && query.offset.is_none() { + ret.max_limit = query.hits_per_page; + ret.max_offset = query.page.saturating_sub(1) * query.hits_per_page; + ret.finite_pagination = true; + } else { + ret.max_limit = query.limit.unwrap_or_else(DEFAULT_SEARCH_LIMIT); + ret.max_offset = query.offset.unwrap_or_default(); + ret.finite_pagination = false; + } + ret.matching_strategy .insert(format!("{:?}", query.matching_strategy), 1); - ret.max_limit = query.limit; - ret.max_offset = query.offset.unwrap_or_default(); - ret.highlight_pre_tag = query.highlight_pre_tag != DEFAULT_HIGHLIGHT_PRE_TAG(); ret.highlight_post_tag = query.highlight_post_tag != DEFAULT_HIGHLIGHT_POST_TAG(); ret.crop_marker = query.crop_marker != DEFAULT_CROP_MARKER(); diff --git a/meilisearch-http/src/routes/indexes/search.rs b/meilisearch-http/src/routes/indexes/search.rs index 973d5eb6e..828fb40ad 100644 --- a/meilisearch-http/src/routes/indexes/search.rs +++ b/meilisearch-http/src/routes/indexes/search.rs @@ -2,8 +2,8 @@ use actix_web::{web, HttpRequest, HttpResponse}; use log::debug; use meilisearch_auth::IndexSearchRules; use meilisearch_lib::index::{ - MatchingStrategy, SearchQuery, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, - DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_SEARCH_LIMIT, + SearchQuery, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, DEFAULT_HIGHLIGHT_POST_TAG, + DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_HIT_PER_PAGE, DEFAULT_PAGE, MatchingStrategy }; use meilisearch_lib::MeiliSearch; use meilisearch_types::error::ResponseError; @@ -29,6 +29,10 @@ pub struct SearchQueryGet { q: Option, offset: Option, limit: Option, + #[serde(default = "DEFAULT_PAGE")] + page: usize, + #[serde(default = "DEFAULT_HIT_PER_PAGE")] + hits_per_page: usize, attributes_to_retrieve: Option>, attributes_to_crop: Option>, #[serde(default = "DEFAULT_CROP_LENGTH")] @@ -62,7 +66,9 @@ impl From for SearchQuery { Self { q: other.q, offset: other.offset, - limit: other.limit.unwrap_or_else(DEFAULT_SEARCH_LIMIT), + limit: other.limit, + page: other.page, + hits_per_page: other.hits_per_page, attributes_to_retrieve: other .attributes_to_retrieve .map(|o| o.into_iter().collect()), diff --git a/meilisearch-lib/src/index/mod.rs b/meilisearch-lib/src/index/mod.rs index 98c25366d..283e44294 100644 --- a/meilisearch-lib/src/index/mod.rs +++ b/meilisearch-lib/src/index/mod.rs @@ -1,6 +1,7 @@ pub use search::{ - MatchingStrategy, SearchQuery, SearchResult, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, - DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_SEARCH_LIMIT, + SearchQuery, SearchResult, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, + DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_HIT_PER_PAGE, DEFAULT_PAGE, + DEFAULT_SEARCH_LIMIT, MatchingStrategy }; pub use updates::{apply_settings_to_builder, Checked, Facets, Settings, Unchecked}; diff --git a/meilisearch-lib/src/index/search.rs b/meilisearch-lib/src/index/search.rs index 1a9aa1d0d..44ece9f2f 100644 --- a/meilisearch-lib/src/index/search.rs +++ b/meilisearch-lib/src/index/search.rs @@ -1,4 +1,4 @@ -use std::cmp::min; +use std::cmp::{max, min}; use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::str::FromStr; use std::time::Instant; @@ -26,6 +26,8 @@ pub const DEFAULT_CROP_LENGTH: fn() -> usize = || 10; pub const DEFAULT_CROP_MARKER: fn() -> String = || "…".to_string(); pub const DEFAULT_HIGHLIGHT_PRE_TAG: fn() -> String = || "".to_string(); pub const DEFAULT_HIGHLIGHT_POST_TAG: fn() -> String = || "".to_string(); +pub const DEFAULT_PAGE: fn() -> usize = || 1; +pub const DEFAULT_HIT_PER_PAGE: fn() -> usize = || 20; /// The maximum number of results that the engine /// will be able to return in one search call. @@ -36,8 +38,11 @@ pub const DEFAULT_PAGINATION_MAX_TOTAL_HITS: usize = 1000; pub struct SearchQuery { pub q: Option, pub offset: Option, - #[serde(default = "DEFAULT_SEARCH_LIMIT")] - pub limit: usize, + pub limit: Option, + #[serde(default = "DEFAULT_PAGE")] + pub page: usize, + #[serde(default = "DEFAULT_HIT_PER_PAGE")] + pub hits_per_page: usize, pub attributes_to_retrieve: Option>, pub attributes_to_crop: Option>, #[serde(default = "DEFAULT_CROP_LENGTH")] @@ -97,15 +102,30 @@ pub struct SearchHit { #[serde(rename_all = "camelCase")] pub struct SearchResult { pub hits: Vec, - pub estimated_total_hits: u64, pub query: String, - pub limit: usize, - pub offset: usize, pub processing_time_ms: u128, + #[serde(flatten)] + pub hits_info: HitsInfo, #[serde(skip_serializing_if = "Option::is_none")] pub facet_distribution: Option>>, } +#[derive(Serialize, Debug, Clone, PartialEq)] +#[serde(rename_all = "camelCase")] +#[serde(untagged)] +pub enum HitsInfo { + Pagination { + hits_per_page: usize, + page: usize, + total_pages: usize, + }, + OffsetLimit { + limit: usize, + offset: usize, + estimated_total_hits: usize, + }, +} + impl Index { pub fn perform_search(&self, query: SearchQuery) -> Result { let before_search = Instant::now(); @@ -125,8 +145,26 @@ impl Index { // Make sure that a user can't get more documents than the hard limit, // we align that on the offset too. - let offset = min(query.offset.unwrap_or(0), max_total_hits); - let limit = min(query.limit, max_total_hits.saturating_sub(offset)); + let is_finite_pagination = query.offset.is_none() && query.limit.is_none(); + + let (offset, limit) = if is_finite_pagination { + // we start at least at page 1. + let page = max(query.page, 1); + // return at least 1 document. + let hits_per_page = max(query.hits_per_page, 1); + let offset = min(hits_per_page * (page - 1), max_total_hits); + let limit = min(hits_per_page, max_total_hits.saturating_sub(offset)); + + (offset, limit) + } else { + let offset = min(query.offset.unwrap_or(0), max_total_hits); + let limit = min( + query.limit.unwrap_or_else(DEFAULT_SEARCH_LIMIT), + max_total_hits.saturating_sub(offset), + ); + + (offset, limit) + }; search.offset(offset); search.limit(limit); @@ -251,7 +289,23 @@ impl Index { documents.push(hit); } - let estimated_total_hits = candidates.len(); + let number_of_hits = min(candidates.len() as usize, max_total_hits); + let hits_info = if is_finite_pagination { + // return at least 1 document. + let hits_per_page = max(query.hits_per_page, 1); + HitsInfo::Pagination { + hits_per_page, + page: offset / hits_per_page + 1, + // TODO @many: estimation for now but we should ask milli to return an exact value + total_pages: (number_of_hits + hits_per_page - 1) / query.hits_per_page, + } + } else { + HitsInfo::OffsetLimit { + limit: query.limit.unwrap_or_else(DEFAULT_SEARCH_LIMIT), + offset, + estimated_total_hits: number_of_hits, + } + }; let facet_distribution = match query.facets { Some(ref fields) => { @@ -274,10 +328,8 @@ impl Index { let result = SearchResult { hits: documents, - estimated_total_hits, + hits_info, query: query.q.clone().unwrap_or_default(), - limit: query.limit, - offset: query.offset.unwrap_or_default(), processing_time_ms: before_search.elapsed().as_millis(), facet_distribution, }; From 30410e870f11935532b025279063eef8580ab6ba Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Tue, 12 Jul 2022 17:11:59 +0200 Subject: [PATCH 02/18] Format all fields in camelCase --- meilisearch-lib/src/index/search.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/meilisearch-lib/src/index/search.rs b/meilisearch-lib/src/index/search.rs index 44ece9f2f..083f64292 100644 --- a/meilisearch-lib/src/index/search.rs +++ b/meilisearch-lib/src/index/search.rs @@ -111,14 +111,15 @@ pub struct SearchResult { } #[derive(Serialize, Debug, Clone, PartialEq)] -#[serde(rename_all = "camelCase")] #[serde(untagged)] pub enum HitsInfo { + #[serde(rename_all = "camelCase")] Pagination { hits_per_page: usize, page: usize, total_pages: usize, }, + #[serde(rename_all = "camelCase")] OffsetLimit { limit: usize, offset: usize, From 062d17fbc0e9f9c80b234cd210bc71a912b80989 Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Wed, 13 Jul 2022 10:58:39 +0200 Subject: [PATCH 03/18] Use a milli version that compute exhaustivelly the number of hits --- Cargo.lock | 37 +++++++++++++++++++---------- meilisearch-auth/Cargo.toml | 2 +- meilisearch-lib/Cargo.toml | 2 +- meilisearch-lib/src/index/search.rs | 2 ++ 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1292741a5..e5fd78adb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1184,7 +1184,7 @@ dependencies = [ [[package]] name = "filter-parser" version = "0.33.4" -source = "git+https://github.com/meilisearch/milli.git?tag=v0.33.4#4fc6331cb6526c07f3137584564cfe3493fb25bd" +source = "git+https://github.com/meilisearch/milli.git?branch=main#19b2326f3dd3b85daa00cf1c752797f4b11c536b" dependencies = [ "nom", "nom_locate", @@ -1203,7 +1203,7 @@ dependencies = [ [[package]] name = "flatten-serde-json" version = "0.33.4" -source = "git+https://github.com/meilisearch/milli.git?tag=v0.33.4#4fc6331cb6526c07f3137584564cfe3493fb25bd" +source = "git+https://github.com/meilisearch/milli.git?branch=main#19b2326f3dd3b85daa00cf1c752797f4b11c536b" dependencies = [ "serde_json", ] @@ -1360,9 +1360,9 @@ dependencies = [ [[package]] name = "geoutils" -version = "0.4.1" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e006f616a407d396ace1d2ebb3f43ed73189db8b098079bd129928d7645dd1e" +checksum = "36d244a08113319b5ebcabad2b8b7925732d15eec46d7e7ac3c11734f3b7a6ad" [[package]] name = "getrandom" @@ -1714,7 +1714,7 @@ dependencies = [ [[package]] name = "json-depth-checker" version = "0.33.4" -source = "git+https://github.com/meilisearch/milli.git?tag=v0.33.4#4fc6331cb6526c07f3137584564cfe3493fb25bd" +source = "git+https://github.com/meilisearch/milli.git?branch=main#19b2326f3dd3b85daa00cf1c752797f4b11c536b" dependencies = [ "serde_json", ] @@ -2195,7 +2195,7 @@ dependencies = [ "rayon", "regex", "reqwest", - "roaring", + "roaring 0.9.0", "rustls", "serde", "serde_json", @@ -2250,11 +2250,11 @@ dependencies = [ [[package]] name = "milli" version = "0.33.4" -source = "git+https://github.com/meilisearch/milli.git?tag=v0.33.4#4fc6331cb6526c07f3137584564cfe3493fb25bd" +source = "git+https://github.com/meilisearch/milli.git?branch=main#19b2326f3dd3b85daa00cf1c752797f4b11c536b" dependencies = [ "bimap", "bincode", - "bstr 0.2.17", + "bstr 1.0.1", "byteorder", "charabia", "concat-arrays", @@ -2278,7 +2278,7 @@ dependencies = [ "once_cell", "ordered-float", "rayon", - "roaring", + "roaring 0.10.1", "rstar", "serde", "serde_json", @@ -2506,9 +2506,9 @@ checksum = "624a8340c38c1b80fd549087862da4ba43e08858af025b236e509b6649fc13d5" [[package]] name = "ordered-float" -version = "2.10.0" +version = "3.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7940cf2ca942593318d07fcf2596cdca60a85c9e7fab408a5e21a4f9dcd40d87" +checksum = "1f74e330193f90ec45e2b257fa3ef6df087784157ac1ad2c1e71c62837b03aa7" dependencies = [ "num-traits", ] @@ -3015,9 +3015,9 @@ dependencies = [ [[package]] name = "retain_mut" -version = "0.1.9" +version = "0.1.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4389f1d5789befaf6029ebd9f7dac4af7f7e3d61b69d4f30e2ac02b57e7712b0" +checksum = "8c31b5c4033f8fdde8700e4657be2c497e7288f01515be52168c631e2e4d4086" [[package]] name = "ring" @@ -3066,6 +3066,17 @@ dependencies = [ "retain_mut", ] +[[package]] +name = "roaring" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ef0fb5e826a8bde011ecae6a8539dd333884335c57ff0f003fbe27c25bbe8f71" +dependencies = [ + "bytemuck", + "byteorder", + "retain_mut", +] + [[package]] name = "rstar" version = "0.9.3" diff --git a/meilisearch-auth/Cargo.toml b/meilisearch-auth/Cargo.toml index cd2c88bdd..828203e7e 100644 --- a/meilisearch-auth/Cargo.toml +++ b/meilisearch-auth/Cargo.toml @@ -7,7 +7,7 @@ edition = "2021" enum-iterator = "1.1.2" hmac = "0.12.1" meilisearch-types = { path = "../meilisearch-types" } -milli = { git = "https://github.com/meilisearch/milli.git", tag = "v0.33.4", default-features = false } +milli = { git = "https://github.com/meilisearch/milli.git", branch = "main", default-features = false } rand = "0.8.5" serde = { version = "1.0.145", features = ["derive"] } serde_json = { version = "1.0.85", features = ["preserve_order"] } diff --git a/meilisearch-lib/Cargo.toml b/meilisearch-lib/Cargo.toml index c48a7bdf7..099c5528c 100644 --- a/meilisearch-lib/Cargo.toml +++ b/meilisearch-lib/Cargo.toml @@ -28,7 +28,7 @@ lazy_static = "1.4.0" log = "0.4.17" meilisearch-auth = { path = "../meilisearch-auth" } meilisearch-types = { path = "../meilisearch-types" } -milli = { git = "https://github.com/meilisearch/milli.git", tag = "v0.33.4", default-features = false } +milli = { git = "https://github.com/meilisearch/milli.git", branch = "main", default-features = false } mime = "0.3.16" num_cpus = "1.13.1" obkv = "0.2.0" diff --git a/meilisearch-lib/src/index/search.rs b/meilisearch-lib/src/index/search.rs index 083f64292..0089e8e12 100644 --- a/meilisearch-lib/src/index/search.rs +++ b/meilisearch-lib/src/index/search.rs @@ -148,6 +148,8 @@ impl Index { // we align that on the offset too. let is_finite_pagination = query.offset.is_none() && query.limit.is_none(); + search.exhaustive_number_hits(is_finite_pagination); + let (offset, limit) = if is_finite_pagination { // we start at least at page 1. let page = max(query.page, 1); From 0fa5c9b51592d16578c212ef02c4268398567112 Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Mon, 18 Jul 2022 18:12:22 +0200 Subject: [PATCH 04/18] Fix tests --- meilisearch-lib/src/index/mod.rs | 2 +- meilisearch-lib/src/index_controller/mod.rs | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/meilisearch-lib/src/index/mod.rs b/meilisearch-lib/src/index/mod.rs index 283e44294..08f1f42d5 100644 --- a/meilisearch-lib/src/index/mod.rs +++ b/meilisearch-lib/src/index/mod.rs @@ -1,5 +1,5 @@ pub use search::{ - SearchQuery, SearchResult, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, + HitsInfo, SearchQuery, SearchResult, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_HIT_PER_PAGE, DEFAULT_PAGE, DEFAULT_SEARCH_LIMIT, MatchingStrategy }; diff --git a/meilisearch-lib/src/index_controller/mod.rs b/meilisearch-lib/src/index_controller/mod.rs index be855300b..b5a1daef6 100644 --- a/meilisearch-lib/src/index_controller/mod.rs +++ b/meilisearch-lib/src/index_controller/mod.rs @@ -659,7 +659,7 @@ mod test { use nelson::Mocker; use crate::index::error::Result as IndexResult; - use crate::index::Index; + use crate::index::{HitsInfo, Index}; use crate::index::{ DEFAULT_CROP_MARKER, DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, }; @@ -692,7 +692,9 @@ mod test { let query = SearchQuery { q: Some(String::from("hello world")), offset: Some(10), - limit: 0, + limit: Some(0), + page: 1, + hits_per_page: 10, attributes_to_retrieve: Some(vec!["string".to_owned()].into_iter().collect()), attributes_to_crop: None, crop_length: 18, @@ -709,10 +711,12 @@ mod test { let result = SearchResult { hits: vec![], - estimated_total_hits: 29, query: "hello world".to_string(), - limit: 24, - offset: 0, + hits_info: HitsInfo::OffsetLimit { + limit: 24, + offset: 0, + estimated_total_hits: 29, + }, processing_time_ms: 50, facet_distribution: None, }; From e9d493c0526827a035f50364b51dcec767382b04 Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Wed, 20 Jul 2022 16:44:09 +0200 Subject: [PATCH 05/18] Add a totalHits field on finite pagination return --- meilisearch-lib/src/index/search.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/meilisearch-lib/src/index/search.rs b/meilisearch-lib/src/index/search.rs index 0089e8e12..11e038126 100644 --- a/meilisearch-lib/src/index/search.rs +++ b/meilisearch-lib/src/index/search.rs @@ -118,6 +118,7 @@ pub enum HitsInfo { hits_per_page: usize, page: usize, total_pages: usize, + total_hits: usize, }, #[serde(rename_all = "camelCase")] OffsetLimit { @@ -299,8 +300,8 @@ impl Index { HitsInfo::Pagination { hits_per_page, page: offset / hits_per_page + 1, - // TODO @many: estimation for now but we should ask milli to return an exact value total_pages: (number_of_hits + hits_per_page - 1) / query.hits_per_page, + total_hits: number_of_hits, } } else { HitsInfo::OffsetLimit { From 815fba9cc3f3a325d8f3d765431c3cb38aeaa0f9 Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Thu, 21 Jul 2022 16:10:03 +0200 Subject: [PATCH 06/18] Fix zero division when computing pages --- meilisearch-lib/src/index/search.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meilisearch-lib/src/index/search.rs b/meilisearch-lib/src/index/search.rs index 11e038126..796c612b2 100644 --- a/meilisearch-lib/src/index/search.rs +++ b/meilisearch-lib/src/index/search.rs @@ -300,7 +300,7 @@ impl Index { HitsInfo::Pagination { hits_per_page, page: offset / hits_per_page + 1, - total_pages: (number_of_hits + hits_per_page - 1) / query.hits_per_page, + total_pages: (number_of_hits + hits_per_page - 1) / hits_per_page, total_hits: number_of_hits, } } else { From dfa70e47f7679f0fbcdf4af86b6353ee43d87924 Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Thu, 21 Jul 2022 17:42:42 +0200 Subject: [PATCH 07/18] Change page and hitsPerPage corner cases --- meilisearch-lib/src/index/search.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/meilisearch-lib/src/index/search.rs b/meilisearch-lib/src/index/search.rs index 796c612b2..62b21c2cc 100644 --- a/meilisearch-lib/src/index/search.rs +++ b/meilisearch-lib/src/index/search.rs @@ -1,4 +1,4 @@ -use std::cmp::{max, min}; +use std::cmp::min; use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::str::FromStr; use std::time::Instant; @@ -152,12 +152,11 @@ impl Index { search.exhaustive_number_hits(is_finite_pagination); let (offset, limit) = if is_finite_pagination { - // we start at least at page 1. - let page = max(query.page, 1); - // return at least 1 document. - let hits_per_page = max(query.hits_per_page, 1); - let offset = min(hits_per_page * (page - 1), max_total_hits); - let limit = min(hits_per_page, max_total_hits.saturating_sub(offset)); + let offset = min( + query.hits_per_page * (query.page.saturating_sub(1)), + max_total_hits, + ); + let limit = min(query.hits_per_page, max_total_hits.saturating_sub(offset)); (offset, limit) } else { @@ -295,12 +294,15 @@ impl Index { let number_of_hits = min(candidates.len() as usize, max_total_hits); let hits_info = if is_finite_pagination { - // return at least 1 document. - let hits_per_page = max(query.hits_per_page, 1); + // If hit_per_page is 0, then pages can't be computed and so we respond 0. + let total_pages = (number_of_hits + query.hits_per_page.saturating_sub(1)) + .checked_div(query.hits_per_page) + .unwrap_or(0); + HitsInfo::Pagination { - hits_per_page, - page: offset / hits_per_page + 1, - total_pages: (number_of_hits + hits_per_page - 1) / hits_per_page, + hits_per_page: query.hits_per_page, + page: query.page, + total_pages, total_hits: number_of_hits, } } else { From e35ea2ad557065523ab14072cbc91e281ae9a7bd Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Thu, 21 Jul 2022 18:03:39 +0200 Subject: [PATCH 08/18] Make search returns 0 hits when pages is set to 0 --- meilisearch-lib/src/index/search.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/meilisearch-lib/src/index/search.rs b/meilisearch-lib/src/index/search.rs index 62b21c2cc..9badd9cb6 100644 --- a/meilisearch-lib/src/index/search.rs +++ b/meilisearch-lib/src/index/search.rs @@ -152,13 +152,16 @@ impl Index { search.exhaustive_number_hits(is_finite_pagination); let (offset, limit) = if is_finite_pagination { - let offset = min( - query.hits_per_page * (query.page.saturating_sub(1)), - max_total_hits, - ); - let limit = min(query.hits_per_page, max_total_hits.saturating_sub(offset)); + match query.page.checked_sub(1) { + Some(page) => { + let offset = min(query.hits_per_page * page, max_total_hits); + let limit = min(query.hits_per_page, max_total_hits.saturating_sub(offset)); - (offset, limit) + (offset, limit) + } + // page 0 returns 0 hits + None => (0, 0), + } } else { let offset = min(query.offset.unwrap_or(0), max_total_hits); let limit = min( From 77e718214fb5bca1463970ca0afc76300abac8ef Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Mon, 8 Aug 2022 15:20:03 +0200 Subject: [PATCH 09/18] Fix pagination analytics --- meilisearch-http/src/analytics/segment_analytics.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/meilisearch-http/src/analytics/segment_analytics.rs b/meilisearch-http/src/analytics/segment_analytics.rs index 2c56530ae..726d0945a 100644 --- a/meilisearch-http/src/analytics/segment_analytics.rs +++ b/meilisearch-http/src/analytics/segment_analytics.rs @@ -373,7 +373,7 @@ pub struct SearchAggregator { // pagination max_limit: usize, max_offset: usize, - finite_pagination: bool, + finite_pagination: usize, // formatting highlight_pre_tag: bool, @@ -431,11 +431,11 @@ impl SearchAggregator { if query.limit.is_none() && query.offset.is_none() { ret.max_limit = query.hits_per_page; ret.max_offset = query.page.saturating_sub(1) * query.hits_per_page; - ret.finite_pagination = true; + ret.finite_pagination = 1; } else { ret.max_limit = query.limit.unwrap_or_else(DEFAULT_SEARCH_LIMIT); ret.max_offset = query.offset.unwrap_or_default(); - ret.finite_pagination = false; + ret.finite_pagination = 0; } ret.matching_strategy @@ -499,6 +499,7 @@ impl SearchAggregator { // pagination self.max_limit = self.max_limit.max(other.max_limit); self.max_offset = self.max_offset.max(other.max_offset); + self.finite_pagination += other.finite_pagination; self.highlight_pre_tag |= other.highlight_pre_tag; self.highlight_post_tag |= other.highlight_post_tag; @@ -542,6 +543,7 @@ impl SearchAggregator { "pagination": { "max_limit": self.max_limit, "max_offset": self.max_offset, + "finite_pagination": self.finite_pagination > self.total_received / 2, }, "formatting": { "highlight_pre_tag": self.highlight_pre_tag, From b423ef72bec35fb2c5f28b60c6d45c479e8b60f4 Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Mon, 8 Aug 2022 15:30:28 +0200 Subject: [PATCH 10/18] PROTO: hardcode version and interval for prototype analytics --- meilisearch-http/src/analytics/segment_analytics.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/meilisearch-http/src/analytics/segment_analytics.rs b/meilisearch-http/src/analytics/segment_analytics.rs index 726d0945a..9f122f190 100644 --- a/meilisearch-http/src/analytics/segment_analytics.rs +++ b/meilisearch-http/src/analytics/segment_analytics.rs @@ -271,8 +271,8 @@ impl Segment { } async fn run(mut self, meilisearch: MeiliSearch) { - const INTERVAL: Duration = Duration::from_secs(60 * 60); // one hour - // The first batch must be sent after one hour. + const INTERVAL: Duration = Duration::from_secs(60); // one minute + // The first batch must be sent after one minute. let mut interval = tokio::time::interval_at(tokio::time::Instant::now() + INTERVAL, INTERVAL); @@ -302,7 +302,7 @@ impl Segment { .push(Identify { context: Some(json!({ "app": { - "version": env!("CARGO_PKG_VERSION").to_string(), + "version": "prototype-pagination-2".to_string(), }, })), user: self.user.clone(), From 506d08a9f450b0591b0d7d6aa3ddbb2d77e7e2cc Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Wed, 14 Sep 2022 14:09:49 +0200 Subject: [PATCH 11/18] Update analytics version --- meilisearch-http/src/analytics/segment_analytics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meilisearch-http/src/analytics/segment_analytics.rs b/meilisearch-http/src/analytics/segment_analytics.rs index 9f122f190..40679279a 100644 --- a/meilisearch-http/src/analytics/segment_analytics.rs +++ b/meilisearch-http/src/analytics/segment_analytics.rs @@ -302,7 +302,7 @@ impl Segment { .push(Identify { context: Some(json!({ "app": { - "version": "prototype-pagination-2".to_string(), + "version": "prototype-pagination-4".to_string(), }, })), user: self.user.clone(), From c02ae4dfc0d18dd7142544352712b23a4bddf77d Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Wed, 19 Oct 2022 14:25:43 +0200 Subject: [PATCH 12/18] Update roaring --- Cargo.lock | 15 ++------------- meilisearch-lib/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e5fd78adb..1f0264357 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2195,7 +2195,7 @@ dependencies = [ "rayon", "regex", "reqwest", - "roaring 0.9.0", + "roaring", "rustls", "serde", "serde_json", @@ -2278,7 +2278,7 @@ dependencies = [ "once_cell", "ordered-float", "rayon", - "roaring 0.10.1", + "roaring", "rstar", "serde", "serde_json", @@ -3055,17 +3055,6 @@ dependencies = [ "regex", ] -[[package]] -name = "roaring" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd539cab4e32019956fe7e0cf160bb6d4802f4be2b52c4253d76d3bb0f85a5f7" -dependencies = [ - "bytemuck", - "byteorder", - "retain_mut", -] - [[package]] name = "roaring" version = "0.10.1" diff --git a/meilisearch-lib/Cargo.toml b/meilisearch-lib/Cargo.toml index 099c5528c..18c0428b5 100644 --- a/meilisearch-lib/Cargo.toml +++ b/meilisearch-lib/Cargo.toml @@ -40,7 +40,7 @@ rand = "0.8.5" rayon = "1.5.3" regex = "1.6.0" reqwest = { version = "0.11.12", features = ["json", "rustls-tls"], default-features = false, optional = true } -roaring = "0.9.0" +roaring = "0.10.1" rustls = "0.20.6" serde = { version = "1.0.145", features = ["derive"] } serde_json = { version = "1.0.85", features = ["preserve_order"] } From 1d217cef19cc3a507ec3e71e0041f67aef274fbf Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Thu, 20 Oct 2022 17:03:07 +0200 Subject: [PATCH 13/18] Add some tests --- meilisearch-http/tests/search/mod.rs | 1 + meilisearch-http/tests/search/pagination.rs | 112 ++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 meilisearch-http/tests/search/pagination.rs diff --git a/meilisearch-http/tests/search/mod.rs b/meilisearch-http/tests/search/mod.rs index d5e916860..da31a3cdf 100644 --- a/meilisearch-http/tests/search/mod.rs +++ b/meilisearch-http/tests/search/mod.rs @@ -3,6 +3,7 @@ mod errors; mod formatted; +mod pagination; use crate::common::Server; use once_cell::sync::Lazy; diff --git a/meilisearch-http/tests/search/pagination.rs b/meilisearch-http/tests/search/pagination.rs new file mode 100644 index 000000000..41c4f31a4 --- /dev/null +++ b/meilisearch-http/tests/search/pagination.rs @@ -0,0 +1,112 @@ +use crate::common::Server; +use crate::search::DOCUMENTS; +use serde_json::json; + +#[actix_rt::test] +async fn default_search_should_return_estimated_total_hit() { + let server = Server::new().await; + let index = server.index("basic"); + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_task(0).await; + + index + .search(json!({}), |response, code| { + assert_eq!(code, 200, "{}", response); + assert!(response.get("estimatedTotalHits").is_some()); + assert!(response.get("limit").is_some()); + assert!(response.get("offset").is_some()); + + // these fields shouldn't be present + assert!(response.get("totalHits").is_none()); + assert!(response.get("page").is_none()); + assert!(response.get("totalPages").is_none()); + }) + .await; +} + +#[actix_rt::test] +async fn simple_search() { + let server = Server::new().await; + let index = server.index("basic"); + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_task(0).await; + + index + .search(json!({"page": 1}), |response, code| { + assert_eq!(code, 200, "{}", response); + assert_eq!(response["hits"].as_array().unwrap().len(), 5); + assert!(response.get("totalHits").is_some()); + assert_eq!(response["page"], 1); + assert_eq!(response["totalPages"], 1); + + // these fields shouldn't be present + assert!(response.get("estimatedTotalHits").is_none()); + assert!(response.get("limit").is_none()); + assert!(response.get("offset").is_none()); + }) + .await; +} + +#[actix_rt::test] +async fn page_zero_should_not_return_any_result() { + let server = Server::new().await; + let index = server.index("basic"); + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_task(0).await; + + index + .search(json!({"page": 0}), |response, code| { + assert_eq!(code, 200, "{}", response); + assert_eq!(response["hits"].as_array().unwrap().len(), 0); + assert!(response.get("totalHits").is_some()); + assert_eq!(response["page"], 0); + assert_eq!(response["totalPages"], 1); + }) + .await; +} + +#[actix_rt::test] +async fn hits_per_page_1() { + let server = Server::new().await; + let index = server.index("basic"); + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_task(0).await; + + index + .search(json!({"hitsPerPage": 1}), |response, code| { + assert_eq!(code, 200, "{}", response); + assert_eq!(response["hits"].as_array().unwrap().len(), 1); + assert_eq!(response["totalHits"], 5); + assert_eq!(response["page"], 1); + assert_eq!(response["totalPages"], 5); + }) + .await; +} + +#[actix_rt::test] +async fn hits_per_page_0_should_not_return_any_result() { + let server = Server::new().await; + let index = server.index("basic"); + + let documents = DOCUMENTS.clone(); + index.add_documents(documents, None).await; + index.wait_task(0).await; + + index + .search(json!({"hitsPerPage": 0}), |response, code| { + assert_eq!(code, 200, "{}", response); + assert_eq!(response["hits"].as_array().unwrap().len(), 0); + assert_eq!(response["totalHits"], 5); + assert_eq!(response["page"], 1); + assert_eq!(response["totalPages"], 0); + }) + .await; +} From 0578aff8c96efa20da16605336742895c2c76482 Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Thu, 20 Oct 2022 17:41:13 +0200 Subject: [PATCH 14/18] Fix the tests --- meilisearch-http/src/routes/indexes/search.rs | 17 +++--- meilisearch-lib/src/index/mod.rs | 6 +- meilisearch-lib/src/index/search.rs | 57 ++++++++----------- meilisearch-lib/src/index_controller/mod.rs | 8 +-- 4 files changed, 41 insertions(+), 47 deletions(-) diff --git a/meilisearch-http/src/routes/indexes/search.rs b/meilisearch-http/src/routes/indexes/search.rs index 828fb40ad..4b5e0dbca 100644 --- a/meilisearch-http/src/routes/indexes/search.rs +++ b/meilisearch-http/src/routes/indexes/search.rs @@ -2,8 +2,9 @@ use actix_web::{web, HttpRequest, HttpResponse}; use log::debug; use meilisearch_auth::IndexSearchRules; use meilisearch_lib::index::{ - SearchQuery, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, DEFAULT_HIGHLIGHT_POST_TAG, - DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_HIT_PER_PAGE, DEFAULT_PAGE, MatchingStrategy + MatchingStrategy, SearchQuery, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, + DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_SEARCH_LIMIT, + DEFAULT_SEARCH_OFFSET, }; use meilisearch_lib::MeiliSearch; use meilisearch_types::error::ResponseError; @@ -27,12 +28,12 @@ pub fn configure(cfg: &mut web::ServiceConfig) { #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct SearchQueryGet { q: Option, - offset: Option, - limit: Option, - #[serde(default = "DEFAULT_PAGE")] - page: usize, - #[serde(default = "DEFAULT_HIT_PER_PAGE")] - hits_per_page: usize, + #[serde(default = "DEFAULT_SEARCH_OFFSET")] + offset: usize, + #[serde(default = "DEFAULT_SEARCH_LIMIT")] + limit: usize, + page: Option, + hits_per_page: Option, attributes_to_retrieve: Option>, attributes_to_crop: Option>, #[serde(default = "DEFAULT_CROP_LENGTH")] diff --git a/meilisearch-lib/src/index/mod.rs b/meilisearch-lib/src/index/mod.rs index 08f1f42d5..0aeaba14e 100644 --- a/meilisearch-lib/src/index/mod.rs +++ b/meilisearch-lib/src/index/mod.rs @@ -1,7 +1,7 @@ pub use search::{ - HitsInfo, SearchQuery, SearchResult, DEFAULT_CROP_LENGTH, DEFAULT_CROP_MARKER, - DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, DEFAULT_HIT_PER_PAGE, DEFAULT_PAGE, - DEFAULT_SEARCH_LIMIT, MatchingStrategy + HitsInfo, MatchingStrategy, SearchQuery, SearchResult, DEFAULT_CROP_LENGTH, + DEFAULT_CROP_MARKER, DEFAULT_HIGHLIGHT_POST_TAG, DEFAULT_HIGHLIGHT_PRE_TAG, + DEFAULT_SEARCH_LIMIT, DEFAULT_SEARCH_OFFSET, }; pub use updates::{apply_settings_to_builder, Checked, Facets, Settings, Unchecked}; diff --git a/meilisearch-lib/src/index/search.rs b/meilisearch-lib/src/index/search.rs index 9badd9cb6..ea2fb65d7 100644 --- a/meilisearch-lib/src/index/search.rs +++ b/meilisearch-lib/src/index/search.rs @@ -21,13 +21,12 @@ use super::index::Index; pub type Document = serde_json::Map; type MatchesPosition = BTreeMap>; +pub const DEFAULT_SEARCH_OFFSET: fn() -> usize = || 0; pub const DEFAULT_SEARCH_LIMIT: fn() -> usize = || 20; pub const DEFAULT_CROP_LENGTH: fn() -> usize = || 10; pub const DEFAULT_CROP_MARKER: fn() -> String = || "…".to_string(); pub const DEFAULT_HIGHLIGHT_PRE_TAG: fn() -> String = || "".to_string(); pub const DEFAULT_HIGHLIGHT_POST_TAG: fn() -> String = || "".to_string(); -pub const DEFAULT_PAGE: fn() -> usize = || 1; -pub const DEFAULT_HIT_PER_PAGE: fn() -> usize = || 20; /// The maximum number of results that the engine /// will be able to return in one search call. @@ -37,12 +36,12 @@ pub const DEFAULT_PAGINATION_MAX_TOTAL_HITS: usize = 1000; #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct SearchQuery { pub q: Option, - pub offset: Option, - pub limit: Option, - #[serde(default = "DEFAULT_PAGE")] - pub page: usize, - #[serde(default = "DEFAULT_HIT_PER_PAGE")] - pub hits_per_page: usize, + #[serde(default = "DEFAULT_SEARCH_OFFSET")] + pub offset: usize, + #[serde(default = "DEFAULT_SEARCH_LIMIT")] + pub limit: usize, + pub page: Option, + pub hits_per_page: Option, pub attributes_to_retrieve: Option>, pub attributes_to_crop: Option>, #[serde(default = "DEFAULT_CROP_LENGTH")] @@ -145,33 +144,26 @@ impl Index { .pagination_max_total_hits(&rtxn)? .unwrap_or(DEFAULT_PAGINATION_MAX_TOTAL_HITS); - // Make sure that a user can't get more documents than the hard limit, - // we align that on the offset too. - let is_finite_pagination = query.offset.is_none() && query.limit.is_none(); + let is_finite_pagination = query.page.or(query.hits_per_page).is_some(); search.exhaustive_number_hits(is_finite_pagination); + // compute the offset on the limit depending on the pagination mode. let (offset, limit) = if is_finite_pagination { - match query.page.checked_sub(1) { - Some(page) => { - let offset = min(query.hits_per_page * page, max_total_hits); - let limit = min(query.hits_per_page, max_total_hits.saturating_sub(offset)); + let limit = query.hits_per_page.unwrap_or_else(DEFAULT_SEARCH_LIMIT); + let page = query.page.unwrap_or(1); - (offset, limit) - } - // page 0 returns 0 hits - None => (0, 0), - } + // page 0 gives a limit of 0 forcing Meilisearch to return no document. + page.checked_sub(1).map_or((0, 0), |p| (limit * p, limit)) } else { - let offset = min(query.offset.unwrap_or(0), max_total_hits); - let limit = min( - query.limit.unwrap_or_else(DEFAULT_SEARCH_LIMIT), - max_total_hits.saturating_sub(offset), - ); - - (offset, limit) + (query.offset, query.limit) }; + // Make sure that a user can't get more documents than the hard limit, + // we align that on the offset too. + let offset = min(offset, max_total_hits); + let limit = min(limit, max_total_hits.saturating_sub(offset)); + search.offset(offset); search.limit(limit); @@ -297,20 +289,21 @@ impl Index { let number_of_hits = min(candidates.len() as usize, max_total_hits); let hits_info = if is_finite_pagination { + let hits_per_page = query.hits_per_page.unwrap_or_else(DEFAULT_SEARCH_LIMIT); // If hit_per_page is 0, then pages can't be computed and so we respond 0. - let total_pages = (number_of_hits + query.hits_per_page.saturating_sub(1)) - .checked_div(query.hits_per_page) + let total_pages = (number_of_hits + hits_per_page.saturating_sub(1)) + .checked_div(hits_per_page) .unwrap_or(0); HitsInfo::Pagination { - hits_per_page: query.hits_per_page, - page: query.page, + hits_per_page: hits_per_page, + page: query.page.unwrap_or(1), total_pages, total_hits: number_of_hits, } } else { HitsInfo::OffsetLimit { - limit: query.limit.unwrap_or_else(DEFAULT_SEARCH_LIMIT), + limit: query.limit, offset, estimated_total_hits: number_of_hits, } diff --git a/meilisearch-lib/src/index_controller/mod.rs b/meilisearch-lib/src/index_controller/mod.rs index b5a1daef6..87644a44a 100644 --- a/meilisearch-lib/src/index_controller/mod.rs +++ b/meilisearch-lib/src/index_controller/mod.rs @@ -691,10 +691,10 @@ mod test { let index_uuid = Uuid::new_v4(); let query = SearchQuery { q: Some(String::from("hello world")), - offset: Some(10), - limit: Some(0), - page: 1, - hits_per_page: 10, + offset: 10, + limit: 0, + page: Some(1), + hits_per_page: Some(10), attributes_to_retrieve: Some(vec!["string".to_owned()].into_iter().collect()), attributes_to_crop: None, crop_length: 18, From a2314cf436d172c870f5e6ad2ddb13db27987b35 Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Mon, 24 Oct 2022 13:56:26 +0200 Subject: [PATCH 15/18] Update analytics --- .../src/analytics/segment_analytics.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/meilisearch-http/src/analytics/segment_analytics.rs b/meilisearch-http/src/analytics/segment_analytics.rs index 40679279a..6655a7f9b 100644 --- a/meilisearch-http/src/analytics/segment_analytics.rs +++ b/meilisearch-http/src/analytics/segment_analytics.rs @@ -271,8 +271,8 @@ impl Segment { } async fn run(mut self, meilisearch: MeiliSearch) { - const INTERVAL: Duration = Duration::from_secs(60); // one minute - // The first batch must be sent after one minute. + const INTERVAL: Duration = Duration::from_secs(60 * 60); // one hour + // The first batch must be sent after one hour. let mut interval = tokio::time::interval_at(tokio::time::Instant::now() + INTERVAL, INTERVAL); @@ -302,7 +302,7 @@ impl Segment { .push(Identify { context: Some(json!({ "app": { - "version": "prototype-pagination-4".to_string(), + "version": env!("CARGO_PKG_VERSION").to_string(), }, })), user: self.user.clone(), @@ -428,13 +428,14 @@ impl SearchAggregator { ret.max_terms_number = q.split_whitespace().count(); } - if query.limit.is_none() && query.offset.is_none() { - ret.max_limit = query.hits_per_page; - ret.max_offset = query.page.saturating_sub(1) * query.hits_per_page; + if query.hits_per_page.or(query.page).is_some() { + let limit = query.hits_per_page.unwrap_or_else(DEFAULT_SEARCH_LIMIT); + ret.max_limit = limit; + ret.max_offset = query.page.unwrap_or(1).saturating_sub(1) * limit; ret.finite_pagination = 1; } else { - ret.max_limit = query.limit.unwrap_or_else(DEFAULT_SEARCH_LIMIT); - ret.max_offset = query.offset.unwrap_or_default(); + ret.max_limit = query.limit; + ret.max_offset = query.offset; ret.finite_pagination = 0; } From 4afed4de4f8772c80aeafec9eecfd6ddddb3dd2f Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Mon, 24 Oct 2022 14:11:40 +0200 Subject: [PATCH 16/18] stabilize milli --- Cargo.lock | 16 ++++++++-------- meilisearch-auth/Cargo.toml | 2 +- meilisearch-lib/Cargo.toml | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1f0264357..69b0af37d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1183,8 +1183,8 @@ dependencies = [ [[package]] name = "filter-parser" -version = "0.33.4" -source = "git+https://github.com/meilisearch/milli.git?branch=main#19b2326f3dd3b85daa00cf1c752797f4b11c536b" +version = "0.34.0" +source = "git+https://github.com/meilisearch/milli.git?tag=v0.34.0#2bf867982ab548a6d749c7534f69b44d3552ef70" dependencies = [ "nom", "nom_locate", @@ -1202,8 +1202,8 @@ dependencies = [ [[package]] name = "flatten-serde-json" -version = "0.33.4" -source = "git+https://github.com/meilisearch/milli.git?branch=main#19b2326f3dd3b85daa00cf1c752797f4b11c536b" +version = "0.34.0" +source = "git+https://github.com/meilisearch/milli.git?tag=v0.34.0#2bf867982ab548a6d749c7534f69b44d3552ef70" dependencies = [ "serde_json", ] @@ -1713,8 +1713,8 @@ dependencies = [ [[package]] name = "json-depth-checker" -version = "0.33.4" -source = "git+https://github.com/meilisearch/milli.git?branch=main#19b2326f3dd3b85daa00cf1c752797f4b11c536b" +version = "0.34.0" +source = "git+https://github.com/meilisearch/milli.git?tag=v0.34.0#2bf867982ab548a6d749c7534f69b44d3552ef70" dependencies = [ "serde_json", ] @@ -2249,8 +2249,8 @@ dependencies = [ [[package]] name = "milli" -version = "0.33.4" -source = "git+https://github.com/meilisearch/milli.git?branch=main#19b2326f3dd3b85daa00cf1c752797f4b11c536b" +version = "0.34.0" +source = "git+https://github.com/meilisearch/milli.git?tag=v0.34.0#2bf867982ab548a6d749c7534f69b44d3552ef70" dependencies = [ "bimap", "bincode", diff --git a/meilisearch-auth/Cargo.toml b/meilisearch-auth/Cargo.toml index 828203e7e..a872b4e9a 100644 --- a/meilisearch-auth/Cargo.toml +++ b/meilisearch-auth/Cargo.toml @@ -7,7 +7,7 @@ edition = "2021" enum-iterator = "1.1.2" hmac = "0.12.1" meilisearch-types = { path = "../meilisearch-types" } -milli = { git = "https://github.com/meilisearch/milli.git", branch = "main", default-features = false } +milli = { git = "https://github.com/meilisearch/milli.git", tag = "v0.34.0", default-features = false } rand = "0.8.5" serde = { version = "1.0.145", features = ["derive"] } serde_json = { version = "1.0.85", features = ["preserve_order"] } diff --git a/meilisearch-lib/Cargo.toml b/meilisearch-lib/Cargo.toml index 18c0428b5..dbaf8faa2 100644 --- a/meilisearch-lib/Cargo.toml +++ b/meilisearch-lib/Cargo.toml @@ -28,7 +28,7 @@ lazy_static = "1.4.0" log = "0.4.17" meilisearch-auth = { path = "../meilisearch-auth" } meilisearch-types = { path = "../meilisearch-types" } -milli = { git = "https://github.com/meilisearch/milli.git", branch = "main", default-features = false } +milli = { git = "https://github.com/meilisearch/milli.git", tag = "v0.34.0", default-features = false } mime = "0.3.16" num_cpus = "1.13.1" obkv = "0.2.0" From 68c9751d49d9780497ee78cee2ee4ef3613eaf0b Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Mon, 24 Oct 2022 13:57:26 +0200 Subject: [PATCH 17/18] Fix clippy --- meilisearch-lib/src/dump/compat/v4.rs | 2 +- meilisearch-lib/src/index/search.rs | 4 ++-- meilisearch-lib/src/index/updates.rs | 10 +++++----- meilisearch-lib/src/tasks/task.rs | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/meilisearch-lib/src/dump/compat/v4.rs b/meilisearch-lib/src/dump/compat/v4.rs index c412e7f17..89e9ee1ab 100644 --- a/meilisearch-lib/src/dump/compat/v4.rs +++ b/meilisearch-lib/src/dump/compat/v4.rs @@ -70,7 +70,7 @@ impl From for NewTaskEvent { } } -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] #[allow(clippy::large_enum_variant)] pub enum TaskContent { DocumentAddition { diff --git a/meilisearch-lib/src/index/search.rs b/meilisearch-lib/src/index/search.rs index ea2fb65d7..a260e52c3 100644 --- a/meilisearch-lib/src/index/search.rs +++ b/meilisearch-lib/src/index/search.rs @@ -109,7 +109,7 @@ pub struct SearchResult { pub facet_distribution: Option>>, } -#[derive(Serialize, Debug, Clone, PartialEq)] +#[derive(Serialize, Debug, Clone, PartialEq, Eq)] #[serde(untagged)] pub enum HitsInfo { #[serde(rename_all = "camelCase")] @@ -296,7 +296,7 @@ impl Index { .unwrap_or(0); HitsInfo::Pagination { - hits_per_page: hits_per_page, + hits_per_page, page: query.page.unwrap_or(1), total_pages, total_hits: number_of_hits, diff --git a/meilisearch-lib/src/index/updates.rs b/meilisearch-lib/src/index/updates.rs index b6f601753..7058d65c3 100644 --- a/meilisearch-lib/src/index/updates.rs +++ b/meilisearch-lib/src/index/updates.rs @@ -38,7 +38,7 @@ pub struct Checked; pub struct Unchecked; #[cfg_attr(test, derive(proptest_derive::Arbitrary))] -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] #[serde(rename_all = "camelCase")] pub struct MinWordSizeTyposSetting { @@ -51,7 +51,7 @@ pub struct MinWordSizeTyposSetting { } #[cfg_attr(test, derive(proptest_derive::Arbitrary))] -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] #[serde(rename_all = "camelCase")] pub struct TypoSettings { @@ -70,7 +70,7 @@ pub struct TypoSettings { } #[cfg_attr(test, derive(proptest_derive::Arbitrary))] -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] #[serde(rename_all = "camelCase")] pub struct FacetingSettings { @@ -80,7 +80,7 @@ pub struct FacetingSettings { } #[cfg_attr(test, derive(proptest_derive::Arbitrary))] -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] #[serde(rename_all = "camelCase")] pub struct PaginationSettings { @@ -92,7 +92,7 @@ pub struct PaginationSettings { /// Holds all the settings for an index. `T` can either be `Checked` if they represents settings /// whose validity is guaranteed, or `Unchecked` if they need to be validated. In the later case, a /// call to `check` will return a `Settings` from a `Settings`. -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] #[serde(rename_all = "camelCase")] #[serde(bound(serialize = "T: Serialize", deserialize = "T: Deserialize<'static>"))] diff --git a/meilisearch-lib/src/tasks/task.rs b/meilisearch-lib/src/tasks/task.rs index 7f9b72964..e0a18895b 100644 --- a/meilisearch-lib/src/tasks/task.rs +++ b/meilisearch-lib/src/tasks/task.rs @@ -80,7 +80,7 @@ impl TaskEvent { /// It's stored on disk and executed from the lowest to highest Task id. /// Every time a new task is created it has a higher Task id than the previous one. /// See also `Job`. -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] #[cfg_attr(test, derive(proptest_derive::Arbitrary))] pub struct Task { pub id: TaskId, @@ -135,7 +135,7 @@ pub enum DocumentDeletion { Ids(Vec), } -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] #[cfg_attr(test, derive(proptest_derive::Arbitrary))] #[allow(clippy::large_enum_variant)] pub enum TaskContent { From f4021273b8d098f83c4b2c99100ed1228c32d24b Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Wed, 26 Oct 2022 18:08:29 +0200 Subject: [PATCH 18/18] Add is_finite_pagination method to SearchQuery --- meilisearch-http/src/analytics/segment_analytics.rs | 2 +- meilisearch-lib/src/index/search.rs | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/meilisearch-http/src/analytics/segment_analytics.rs b/meilisearch-http/src/analytics/segment_analytics.rs index 6655a7f9b..21d41d84f 100644 --- a/meilisearch-http/src/analytics/segment_analytics.rs +++ b/meilisearch-http/src/analytics/segment_analytics.rs @@ -428,7 +428,7 @@ impl SearchAggregator { ret.max_terms_number = q.split_whitespace().count(); } - if query.hits_per_page.or(query.page).is_some() { + if query.is_finite_pagination() { let limit = query.hits_per_page.unwrap_or_else(DEFAULT_SEARCH_LIMIT); ret.max_limit = limit; ret.max_offset = query.page.unwrap_or(1).saturating_sub(1) * limit; diff --git a/meilisearch-lib/src/index/search.rs b/meilisearch-lib/src/index/search.rs index a260e52c3..558a530c0 100644 --- a/meilisearch-lib/src/index/search.rs +++ b/meilisearch-lib/src/index/search.rs @@ -63,6 +63,12 @@ pub struct SearchQuery { pub matching_strategy: MatchingStrategy, } +impl SearchQuery { + pub fn is_finite_pagination(&self) -> bool { + self.page.or(self.hits_per_page).is_some() + } +} + #[derive(Deserialize, Debug, Clone, PartialEq, Eq)] #[serde(rename_all = "camelCase")] pub enum MatchingStrategy { @@ -138,14 +144,13 @@ impl Index { search.query(query); } + let is_finite_pagination = query.is_finite_pagination(); search.terms_matching_strategy(query.matching_strategy.into()); let max_total_hits = self .pagination_max_total_hits(&rtxn)? .unwrap_or(DEFAULT_PAGINATION_MAX_TOTAL_HITS); - let is_finite_pagination = query.page.or(query.hits_per_page).is_some(); - search.exhaustive_number_hits(is_finite_pagination); // compute the offset on the limit depending on the pagination mode.