Apply PR requests related to Refactor the FieldIdMapWithMetadata

crates/milli/src/fields_ids_map/global.rs

@@ -107,8 +107,15 @@ impl<'indexing> GlobalFieldsIdsMap<'indexing> {
     }
 
     /// Get the metadata of a field based on its id.
-    pub fn metadata(&self, id: FieldId) -> Option<Metadata> {
+    pub fn metadata(&mut self, id: FieldId) -> Option<Metadata> {
-        self.local.metadata(id).or_else(|| self.global.read().unwrap().metadata(id))
+        if self.local.metadata(id).is_none() {
+            let global = self.global.read().unwrap();
+
+            let (name, metadata) = global.name_with_metadata(id)?;
+            self.local.insert(name, id, metadata);
+        }
+
+        self.local.metadata(id)
     }
 }
 

crates/milli/src/fields_ids_map/metadata.rs

@@ -22,7 +22,7 @@ pub struct Metadata {
     pub distinct: bool,
     /// The field has been defined as asc/desc in the ranking rules.
     pub asc_desc: bool,
-    /// The field is a geo field.
+    /// The field is a geo field (`_geo`, `_geo.lat`, `_geo.lng`).
     pub geo: bool,
     /// The id of the localized attributes rule if the field is localized.
     pub localized_attributes_rule_id: Option<NonZeroU16>,
@@ -215,9 +215,8 @@ pub struct MetadataBuilder {
 impl MetadataBuilder {
     pub fn from_index(index: &Index, rtxn: &RoTxn) -> Result<Self> {
         let searchable_attributes = match index.user_defined_searchable_fields(rtxn)? {
-            Some(fields) if fields.contains(&"*") => None,
-            None => None,
             Some(fields) => Some(fields.into_iter().map(|s| s.to_string()).collect()),
+            None => None,
         };
         let filterable_attributes = index.filterable_attributes_rules(rtxn)?;
         let sortable_attributes = index.sortable_fields(rtxn)?;
@@ -225,14 +224,14 @@ impl MetadataBuilder {
         let distinct_attribute = index.distinct_field(rtxn)?.map(|s| s.to_string());
         let asc_desc_attributes = index.asc_desc_fields(rtxn)?;
 
-        Ok(Self {
+        Ok(Self::_new(
             searchable_attributes,
             filterable_attributes,
             sortable_attributes,
             localized_attributes,
             distinct_attribute,
             asc_desc_attributes,
-        })
+        ))
     }
 
     #[cfg(test)]
@@ -246,11 +245,29 @@ impl MetadataBuilder {
         localized_attributes: Option<Vec<LocalizedAttributesRule>>,
         distinct_attribute: Option<String>,
         asc_desc_attributes: HashSet<String>,
+    ) -> Self {
+        Self::_new(
+            searchable_attributes,
+            filterable_attributes,
+            sortable_attributes,
+            localized_attributes,
+            distinct_attribute,
+            asc_desc_attributes,
+        )
+    }
+
+    fn _new(
+        searchable_attributes: Option<Vec<String>>,
+        filterable_attributes: Vec<FilterableAttributesRule>,
+        sortable_attributes: HashSet<String>,
+        localized_attributes: Option<Vec<LocalizedAttributesRule>>,
+        distinct_attribute: Option<String>,
+        asc_desc_attributes: HashSet<String>,
     ) -> Self {
         let searchable_attributes = match searchable_attributes {
             Some(fields) if fields.iter().any(|f| f == "*") => None,
-            None => None,
             Some(fields) => Some(fields),
+            None => None,
         };
 
         Self {

crates/milli/src/search/facet/facet_distribution.rs

@@ -294,11 +294,7 @@ impl<'a> FacetDistribution<'a> {
             return Ok(Default::default());
         };
 
-        let fields_ids_map = self.index.fields_ids_map(self.rtxn)?;
+        let fields_ids_map = self.index.fields_ids_map_with_metadata(self.rtxn)?;
-        let fields_ids_map = FieldIdMapWithMetadata::new(
-            fields_ids_map,
-            MetadataBuilder::from_index(self.index, self.rtxn)?,
-        );
         let filterable_attributes_rules = self.index.filterable_attributes_rules(self.rtxn)?;
         self.check_faceted_fields(&fields_ids_map, &filterable_attributes_rules)?;
 
@@ -365,12 +361,17 @@ impl<'a> FacetDistribution<'a> {
         metadata: &Metadata,
         filterable_attributes_rules: &[FilterableAttributesRule],
     ) -> bool {
+        // If the field is not filterable, we don't want to compute the facet distribution.
+        if !metadata.filterable_attributes_features(filterable_attributes_rules).is_filterable() {
+            return false;
+        }
+
         match &self.facets {
             Some(facets) => {
                 // The list of facets provided by the user is a legacy pattern ("dog.age" must be selected with "dog").
                 facets.keys().any(|key| match_field_legacy(key, name) == PatternMatch::Match)
             }
-            None => metadata.is_faceted(filterable_attributes_rules),
+            None => true,
         }
     }
 
@@ -385,7 +386,9 @@ impl<'a> FacetDistribution<'a> {
             for field in facets.keys() {
                 let is_valid_faceted_field =
                     fields_ids_map.id_with_metadata(field).map_or(false, |(_, metadata)| {
-                        metadata.is_faceted(filterable_attributes_rules)
+                        metadata
+                            .filterable_attributes_features(filterable_attributes_rules)
+                            .is_filterable()
                     });
                 if !is_valid_faceted_field {
                     invalid_facets.insert(field.to_string());
@@ -397,7 +400,10 @@ impl<'a> FacetDistribution<'a> {
             let valid_facets_name = fields_ids_map
                 .iter()
                 .filter_map(|(_, name, metadata)| {
-                    if metadata.is_faceted(filterable_attributes_rules) {
+                    if metadata
+                        .filterable_attributes_features(filterable_attributes_rules)
+                        .is_filterable()
+                    {
                         Some(name.to_string())
                     } else {
                         None

crates/milli/src/update/new/indexer/post_processing.rs

@@ -25,7 +25,7 @@ use crate::{GlobalFieldsIdsMap, Index, Result};
 pub(super) fn post_process<MSP>(
     indexing_context: IndexingContext<MSP>,
     wtxn: &mut RwTxn<'_>,
-    global_fields_ids_map: GlobalFieldsIdsMap<'_>,
+    mut global_fields_ids_map: GlobalFieldsIdsMap<'_>,
     facet_field_ids_delta: FacetFieldIdsDelta,
 ) -> Result<()>
 where
@@ -33,7 +33,7 @@ where
 {
     let index = indexing_context.index;
     indexing_context.progress.update_progress(IndexingStep::PostProcessingFacets);
-    compute_facet_level_database(index, wtxn, facet_field_ids_delta, &global_fields_ids_map)?;
+    compute_facet_level_database(index, wtxn, facet_field_ids_delta, &mut global_fields_ids_map)?;
     compute_facet_search_database(index, wtxn, global_fields_ids_map)?;
     indexing_context.progress.update_progress(IndexingStep::PostProcessingWords);
     if let Some(prefix_delta) = compute_word_fst(index, wtxn)? {
@@ -170,7 +170,7 @@ fn compute_facet_level_database(
     index: &Index,
     wtxn: &mut RwTxn,
     mut facet_field_ids_delta: FacetFieldIdsDelta,
-    global_fields_ids_map: &GlobalFieldsIdsMap,
+    global_fields_ids_map: &mut GlobalFieldsIdsMap,
 ) -> Result<()> {
     let rtxn = index.read_txn()?;
     let filterable_attributes_rules = index.filterable_attributes_rules(&rtxn)?;