From ee8cbea8107e3f3a4b5a58ef4b404aef6cc23f4a Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Wed, 27 Mar 2024 16:59:22 +0100 Subject: [PATCH] Don't optimize reindexing when fields contain dots --- milli/src/update/settings.rs | 39 ++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/milli/src/update/settings.rs b/milli/src/update/settings.rs index 2f53718ac..344905f92 100644 --- a/milli/src/update/settings.rs +++ b/milli/src/update/settings.rs @@ -1032,13 +1032,14 @@ impl<'a, 't, 'i> Settings<'a, 't, 'i> { { self.index.set_updated_at(self.wtxn, &OffsetDateTime::now_utc())?; + // Note: this MUST be before `update_sortable` so that we can get the old value to compare with the updated value afterwards + let existing_fields: HashSet<_> = self .index .field_distribution(self.wtxn)? .into_iter() .filter_map(|(field, count)| (count != 0).then_some(field)) .collect(); - let old_faceted_fields = self.index.user_defined_faceted_fields(self.wtxn)?; let old_fields_ids_map = self.index.fields_ids_map(self.wtxn)?; @@ -1055,13 +1056,7 @@ impl<'a, 't, 'i> Settings<'a, 't, 'i> { self.update_sort_facet_values_by()?; self.update_pagination_max_total_hits()?; - // If there is new faceted fields we indicate that we must reindex as we must - // index new fields as facets. It means that the distinct attribute, - // an Asc/Desc criterion or a filtered attribute as be added or removed. - let new_faceted_fields = self.index.user_defined_faceted_fields(self.wtxn)?; - let faceted_updated = - (&existing_fields - &old_faceted_fields) != (&existing_fields - &new_faceted_fields); - + let faceted_updated = self.update_faceted(existing_fields, old_faceted_fields)?; let stop_words_updated = self.update_stop_words()?; let non_separator_tokens_updated = self.update_non_separator_tokens()?; let separator_tokens_updated = self.update_separator_tokens()?; @@ -1094,6 +1089,34 @@ impl<'a, 't, 'i> Settings<'a, 't, 'i> { Ok(()) } + + fn update_faceted( + &self, + existing_fields: HashSet, + old_faceted_fields: HashSet, + ) -> Result { + if existing_fields.iter().any(|field| field.contains('.')) { + return Ok(true); + } + + if old_faceted_fields.iter().any(|field| field.contains('.')) { + return Ok(true); + } + + // If there is new faceted fields we indicate that we must reindex as we must + // index new fields as facets. It means that the distinct attribute, + // an Asc/Desc criterion or a filtered attribute as be added or removed. + let new_faceted_fields = self.index.user_defined_faceted_fields(self.wtxn)?; + + if new_faceted_fields.iter().any(|field| field.contains('.')) { + return Ok(true); + } + + let faceted_updated = + (&existing_fields - &old_faceted_fields) != (&existing_fields - &new_faceted_fields); + + Ok(faceted_updated) + } } fn validate_prompt(