From 3e94a907229c9a22bbad9bd470601c34ac1e119b Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Tue, 21 May 2024 13:39:46 +0200 Subject: [PATCH] Fixes --- meilisearch/tests/search/geo.rs | 32 ++++++++++++------- .../extract/extract_fid_docid_facet_values.rs | 15 ++++++--- .../extract/extract_geo_points.rs | 4 +-- .../src/update/index_documents/extract/mod.rs | 2 +- milli/src/update/index_documents/mod.rs | 3 -- milli/src/update/settings.rs | 2 +- 6 files changed, 34 insertions(+), 24 deletions(-) diff --git a/meilisearch/tests/search/geo.rs b/meilisearch/tests/search/geo.rs index 0169734c8..8754453ba 100644 --- a/meilisearch/tests/search/geo.rs +++ b/meilisearch/tests/search/geo.rs @@ -141,17 +141,6 @@ async fn bug_4640() { snapshot!(json_string!(response, { ".processingTimeMs" => "[time]" }), @r###" { "hits": [ - { - "id": 1, - "name": "Taco Truck", - "address": "444 Salsa Street, Burritoville", - "type": "Mexican", - "rating": 9, - "_geo": { - "lat": 34.0522, - "lng": -118.2437 - } - }, { "id": 2, "name": "La Bella Italia", @@ -162,13 +151,32 @@ async fn bug_4640() { "lat": "45.4777599", "lng": "9.1967508" } + }, + { + "id": 1, + "name": "Taco Truck", + "address": "444 Salsa Street, Burritoville", + "type": "Mexican", + "rating": 9, + "_geo": { + "lat": 34.0522, + "lng": -118.2437 + }, + "_geoDistance": 9714063 + }, + { + "id": 3, + "name": "Crêpe Truck", + "address": "2 Billig Avenue, Rouenville", + "type": "French", + "rating": 10 } ], "query": "", "processingTimeMs": "[time]", "limit": 20, "offset": 0, - "estimatedTotalHits": 2 + "estimatedTotalHits": 3 } "###); }, diff --git a/milli/src/update/index_documents/extract/extract_fid_docid_facet_values.rs b/milli/src/update/index_documents/extract/extract_fid_docid_facet_values.rs index 030303cd9..caf53550c 100644 --- a/milli/src/update/index_documents/extract/extract_fid_docid_facet_values.rs +++ b/milli/src/update/index_documents/extract/extract_fid_docid_facet_values.rs @@ -45,7 +45,6 @@ pub fn extract_fid_docid_facet_values( obkv_documents: grenad::Reader, indexer: GrenadParameters, settings_diff: &InnerIndexSettingsDiff, - geo_fields_ids: Option<(FieldId, FieldId)>, ) -> Result { puffin::profile_function!(); @@ -127,12 +126,18 @@ pub fn extract_fid_docid_facet_values( add_exists.insert(document); } - let geo_support = - geo_fields_ids.map_or(false, |(lat, lng)| field_id == lat || field_id == lng); + let del_geo_support = settings_diff + .old + .geo_fields_ids + .map_or(false, |(lat, lng)| field_id == lat || field_id == lng); + let add_geo_support = settings_diff + .new + .geo_fields_ids + .map_or(false, |(lat, lng)| field_id == lat || field_id == lng); let del_filterable_values = - del_value.map(|value| extract_facet_values(&value, geo_support)); + del_value.map(|value| extract_facet_values(&value, del_geo_support)); let add_filterable_values = - add_value.map(|value| extract_facet_values(&value, geo_support)); + add_value.map(|value| extract_facet_values(&value, add_geo_support)); // Those closures are just here to simplify things a bit. let mut insert_numbers_diff = |del_numbers, add_numbers| { diff --git a/milli/src/update/index_documents/extract/extract_geo_points.rs b/milli/src/update/index_documents/extract/extract_geo_points.rs index 01d53be1a..cbbb4e20b 100644 --- a/milli/src/update/index_documents/extract/extract_geo_points.rs +++ b/milli/src/update/index_documents/extract/extract_geo_points.rs @@ -81,10 +81,10 @@ fn extract_lat_lng( let lng = document.get(lng_fid).map(KvReaderDelAdd::new).and_then(|r| r.get(deladd)); let (lat, lng) = match (lat, lng) { (Some(lat), Some(lng)) => (lat, lng), - (Some(lat), None) => { + (Some(_), None) => { return Err(GeoError::MissingLatitude { document_id: document_id() }.into()) } - (None, Some(lng)) => { + (None, Some(_)) => { return Err(GeoError::MissingLongitude { document_id: document_id() }.into()) } (None, None) => return Ok(None), diff --git a/milli/src/update/index_documents/extract/mod.rs b/milli/src/update/index_documents/extract/mod.rs index 35a4c4344..237e19b2a 100644 --- a/milli/src/update/index_documents/extract/mod.rs +++ b/milli/src/update/index_documents/extract/mod.rs @@ -310,6 +310,7 @@ fn send_and_extract_flattened_documents_data( if settings_diff.run_geo_indexing() { let documents_chunk_cloned = flattened_documents_chunk.clone(); let lmdb_writer_sx_cloned = lmdb_writer_sx.clone(); + let settings_diff = settings_diff.clone(); rayon::spawn(move || { let result = extract_geo_points(documents_chunk_cloned, indexer, primary_key_id, &settings_diff); @@ -351,7 +352,6 @@ fn send_and_extract_flattened_documents_data( flattened_documents_chunk.clone(), indexer, &settings_diff, - geo_fields_ids, )?; // send fid_docid_facet_numbers_chunk to DB writer diff --git a/milli/src/update/index_documents/mod.rs b/milli/src/update/index_documents/mod.rs index b4ab63def..01684a54a 100644 --- a/milli/src/update/index_documents/mod.rs +++ b/milli/src/update/index_documents/mod.rs @@ -324,9 +324,6 @@ where // get the primary key field id let primary_key_id = settings_diff.new.fields_ids_map.id(&primary_key).unwrap(); - // get the fid of the `_geo.lat` and `_geo.lng` fields. - let mut field_id_map = self.index.fields_ids_map(self.wtxn)?; - let pool_params = GrenadParameters { chunk_compression_type: self.indexer_config.chunk_compression_type, chunk_compression_level: self.indexer_config.chunk_compression_level, diff --git a/milli/src/update/settings.rs b/milli/src/update/settings.rs index 5c53e1324..dfbc42f6c 100644 --- a/milli/src/update/settings.rs +++ b/milli/src/update/settings.rs @@ -1191,7 +1191,7 @@ impl InnerIndexSettings { let stop_words = stop_words.map(|sw| sw.map_data(Vec::from).unwrap()); let allowed_separators = index.allowed_separators(rtxn)?; let dictionary = index.dictionary(rtxn)?; - let fields_ids_map = index.fields_ids_map(rtxn)?; + let mut fields_ids_map = index.fields_ids_map(rtxn)?; let user_defined_searchable_fields = index.user_defined_searchable_fields(rtxn)?; let user_defined_searchable_fields = user_defined_searchable_fields.map(|sf| sf.into_iter().map(String::from).collect());