From 2a3f9b32ff2d928d77ea4281e800023e976ae681 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Tue, 1 Jun 2021 12:19:55 +0200 Subject: [PATCH] Rename the faceted fields into filterable fields --- milli/src/index.rs | 38 ++++++++++---- milli/src/search/distinct/mod.rs | 2 +- milli/src/search/facet/facet_condition.rs | 52 ++++++++++---------- milli/src/search/facet/facet_distribution.rs | 4 +- milli/src/search/mod.rs | 4 +- milli/src/update/facets.rs | 16 +++--- milli/src/update/index_documents/mod.rs | 4 +- milli/src/update/settings.rs | 34 ++++++------- 8 files changed, 87 insertions(+), 67 deletions(-) diff --git a/milli/src/index.rs b/milli/src/index.rs index bd057a02a..9a52188dc 100644 --- a/milli/src/index.rs +++ b/milli/src/index.rs @@ -25,7 +25,7 @@ pub const CRITERIA_KEY: &str = "criteria"; pub const DISPLAYED_FIELDS_KEY: &str = "displayed-fields"; pub const DISTINCT_ATTRIBUTE_KEY: &str = "distinct-attribute-key"; pub const DOCUMENTS_IDS_KEY: &str = "documents-ids"; -pub const FACETED_FIELDS_KEY: &str = "faceted-fields"; +pub const FILTERABLE_FIELDS_KEY: &str = "filterable-fields"; pub const FIELDS_DISTRIBUTION_KEY: &str = "fields-distribution"; pub const FIELDS_IDS_MAP_KEY: &str = "fields-ids-map"; pub const HARD_EXTERNAL_DOCUMENTS_IDS_KEY: &str = "hard-external-documents-ids"; @@ -324,19 +324,39 @@ impl Index { } } - /* faceted fields */ + /* filterable fields */ - /// Writes the facet fields names in the database. - pub fn put_faceted_fields(&self, wtxn: &mut RwTxn, fields: &HashSet) -> heed::Result<()> { - self.main.put::<_, Str, SerdeJson<_>>(wtxn, FACETED_FIELDS_KEY, fields) + /// Writes the filterable fields names in the database. + pub fn put_filterable_fields(&self, wtxn: &mut RwTxn, fields: &HashSet) -> heed::Result<()> { + self.main.put::<_, Str, SerdeJson<_>>(wtxn, FILTERABLE_FIELDS_KEY, fields) } - /// Deletes the facet fields ids in the database. - pub fn delete_faceted_fields(&self, wtxn: &mut RwTxn) -> heed::Result { - self.main.delete::<_, Str>(wtxn, FACETED_FIELDS_KEY) + /// Deletes the filterable fields ids in the database. + pub fn delete_filterable_fields(&self, wtxn: &mut RwTxn) -> heed::Result { + self.main.delete::<_, Str>(wtxn, FILTERABLE_FIELDS_KEY) } - /// Returns the facet fields names. + /// Returns the filterable fields names. + pub fn filterable_fields(&self, rtxn: &RoTxn) -> heed::Result> { + Ok(self.main.get::<_, Str, SerdeJson<_>>(rtxn, FILTERABLE_FIELDS_KEY)?.unwrap_or_default()) + } + + /// Same as `filterable_fields`, but returns ids instead. + pub fn filterable_fields_ids(&self, rtxn: &RoTxn) -> heed::Result> { + let filterable_fields = self.filterable_fields(rtxn)?; + let fields_ids_map = self.fields_ids_map(rtxn)?; + let filterable_fields = filterable_fields + .iter() + .map(|k| { + fields_ids_map + .id(k) + .ok_or_else(|| format!("{:?} should be present in the field id map", k)) + .expect("corrupted data: ") + }) + .collect(); + + Ok(filterable_fields) + } pub fn faceted_fields(&self, rtxn: &RoTxn) -> heed::Result> { Ok(self.main.get::<_, Str, SerdeJson<_>>(rtxn, FACETED_FIELDS_KEY)?.unwrap_or_default()) } diff --git a/milli/src/search/distinct/mod.rs b/milli/src/search/distinct/mod.rs index 0dd628d5b..eed475863 100644 --- a/milli/src/search/distinct/mod.rs +++ b/milli/src/search/distinct/mod.rs @@ -83,7 +83,7 @@ mod test { let mut update = builder.settings(&mut txn, &index); update.set_distinct_attribute(distinct.to_string()); if !facets.is_empty() { - update.set_faceted_fields(facets) + update.set_filterable_fields(facets) } update.execute(|_, _| ()).unwrap(); diff --git a/milli/src/search/facet/facet_condition.rs b/milli/src/search/facet/facet_condition.rs index fd7053269..0c1f9e046 100644 --- a/milli/src/search/facet/facet_condition.rs +++ b/milli/src/search/facet/facet_condition.rs @@ -57,7 +57,7 @@ pub enum FacetCondition { fn field_id( fields_ids_map: &FieldsIdsMap, - faceted_fields: &HashSet, + filterable_fields: &HashSet, items: &mut Pairs, ) -> Result> { @@ -78,13 +78,13 @@ fn field_id( )), }; - if !faceted_fields.contains(&field_id) { + if !filterable_fields.contains(&field_id) { return Err(PestError::new_from_span( ErrorVariant::CustomError { message: format!( - "attribute `{}` is not faceted, available faceted attributes are: {}", + "attribute `{}` is not filterable, available filterable attributes are: {}", key.as_str(), - faceted_fields.iter().flat_map(|id| { + filterable_fields.iter().flat_map(|id| { fields_ids_map.name(*id) }).collect::>().join(", "), ), @@ -163,9 +163,9 @@ impl FacetCondition { ) -> anyhow::Result { let fields_ids_map = index.fields_ids_map(rtxn)?; - let faceted_fields = index.faceted_fields_ids(rtxn)?; + let filterable_fields = index.filterable_fields_ids(rtxn)?; let lexed = FilterParser::parse(Rule::prgm, expression)?; - FacetCondition::from_pairs(&fields_ids_map, &faceted_fields, lexed) + FacetCondition::from_pairs(&fields_ids_map, &filterable_fields, lexed) } fn from_pairs( @@ -212,12 +212,12 @@ impl FacetCondition { fn between( fields_ids_map: &FieldsIdsMap, - faceted_fields: &HashSet, + filterable_fields: &HashSet, item: Pair, ) -> anyhow::Result { let mut items = item.into_inner(); - let fid = field_id(fields_ids_map, faceted_fields, &mut items)?; + let fid = field_id(fields_ids_map, filterable_fields, &mut items)?; let (lresult, _) = pest_parse(items.next().unwrap()); let (rresult, _) = pest_parse(items.next().unwrap()); @@ -230,12 +230,12 @@ impl FacetCondition { fn equal( fields_ids_map: &FieldsIdsMap, - faceted_fields: &HashSet, + filterable_fields: &HashSet, item: Pair, ) -> anyhow::Result { let mut items = item.into_inner(); - let fid = field_id(fields_ids_map, faceted_fields, &mut items)?; + let fid = field_id(fields_ids_map, filterable_fields, &mut items)?; let value = items.next().unwrap(); let (result, svalue) = pest_parse(value); @@ -246,12 +246,12 @@ impl FacetCondition { fn greater_than( fields_ids_map: &FieldsIdsMap, - faceted_fields: &HashSet, + filterable_fields: &HashSet, item: Pair, ) -> anyhow::Result { let mut items = item.into_inner(); - let fid = field_id(fields_ids_map, faceted_fields, &mut items)?; + let fid = field_id(fields_ids_map, filterable_fields, &mut items)?; let value = items.next().unwrap(); let (result, _svalue) = pest_parse(value); @@ -261,12 +261,12 @@ impl FacetCondition { fn greater_than_or_equal( fields_ids_map: &FieldsIdsMap, - faceted_fields: &HashSet, + filterable_fields: &HashSet, item: Pair, ) -> anyhow::Result { let mut items = item.into_inner(); - let fid = field_id(fields_ids_map, faceted_fields, &mut items)?; + let fid = field_id(fields_ids_map, filterable_fields, &mut items)?; let value = items.next().unwrap(); let (result, _svalue) = pest_parse(value); @@ -276,12 +276,12 @@ impl FacetCondition { fn lower_than( fields_ids_map: &FieldsIdsMap, - faceted_fields: &HashSet, + filterable_fields: &HashSet, item: Pair, ) -> anyhow::Result { let mut items = item.into_inner(); - let fid = field_id(fields_ids_map, faceted_fields, &mut items)?; + let fid = field_id(fields_ids_map, filterable_fields, &mut items)?; let value = items.next().unwrap(); let (result, _svalue) = pest_parse(value); @@ -291,12 +291,12 @@ impl FacetCondition { fn lower_than_or_equal( fields_ids_map: &FieldsIdsMap, - faceted_fields: &HashSet, + filterable_fields: &HashSet, item: Pair, ) -> anyhow::Result { let mut items = item.into_inner(); - let fid = field_id(fields_ids_map, faceted_fields, &mut items)?; + let fid = field_id(fields_ids_map, filterable_fields, &mut items)?; let value = items.next().unwrap(); let (result, _svalue) = pest_parse(value); @@ -484,10 +484,10 @@ mod tests { options.map_size(10 * 1024 * 1024); // 10 MB let index = Index::new(options, &path).unwrap(); - // Set the faceted fields to be the channel. + // Set the filterable fields to be the channel. let mut wtxn = index.write_txn().unwrap(); let mut builder = Settings::new(&mut wtxn, &index, 0); - builder.set_faceted_fields(hashset!{ S("channel") }); + builder.set_filterable_fields(hashset!{ S("channel") }); builder.execute(|_, _| ()).unwrap(); wtxn.commit().unwrap(); @@ -513,10 +513,10 @@ mod tests { options.map_size(10 * 1024 * 1024); // 10 MB let index = Index::new(options, &path).unwrap(); - // Set the faceted fields to be the channel. + // Set the filterable fields to be the channel. let mut wtxn = index.write_txn().unwrap(); let mut builder = Settings::new(&mut wtxn, &index, 0); - builder.set_faceted_fields(hashset!{ "timestamp".into() }); + builder.set_filterable_fields(hashset!{ "timestamp".into() }); builder.execute(|_, _| ()).unwrap(); wtxn.commit().unwrap(); @@ -541,11 +541,11 @@ mod tests { options.map_size(10 * 1024 * 1024); // 10 MB let index = Index::new(options, &path).unwrap(); - // Set the faceted fields to be the channel. + // Set the filterable fields to be the channel. let mut wtxn = index.write_txn().unwrap(); let mut builder = Settings::new(&mut wtxn, &index, 0); builder.set_searchable_fields(vec![S("channel"), S("timestamp")]); // to keep the fields order - builder.set_faceted_fields(hashset!{ S("channel"), S("timestamp") }); + builder.set_filterable_fields(hashset!{ S("channel"), S("timestamp") }); builder.execute(|_, _| ()).unwrap(); wtxn.commit().unwrap(); @@ -588,11 +588,11 @@ mod tests { options.map_size(10 * 1024 * 1024); // 10 MB let index = Index::new(options, &path).unwrap(); - // Set the faceted fields to be the channel. + // Set the filterable fields to be the channel. let mut wtxn = index.write_txn().unwrap(); let mut builder = Settings::new(&mut wtxn, &index, 0); builder.set_searchable_fields(vec![S("channel"), S("timestamp")]); // to keep the fields order - builder.set_faceted_fields(hashset!{ S("channel"), S("timestamp") }); + builder.set_filterable_fields(hashset!{ S("channel"), S("timestamp") }); builder.execute(|_, _| ()).unwrap(); wtxn.commit().unwrap(); diff --git a/milli/src/search/facet/facet_distribution.rs b/milli/src/search/facet/facet_distribution.rs index c6122cc77..565f4c6dd 100644 --- a/milli/src/search/facet/facet_distribution.rs +++ b/milli/src/search/facet/facet_distribution.rs @@ -197,10 +197,10 @@ impl<'a> FacetDistribution<'a> { pub fn execute(&self) -> anyhow::Result>> { let fields_ids_map = self.index.fields_ids_map(self.rtxn)?; - let faceted_fields = self.index.faceted_fields(self.rtxn)?; + let filterable_fields = self.index.filterable_fields(self.rtxn)?; let mut distribution = BTreeMap::new(); - for name in faceted_fields { + for name in filterable_fields { let fid = fields_ids_map.id(&name).with_context(|| { format!("missing field name {:?} from the fields id map", name) })?; diff --git a/milli/src/search/mod.rs b/milli/src/search/mod.rs index fc64d020f..abf19844e 100644 --- a/milli/src/search/mod.rs +++ b/milli/src/search/mod.rs @@ -141,8 +141,8 @@ impl<'a> Search<'a> { Some(name) => { let field_ids_map = self.index.fields_ids_map(self.rtxn)?; let id = field_ids_map.id(name).expect("distinct not present in field map"); - let faceted_fields = self.index.faceted_fields(self.rtxn)?; - if faceted_fields.contains(name) { + let filterable_fields = self.index.filterable_fields(self.rtxn)?; + if filterable_fields.contains(name) { let distinct = FacetDistinct::new(id, self.index, self.rtxn); self.perform_sort(distinct, matching_words, criteria) } else { diff --git a/milli/src/update/facets.rs b/milli/src/update/facets.rs index f0eab6023..1c235a509 100644 --- a/milli/src/update/facets.rs +++ b/milli/src/update/facets.rs @@ -57,14 +57,14 @@ impl<'t, 'u, 'i> Facets<'t, 'u, 'i> { pub fn execute(self) -> anyhow::Result<()> { self.index.set_updated_at(self.wtxn, &Utc::now())?; - // We get the faceted fields to be able to create the facet levels. - let faceted_fields = self.index.faceted_fields_ids(self.wtxn)?; + // We get the filterable fields to be able to create the facet levels. + let filterable_fields = self.index.filterable_fields_ids(self.wtxn)?; debug!("Computing and writing the facet values levels docids into LMDB on disk..."); - for field_id in faceted_fields { - // Compute and store the faceted strings documents ids. - let string_documents_ids = compute_faceted_documents_ids( + for field_id in filterable_fields { + // Compute and store the filterable strings documents ids. + let string_documents_ids = compute_filterable_documents_ids( self.wtxn, self.index.facet_id_string_docids.remap_key_type::(), field_id, @@ -77,8 +77,8 @@ impl<'t, 'u, 'i> Facets<'t, 'u, 'i> { field_id, )?; - // Compute and store the faceted numbers documents ids. - let number_documents_ids = compute_faceted_documents_ids( + // Compute and store the filterable numbers documents ids. + let number_documents_ids = compute_filterable_documents_ids( self.wtxn, self.index.facet_id_f64_docids.remap_key_type::(), field_id, @@ -191,7 +191,7 @@ fn compute_facet_number_levels<'t>( writer_into_reader(writer, shrink_size) } -fn compute_faceted_documents_ids( +fn compute_filterable_documents_ids( rtxn: &heed::RoTxn, db: heed::Database, field_id: u8, diff --git a/milli/src/update/index_documents/mod.rs b/milli/src/update/index_documents/mod.rs index 71f281e98..7efd3631c 100644 --- a/milli/src/update/index_documents/mod.rs +++ b/milli/src/update/index_documents/mod.rs @@ -417,7 +417,7 @@ impl<'t, 'u, 'i, 'a> IndexDocuments<'t, 'u, 'i, 'a> { FacetLevel0NumbersDocids, } - let faceted_fields = self.index.faceted_fields_ids(self.wtxn)?; + let filterable_fields = self.index.filterable_fields_ids(self.wtxn)?; let searchable_fields: HashSet<_> = match self.index.searchable_fields_ids(self.wtxn)? { Some(fields) => fields.iter().copied().collect(), None => fields_ids_map.iter().map(|(id, _name)| id).collect(), @@ -453,7 +453,7 @@ impl<'t, 'u, 'i, 'a> IndexDocuments<'t, 'u, 'i, 'a> { .map(|(i, documents)| { let store = Store::new( searchable_fields.clone(), - faceted_fields.clone(), + filterable_fields.clone(), linked_hash_map_size, max_nb_chunks, max_memory_by_job, diff --git a/milli/src/update/settings.rs b/milli/src/update/settings.rs index 1571f627d..10b6b8cbe 100644 --- a/milli/src/update/settings.rs +++ b/milli/src/update/settings.rs @@ -66,7 +66,7 @@ pub struct Settings<'a, 't, 'u, 'i> { searchable_fields: Setting>, displayed_fields: Setting>, - faceted_fields: Setting>, + filterable_fields: Setting>, criteria: Setting>, stop_words: Setting>, distinct_attribute: Setting, @@ -92,7 +92,7 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> { thread_pool: None, searchable_fields: Setting::NotSet, displayed_fields: Setting::NotSet, - faceted_fields: Setting::NotSet, + filterable_fields: Setting::NotSet, criteria: Setting::NotSet, stop_words: Setting::NotSet, distinct_attribute: Setting::NotSet, @@ -117,12 +117,12 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> { self.displayed_fields = Setting::Set(names); } - pub fn reset_faceted_fields(&mut self) { - self.faceted_fields = Setting::Reset; + pub fn reset_filterable_fields(&mut self) { + self.filterable_fields = Setting::Reset; } - pub fn set_faceted_fields(&mut self, names_facet_types: HashSet) { - self.faceted_fields = Setting::Set(names_facet_types); + pub fn set_filterable_fields(&mut self, names: HashSet) { + self.filterable_fields = Setting::Set(names); } pub fn reset_criteria(&mut self) { @@ -267,7 +267,7 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> { Setting::Set(ref fields) => { // every time the searchable attributes are updated, we need to update the // ids for any settings that uses the facets. (displayed_fields, - // faceted_fields) + // filterable_fields) let old_fields_ids_map = self.index.fields_ids_map(self.wtxn)?; let mut new_fields_ids_map = FieldsIdsMap::new(); @@ -382,7 +382,7 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> { } fn update_facets(&mut self) -> anyhow::Result { - match self.faceted_fields { + match self.filterable_fields { Setting::Set(ref fields) => { let mut fields_ids_map = self.index.fields_ids_map(self.wtxn)?; let mut new_facets = HashSet::new(); @@ -390,10 +390,10 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> { fields_ids_map.insert(name).context("field id limit exceeded")?; new_facets.insert(name.clone()); } - self.index.put_faceted_fields(self.wtxn, &new_facets)?; + self.index.put_filterable_fields(self.wtxn, &new_facets)?; self.index.put_fields_ids_map(self.wtxn, &fields_ids_map)?; } - Setting::Reset => { self.index.delete_faceted_fields(self.wtxn)?; } + Setting::Reset => { self.index.delete_filterable_fields(self.wtxn)?; } Setting::NotSet => return Ok(false) } Ok(true) @@ -402,10 +402,10 @@ 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 faceted_fields = self.index.faceted_fields(&self.wtxn)?; + let filterable_fields = self.index.filterable_fields(&self.wtxn)?; let mut new_criteria = Vec::new(); for name in fields { - let criterion = Criterion::from_str(&faceted_fields, &name)?; + let criterion = Criterion::from_str(&filterable_fields, &name)?; new_criteria.push(criterion); } self.index.put_criteria(self.wtxn, &new_criteria)?; @@ -611,16 +611,16 @@ mod tests { } #[test] - fn set_faceted_fields() { + fn set_filterable_fields() { let path = tempfile::tempdir().unwrap(); let mut options = EnvOpenOptions::new(); options.map_size(10 * 1024 * 1024); // 10 MB let index = Index::new(options, &path).unwrap(); - // Set the faceted fields to be the age. + // Set the filterable fields to be the age. let mut wtxn = index.write_txn().unwrap(); let mut builder = Settings::new(&mut wtxn, &index, 0); - builder.set_faceted_fields(hashset!{ S("age") }); + builder.set_filterable_fields(hashset!{ S("age") }); builder.execute(|_, _| ()).unwrap(); // Then index some documents. @@ -637,7 +637,7 @@ mod tests { // Check that the displayed fields are correctly set. let rtxn = index.read_txn().unwrap(); - let fields_ids = index.faceted_fields(&rtxn).unwrap(); + let fields_ids = index.filterable_fields(&rtxn).unwrap(); assert_eq!(fields_ids, hashset!{ S("age") }); // Only count the field_id 0 and level 0 facet values. // TODO we must support typed CSVs for numbers to be understood. @@ -833,7 +833,7 @@ mod tests { let mut wtxn = index.write_txn().unwrap(); let mut builder = Settings::new(&mut wtxn, &index, 0); builder.set_displayed_fields(vec!["hello".to_string()]); - builder.set_faceted_fields(hashset!{ S("age"), S("toto") }); + builder.set_filterable_fields(hashset!{ S("age"), S("toto") }); builder.set_criteria(vec!["asc(toto)".to_string()]); builder.execute(|_, _| ()).unwrap(); wtxn.commit().unwrap();