From e35ef31738fdf2cd473ce55986c8e99d04966b69 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Thu, 13 Jun 2024 14:20:48 +0200 Subject: [PATCH] Small changes following review --- .../index_documents/extract/extract_vector_points.rs | 12 +++++++++--- milli/src/update/index_documents/transform.rs | 6 +++++- milli/src/update/settings.rs | 1 - milli/src/vector/settings.rs | 7 ------- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/milli/src/update/index_documents/extract/extract_vector_points.rs b/milli/src/update/index_documents/extract/extract_vector_points.rs index 0a27a28bd..736c21c9f 100644 --- a/milli/src/update/index_documents/extract/extract_vector_points.rs +++ b/milli/src/update/index_documents/extract/extract_vector_points.rs @@ -422,8 +422,11 @@ fn extract_vector_document_diff( VectorStateDelta::NowRemoved } } - // when the vectors are no longer user-provided, - // we generate the prompt unconditionally + // inline to the left is not supposed to be possible because the embedder is not new, so `_vectors` was removed from + // the previous version of the document. + // Manual -> Generated is also not possible without an Inline to the right (which is handled above) + // Generated -> Generated is handled above, so not possible + // As a result, this code is unreachable (_not_generated, VectorState::Generated) => { // Do we keep this document? let document_is_kept = obkv @@ -443,7 +446,10 @@ fn extract_vector_document_diff( VectorStateDelta::NowRemoved } } - (_old, VectorState::Manual) => { + // inline to the left is not possible because the embedder is not new, and so `_vectors` was removed from the previous + // version of the document. + // however the Rust type system cannot know that. + (_manual, VectorState::Manual) => { // Do we keep this document? let document_is_kept = obkv .iter() diff --git a/milli/src/update/index_documents/transform.rs b/milli/src/update/index_documents/transform.rs index 467a2810a..997ab64ff 100644 --- a/milli/src/update/index_documents/transform.rs +++ b/milli/src/update/index_documents/transform.rs @@ -866,6 +866,7 @@ impl<'a, 'i> Transform<'a, 'i> { // The operations that we must perform on the different fields. let mut operations = HashMap::new(); + let mut error_seen = false; let mut obkv_writer = KvWriter::<_, FieldId>::memory(); 'write_fid: for (id, val) in old_obkv.iter() { @@ -886,7 +887,10 @@ impl<'a, 'i> Transform<'a, 'i> { match existing_vectors { Ok(existing_vectors) => existing_vectors, Err(error) => { - tracing::error!(%error, "Unexpected `_vectors` field that is not a map. Treating as an empty map"); + if !error_seen { + tracing::error!(%error, "Unexpected `_vectors` field that is not a map. Treating as an empty map"); + error_seen = true; + } Default::default() } } diff --git a/milli/src/update/settings.rs b/milli/src/update/settings.rs index 5421b64a7..b792cde52 100644 --- a/milli/src/update/settings.rs +++ b/milli/src/update/settings.rs @@ -1182,7 +1182,6 @@ pub struct InnerIndexSettingsDiff { pub(crate) old: InnerIndexSettings, pub(crate) new: InnerIndexSettings, pub(crate) primary_key_id: Option, - // TODO: compare directly the embedders. pub(crate) embedding_config_updates: BTreeMap, pub(crate) settings_update_only: bool, /// The set of only the additional searchable fields. diff --git a/milli/src/vector/settings.rs b/milli/src/vector/settings.rs index edbed462c..9c7fb09b1 100644 --- a/milli/src/vector/settings.rs +++ b/milli/src/vector/settings.rs @@ -101,13 +101,6 @@ pub struct WriteBackToDocuments { } impl SettingsDiff { - pub fn should_reindex(&self) -> bool { - match self { - SettingsDiff::Remove { .. } | SettingsDiff::Reindex { .. } => true, - SettingsDiff::UpdateWithoutReindex { .. } => false, - } - } - pub fn from_settings(old: EmbeddingSettings, new: Setting) -> Self { match new { Setting::Set(new) => {