From 82df524e091aa016626a63c0fc73e532e09b3967 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Tue, 8 Jun 2021 17:03:27 +0200 Subject: [PATCH 1/2] Make sure that we register the field when setting criteria --- milli/src/criterion.rs | 10 ++++++++++ milli/src/update/settings.rs | 12 +++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/milli/src/criterion.rs b/milli/src/criterion.rs index 81a2878b3..c2205613d 100644 --- a/milli/src/criterion.rs +++ b/milli/src/criterion.rs @@ -30,6 +30,16 @@ pub enum Criterion { Desc(String), } +impl Criterion { + /// Returns the field name parameter of this criterion. + pub fn field_name(&self) -> Option<&str> { + match self { + Criterion::Asc(name) | Criterion::Desc(name) => Some(name), + _otherwise => None, + } + } +} + impl FromStr for Criterion { type Err = anyhow::Error; diff --git a/milli/src/update/settings.rs b/milli/src/update/settings.rs index ef32c5c44..ec4618158 100644 --- a/milli/src/update/settings.rs +++ b/milli/src/update/settings.rs @@ -8,9 +8,10 @@ use meilisearch_tokenizer::{Analyzer, AnalyzerConfig}; use rayon::ThreadPool; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use crate::{FieldsIdsMap, Index}; -use crate::update::{ClearDocuments, IndexDocuments, UpdateIndexingStep}; +use crate::criterion::Criterion; use crate::update::index_documents::{IndexDocumentsMethod, Transform}; +use crate::update::{ClearDocuments, IndexDocuments, UpdateIndexingStep}; +use crate::{FieldsIdsMap, Index}; #[derive(Debug, Clone, PartialEq)] pub enum Setting { @@ -403,12 +404,17 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> { fn update_criteria(&mut self) -> anyhow::Result<()> { match self.criteria { Setting::Set(ref fields) => { + let mut fields_ids_map = self.index.fields_ids_map(self.wtxn)?; let mut new_criteria = Vec::new(); for name in fields { - let criterion = name.parse()?; + let criterion: Criterion = name.parse()?; + if let Some(name) = criterion.field_name() { + fields_ids_map.insert(name).context("field id limit exceeded")?; + } new_criteria.push(criterion); } self.index.put_criteria(self.wtxn, &new_criteria)?; + self.index.put_fields_ids_map(self.wtxn, &fields_ids_map)?; } Setting::Reset => { self.index.delete_criteria(self.wtxn)?; } Setting::NotSet => (), From 0bf4f3f48a917b1225f9c5ea574b8a0cadf7db7e Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Tue, 8 Jun 2021 17:55:08 +0200 Subject: [PATCH 2/2] Modify a test to check that criteria additions change the fields ids map --- milli/src/update/settings.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/milli/src/update/settings.rs b/milli/src/update/settings.rs index ec4618158..1c687e089 100644 --- a/milli/src/update/settings.rs +++ b/milli/src/update/settings.rs @@ -697,7 +697,7 @@ mod tests { let mut wtxn = index.write_txn().unwrap(); let mut builder = Settings::new(&mut wtxn, &index, 0); // Don't display the generated `id` field. - builder.set_displayed_fields(vec![S("name"), S("age")]); + builder.set_displayed_fields(vec![S("name")]); builder.set_criteria(vec![S("asc(age)")]); builder.execute(|_, _| ()).unwrap();