From 6167a10e5e2792731cdc249fe24d8442fac9a6db Mon Sep 17 00:00:00 2001 From: mpostma Date: Mon, 15 Jun 2020 16:34:06 +0200 Subject: [PATCH 1/4] change ranking rule addition behavior --- meilisearch-core/src/update/settings_update.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/meilisearch-core/src/update/settings_update.rs b/meilisearch-core/src/update/settings_update.rs index 2717d7aa5..fbf266001 100644 --- a/meilisearch-core/src/update/settings_update.rs +++ b/meilisearch-core/src/update/settings_update.rs @@ -46,12 +46,6 @@ pub fn apply_settings_update( UpdateState::Update(v) => { let ranked_field: Vec<&str> = v.iter().filter_map(RankingRule::field).collect(); schema.update_ranked(&ranked_field)?; - for name in ranked_field { - if schema.accept_new_fields() { - schema.set_indexed(name.as_ref())?; - schema.set_displayed(name.as_ref())?; - } - } index.main.put_ranking_rules(writer, &v)?; must_reindex = true; }, From 60a90e96f323370ca818c6f824668509e843635e Mon Sep 17 00:00:00 2001 From: mpostma Date: Tue, 16 Jun 2020 09:52:58 +0200 Subject: [PATCH 2/4] add test for ranking rules settings --- meilisearch-http/tests/settings.rs | 14 ++++++++++++++ meilisearch-http/tests/settings_ranking_rules.rs | 1 - 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/meilisearch-http/tests/settings.rs b/meilisearch-http/tests/settings.rs index dcbe5760f..5716860b9 100644 --- a/meilisearch-http/tests/settings.rs +++ b/meilisearch-http/tests/settings.rs @@ -455,3 +455,17 @@ async fn attributes_for_faceting_settings() { let (response, _status_code) = server.get_request("/indexes/test/settings/attributes-for-faceting").await; assert_eq!(response, json!([])); } + +#[actix_rt::test] +async fn setting_ranking_rules_dont_mess_with_other_settings() { + let mut server = common::Server::test_server().await; + let body = json!({ + "rankingRules": ["asc(foobar)"] + }); + server.update_all_settings(body).await; + let (response, _) = server.get_all_settings().await; + assert_eq!(response["rankingRules"].as_array().unwrap().len(), 1); + assert_eq!(response["rankingRules"].as_array().unwrap().first().unwrap().as_str().unwrap(), "asc(foobar)"); + assert!(!response["searchableAttributes"].as_array().unwrap().iter().any(|e| e.as_str().unwrap() == "foobar")); + assert!(!response["displayedAttributes"].as_array().unwrap().iter().any(|e| e.as_str().unwrap() == "foobar")); +} diff --git a/meilisearch-http/tests/settings_ranking_rules.rs b/meilisearch-http/tests/settings_ranking_rules.rs index 7dbe9c8ea..67ddcc9ac 100644 --- a/meilisearch-http/tests/settings_ranking_rules.rs +++ b/meilisearch-http/tests/settings_ranking_rules.rs @@ -173,7 +173,6 @@ async fn write_custom_ranking_and_index_documents() { let expected = json!({ "id": 1, - "title": "Le Petit Prince", "author": "Exupéry" }); From 8035ca71387989e10784a25dc42046130d4c5a63 Mon Sep 17 00:00:00 2001 From: mpostma Date: Tue, 16 Jun 2020 10:45:17 +0200 Subject: [PATCH 3/4] fix distinct attribute behavior --- meilisearch-core/src/store/main.rs | 10 ++++----- .../src/update/settings_update.rs | 3 ++- meilisearch-http/src/helpers/meilisearch.rs | 22 +++++++++---------- meilisearch-http/src/routes/setting.rs | 6 ++++- 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/meilisearch-core/src/store/main.rs b/meilisearch-core/src/store/main.rs index f1ef6fc5e..9d5e95425 100644 --- a/meilisearch-core/src/store/main.rs +++ b/meilisearch-core/src/store/main.rs @@ -286,15 +286,15 @@ impl Main { Ok(self.main.delete::<_, Str>(writer, RANKING_RULES_KEY)?) } - pub fn distinct_attribute(&self, reader: &heed::RoTxn) -> MResult> { - if let Some(value) = self.main.get::<_, Str, Str>(reader, DISTINCT_ATTRIBUTE_KEY)? { - return Ok(Some(value.to_owned())) + pub fn distinct_attribute(&self, reader: &heed::RoTxn) -> MResult> { + if let Some(value) = self.main.get::<_, Str, OwnedType>(reader, DISTINCT_ATTRIBUTE_KEY)? { + return Ok(Some(FieldId(value.to_owned()))) } return Ok(None) } - pub fn put_distinct_attribute(self, writer: &mut heed::RwTxn, value: &str) -> MResult<()> { - Ok(self.main.put::<_, Str, Str>(writer, DISTINCT_ATTRIBUTE_KEY, value)?) + pub fn put_distinct_attribute(self, writer: &mut heed::RwTxn, value: FieldId) -> MResult<()> { + Ok(self.main.put::<_, Str, OwnedType>(writer, DISTINCT_ATTRIBUTE_KEY, &value.0)?) } pub fn delete_distinct_attribute(self, writer: &mut heed::RwTxn) -> MResult { diff --git a/meilisearch-core/src/update/settings_update.rs b/meilisearch-core/src/update/settings_update.rs index fbf266001..9aecb23f7 100644 --- a/meilisearch-core/src/update/settings_update.rs +++ b/meilisearch-core/src/update/settings_update.rs @@ -59,7 +59,8 @@ pub fn apply_settings_update( match settings.distinct_attribute { UpdateState::Update(v) => { - index.main.put_distinct_attribute(writer, &v)?; + let field_id = schema.insert(&v)?; + index.main.put_distinct_attribute(writer, field_id)?; }, UpdateState::Clear => { index.main.delete_distinct_attribute(writer)?; diff --git a/meilisearch-http/src/helpers/meilisearch.rs b/meilisearch-http/src/helpers/meilisearch.rs index 65b6da08f..f9183d013 100644 --- a/meilisearch-http/src/helpers/meilisearch.rs +++ b/meilisearch-http/src/helpers/meilisearch.rs @@ -139,19 +139,17 @@ impl<'a> SearchBuilder<'a> { } if let Some(field) = self.index.main.distinct_attribute(reader)? { - if let Some(field_id) = schema.id(&field) { - let index = &self.index; - query_builder.with_distinct(1, move |id| { - match index.document_attribute_bytes(reader, id, field_id) { - Ok(Some(bytes)) => { - let mut s = SipHasher::new(); - bytes.hash(&mut s); - Some(s.finish()) - } - _ => None, + let index = &self.index; + query_builder.with_distinct(1, move |id| { + match index.document_attribute_bytes(reader, id, field) { + Ok(Some(bytes)) => { + let mut s = SipHasher::new(); + bytes.hash(&mut s); + Some(s.finish()) } - }); - } + _ => None, + } + }); } query_builder.set_facet_filter(self.facet_filters); diff --git a/meilisearch-http/src/routes/setting.rs b/meilisearch-http/src/routes/setting.rs index 37fe5bda2..ea35470ae 100644 --- a/meilisearch-http/src/routes/setting.rs +++ b/meilisearch-http/src/routes/setting.rs @@ -89,10 +89,14 @@ async fn get_all( .map(|r| r.to_string()) .collect(); - let distinct_attribute = index.main.distinct_attribute(&reader)?; let schema = index.main.schema(&reader)?; + let distinct_attribute = match (index.main.distinct_attribute(&reader)?, &schema) { + (Some(id), Some(schema)) => schema.name(id).map(str::to_string), + _ => None, + }; + let attributes_for_faceting = match (&schema, &index.main.attributes_for_faceting(&reader)?) { (Some(schema), Some(attrs)) => { attrs From 3d771f22897320d3f0025ed93ca90d6bae5a38b0 Mon Sep 17 00:00:00 2001 From: mpostma Date: Tue, 16 Jun 2020 11:55:58 +0200 Subject: [PATCH 4/4] test distinct attribute --- meilisearch-http/tests/settings.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/meilisearch-http/tests/settings.rs b/meilisearch-http/tests/settings.rs index 5716860b9..499c2dba9 100644 --- a/meilisearch-http/tests/settings.rs +++ b/meilisearch-http/tests/settings.rs @@ -469,3 +469,19 @@ async fn setting_ranking_rules_dont_mess_with_other_settings() { assert!(!response["searchableAttributes"].as_array().unwrap().iter().any(|e| e.as_str().unwrap() == "foobar")); assert!(!response["displayedAttributes"].as_array().unwrap().iter().any(|e| e.as_str().unwrap() == "foobar")); } + +#[actix_rt::test] +async fn distinct_attribute_recorded_as_known_field() { + let mut server = common::Server::test_server().await; + let body = json!({ + "distinctAttribute": "foobar", + "acceptNewFields": true + }); + server.update_all_settings(body).await; + let document = json!([{"id": 9348127, "foobar": "hello", "foo": "bar"}]); + server.add_or_update_multiple_documents(document).await; + // foobar should not be added to the searchable attributes because it is already known, but "foo" should + let (response, _) = server.get_all_settings().await; + assert!(response["searchableAttributes"].as_array().unwrap().iter().any(|v| v.as_str().unwrap() == "foo")); + assert!(!response["searchableAttributes"].as_array().unwrap().iter().any(|v| v.as_str().unwrap() == "foobar")); +}