From c9b2d3ae1a27a6660b763873a5fb92d51d4f4081 Mon Sep 17 00:00:00 2001
From: Kerollmops <clement@meilisearch.com>
Date: Wed, 7 Apr 2021 14:52:51 +0200
Subject: [PATCH] Warn instead of returning an error when a conversion fails

---
 Cargo.lock                                |  6 ++---
 milli/src/update/index_documents/mod.rs   |  2 ++
 milli/src/update/index_documents/store.rs | 33 +++++++++++++++++++----
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index a78696e1e..bbe86a2a7 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1521,7 +1521,8 @@ checksum = "d4fd5641d01c8f18a23da7b6fe29298ff4b55afcccdf78973b24cf3175fee32e"
 [[package]]
 name = "pest"
 version = "2.1.3"
-source = "git+https://github.com/pest-parser/pest.git?rev=51fd1d49f1041f7839975664ef71fe15c7dcaf67#51fd1d49f1041f7839975664ef71fe15c7dcaf67"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "10f4872ae94d7b90ae48754df22fd42ad52ce740b8f370b03da4835417403e53"
 dependencies = [
  "ucd-trie",
 ]
@@ -1529,8 +1530,7 @@ dependencies = [
 [[package]]
 name = "pest"
 version = "2.1.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "10f4872ae94d7b90ae48754df22fd42ad52ce740b8f370b03da4835417403e53"
+source = "git+https://github.com/pest-parser/pest.git?rev=51fd1d49f1041f7839975664ef71fe15c7dcaf67#51fd1d49f1041f7839975664ef71fe15c7dcaf67"
 dependencies = [
  "ucd-trie",
 ]
diff --git a/milli/src/update/index_documents/mod.rs b/milli/src/update/index_documents/mod.rs
index fb1a2d6c0..52949c13c 100644
--- a/milli/src/update/index_documents/mod.rs
+++ b/milli/src/update/index_documents/mod.rs
@@ -440,6 +440,8 @@ impl<'t, 'u, 'i, 'a> IndexDocuments<'t, 'u, 'i, 'a> {
                 .enumerate()
                 .map(|(i, documents)| {
                     let store = Store::new(
+                        primary_key.clone(),
+                        fields_ids_map.clone(),
                         searchable_fields.clone(),
                         faceted_fields.clone(),
                         linked_hash_map_size,
diff --git a/milli/src/update/index_documents/store.rs b/milli/src/update/index_documents/store.rs
index 79b3cfc5f..0bd83b692 100644
--- a/milli/src/update/index_documents/store.rs
+++ b/milli/src/update/index_documents/store.rs
@@ -12,7 +12,7 @@ use fst::Set;
 use grenad::{Reader, FileFuse, Writer, Sorter, CompressionType};
 use heed::BytesEncode;
 use linked_hash_map::LinkedHashMap;
-use log::{debug, info};
+use log::{debug, info, warn};
 use meilisearch_tokenizer::{Analyzer, AnalyzerConfig, Token, TokenKind, token::SeparatorKind};
 use ordered_float::OrderedFloat;
 use roaring::RoaringBitmap;
@@ -24,7 +24,7 @@ use crate::heed_codec::facet::{FacetValueStringCodec, FacetLevelValueF64Codec};
 use crate::heed_codec::facet::{FieldDocIdFacetStringCodec, FieldDocIdFacetF64Codec};
 use crate::heed_codec::{BoRoaringBitmapCodec, CboRoaringBitmapCodec};
 use crate::update::UpdateIndexingStep;
-use crate::{json_to_string, SmallVec8, SmallVec32, Position, DocumentId, FieldId};
+use crate::{json_to_string, SmallVec8, SmallVec32, Position, DocumentId, FieldId, FieldsIdsMap};
 
 use super::{MergeFn, create_writer, create_sorter, writer_into_reader};
 use super::merge_function::{
@@ -50,6 +50,8 @@ pub struct Readers {
 
 pub struct Store<'s, A> {
     // Indexing parameters
+    primary_key: String,
+    fields_ids_map: FieldsIdsMap,
     searchable_fields: HashSet<FieldId>,
     faceted_fields: HashMap<FieldId, FacetType>,
     // Caches
@@ -78,6 +80,8 @@ pub struct Store<'s, A> {
 
 impl<'s, A: AsRef<[u8]>> Store<'s, A> {
     pub fn new(
+        primary_key: String,
+        fields_ids_map: FieldsIdsMap,
         searchable_fields: HashSet<FieldId>,
         faceted_fields: HashMap<FieldId, FacetType>,
         linked_hash_map_size: Option<usize>,
@@ -149,6 +153,8 @@ impl<'s, A: AsRef<[u8]>> Store<'s, A> {
 
         Ok(Store {
             // Indexing parameters.
+            primary_key,
+            fields_ids_map,
             searchable_fields,
             faceted_fields,
             // Caches
@@ -462,9 +468,26 @@ impl<'s, A: AsRef<[u8]>> Store<'s, A> {
                         let value = serde_json::from_slice(content)?;
 
                         if let Some(ftype) = self.faceted_fields.get(&attr) {
-                            let mut values = parse_facet_value(*ftype, &value).with_context(|| {
-                                format!("extracting facets from the value {}", value)
-                            })?;
+                            let mut values = match parse_facet_value(*ftype, &value) {
+                                Ok(values) => values,
+                                Err(e) => {
+                                    // We extract the name of the attribute and the document id
+                                    // to help users debug a facet type conversion.
+                                    let attr_name = self.fields_ids_map.name(attr).unwrap();
+                                    let document_id: Value = self.fields_ids_map.id(&self.primary_key)
+                                        .and_then(|fid| document.get(fid))
+                                        .map(serde_json::from_slice)
+                                        .unwrap()?;
+
+                                    let context = format!(
+                                        "while extracting facet from the {:?} attribute in the {} document",
+                                        attr_name, document_id,
+                                    );
+                                    warn!("{}", e.context(context));
+
+                                    SmallVec8::default()
+                                },
+                            };
                             facet_values.entry(attr).or_insert_with(SmallVec8::new).extend(values.drain(..));
                         }