Do not insert fields in the map when changing the settings

This commit is contained in:
Kerollmops 2021-07-22 17:11:17 +02:00
parent aa02a7fdd8
commit 7aa6cc9b04
No known key found for this signature in database
GPG Key ID: 92ADA4E935E71FA4
6 changed files with 46 additions and 96 deletions

View File

@ -8,7 +8,7 @@ use heed::types::*;
use heed::{Database, PolyDatabase, RoTxn, RwTxn}; use heed::{Database, PolyDatabase, RoTxn, RwTxn};
use roaring::RoaringBitmap; use roaring::RoaringBitmap;
use crate::error::{FieldIdMapMissingEntry, InternalError, UserError}; use crate::error::{InternalError, UserError};
use crate::fields_ids_map::FieldsIdsMap; use crate::fields_ids_map::FieldsIdsMap;
use crate::heed_codec::facet::{ use crate::heed_codec::facet::{
FacetLevelValueF64Codec, FacetStringLevelZeroCodec, FacetStringLevelZeroValueCodec, FacetLevelValueF64Codec, FacetStringLevelZeroCodec, FacetStringLevelZeroValueCodec,
@ -353,15 +353,8 @@ impl Index {
let fields_ids_map = self.fields_ids_map(rtxn)?; let fields_ids_map = self.fields_ids_map(rtxn)?;
let mut fields_ids = Vec::new(); let mut fields_ids = Vec::new();
for name in fields.into_iter() { for name in fields.into_iter() {
match fields_ids_map.id(name) { if let Some(field_id) = fields_ids_map.id(name) {
Some(field_id) => fields_ids.push(field_id), fields_ids.push(field_id);
None => {
return Err(FieldIdMapMissingEntry::FieldName {
field_name: name.to_string(),
process: "Index::displayed_fields_ids",
}
.into())
}
} }
} }
Ok(Some(fields_ids)) Ok(Some(fields_ids))
@ -403,15 +396,8 @@ impl Index {
let fields_ids_map = self.fields_ids_map(rtxn)?; let fields_ids_map = self.fields_ids_map(rtxn)?;
let mut fields_ids = Vec::new(); let mut fields_ids = Vec::new();
for name in fields { for name in fields {
match fields_ids_map.id(name) { if let Some(field_id) = fields_ids_map.id(name) {
Some(field_id) => fields_ids.push(field_id), fields_ids.push(field_id);
None => {
return Err(FieldIdMapMissingEntry::FieldName {
field_name: name.to_string(),
process: "Index::searchable_fields_ids",
}
.into())
}
} }
} }
Ok(Some(fields_ids)) Ok(Some(fields_ids))
@ -451,17 +437,8 @@ impl Index {
let mut fields_ids = HashSet::new(); let mut fields_ids = HashSet::new();
for name in fields { for name in fields {
match fields_ids_map.id(&name) { if let Some(field_id) = fields_ids_map.id(&name) {
Some(field_id) => { fields_ids.insert(field_id);
fields_ids.insert(field_id);
}
None => {
return Err(FieldIdMapMissingEntry::FieldName {
field_name: name,
process: "Index::filterable_fields_ids",
}
.into())
}
} }
} }
@ -498,17 +475,8 @@ impl Index {
let mut fields_ids = HashSet::new(); let mut fields_ids = HashSet::new();
for name in fields.into_iter() { for name in fields.into_iter() {
match fields_ids_map.id(&name) { if let Some(field_id) = fields_ids_map.id(&name) {
Some(field_id) => { fields_ids.insert(field_id);
fields_ids.insert(field_id);
}
None => {
return Err(FieldIdMapMissingEntry::FieldName {
field_name: name,
process: "Index::faceted_fields_ids",
}
.into())
}
} }
} }

View File

@ -6,7 +6,6 @@ use ordered_float::OrderedFloat;
use roaring::RoaringBitmap; use roaring::RoaringBitmap;
use super::{Criterion, CriterionParameters, CriterionResult}; use super::{Criterion, CriterionParameters, CriterionResult};
use crate::error::FieldIdMapMissingEntry;
use crate::search::criteria::{resolve_query_tree, CriteriaBuilder}; use crate::search::criteria::{resolve_query_tree, CriteriaBuilder};
use crate::search::facet::FacetNumberIter; use crate::search::facet::FacetNumberIter;
use crate::search::query_tree::Operation; use crate::search::query_tree::Operation;
@ -20,7 +19,7 @@ pub struct AscDesc<'t> {
index: &'t Index, index: &'t Index,
rtxn: &'t heed::RoTxn<'t>, rtxn: &'t heed::RoTxn<'t>,
field_name: String, field_name: String,
field_id: FieldId, field_id: Option<FieldId>,
ascending: bool, ascending: bool,
query_tree: Option<Operation>, query_tree: Option<Operation>,
candidates: Box<dyn Iterator<Item = heed::Result<RoaringBitmap>> + 't>, candidates: Box<dyn Iterator<Item = heed::Result<RoaringBitmap>> + 't>,
@ -57,11 +56,11 @@ impl<'t> AscDesc<'t> {
ascending: bool, ascending: bool,
) -> Result<Self> { ) -> Result<Self> {
let fields_ids_map = index.fields_ids_map(rtxn)?; let fields_ids_map = index.fields_ids_map(rtxn)?;
let field_id = let field_id = fields_ids_map.id(&field_name);
fields_ids_map.id(&field_name).ok_or_else(|| FieldIdMapMissingEntry::FieldName { let faceted_candidates = match field_id {
field_name: field_name.clone(), Some(field_id) => index.number_faceted_documents_ids(rtxn, field_id)?,
process: "AscDesc::new", None => RoaringBitmap::default(),
})?; };
Ok(AscDesc { Ok(AscDesc {
index, index,
@ -72,7 +71,7 @@ impl<'t> AscDesc<'t> {
query_tree: None, query_tree: None,
candidates: Box::new(std::iter::empty()), candidates: Box::new(std::iter::empty()),
allowed_candidates: RoaringBitmap::new(), allowed_candidates: RoaringBitmap::new(),
faceted_candidates: index.number_faceted_documents_ids(rtxn, field_id)?, faceted_candidates,
bucket_candidates: RoaringBitmap::new(), bucket_candidates: RoaringBitmap::new(),
parent, parent,
}) })
@ -132,13 +131,16 @@ impl<'t> Criterion for AscDesc<'t> {
} }
self.allowed_candidates = &candidates - params.excluded_candidates; self.allowed_candidates = &candidates - params.excluded_candidates;
self.candidates = facet_ordered( self.candidates = match self.field_id {
self.index, Some(field_id) => facet_ordered(
self.rtxn, self.index,
self.field_id, self.rtxn,
self.ascending, field_id,
candidates & &self.faceted_candidates, self.ascending,
)?; candidates & &self.faceted_candidates,
)?,
None => Box::new(std::iter::empty()),
};
} }
None => return Ok(None), None => return Ok(None),
}, },

View File

@ -5,7 +5,7 @@ use std::{cmp, fmt, mem};
use heed::types::ByteSlice; use heed::types::ByteSlice;
use roaring::RoaringBitmap; use roaring::RoaringBitmap;
use crate::error::{FieldIdMapMissingEntry, UserError}; use crate::error::UserError;
use crate::facet::FacetType; use crate::facet::FacetType;
use crate::heed_codec::facet::{ use crate::heed_codec::facet::{
FacetStringLevelZeroCodec, FieldDocIdFacetF64Codec, FieldDocIdFacetStringCodec, FacetStringLevelZeroCodec, FieldDocIdFacetF64Codec, FieldDocIdFacetStringCodec,
@ -277,13 +277,10 @@ impl<'a> FacetDistribution<'a> {
let mut distribution = BTreeMap::new(); let mut distribution = BTreeMap::new();
for name in fields { for name in fields {
let fid = if let Some(fid) = fields_ids_map.id(&name) {
fields_ids_map.id(&name).ok_or_else(|| FieldIdMapMissingEntry::FieldName { let values = self.facet_values(fid)?;
field_name: name.clone(), distribution.insert(name, values);
process: "FacetDistribution::execute", }
})?;
let values = self.facet_values(fid)?;
distribution.insert(name, values);
} }
Ok(distribution) Ok(distribution)

View File

@ -18,7 +18,6 @@ pub(crate) use self::facet::ParserRule;
pub use self::facet::{FacetDistribution, FacetNumberIter, FilterCondition, Operator}; pub use self::facet::{FacetDistribution, FacetNumberIter, FilterCondition, Operator};
pub use self::matching_words::MatchingWords; pub use self::matching_words::MatchingWords;
use self::query_tree::QueryTreeBuilder; use self::query_tree::QueryTreeBuilder;
use crate::error::FieldIdMapMissingEntry;
use crate::search::criteria::r#final::{Final, FinalResult}; use crate::search::criteria::r#final::{Final, FinalResult};
use crate::{DocumentId, Index, Result}; use crate::{DocumentId, Index, Result};
@ -142,13 +141,13 @@ impl<'a> Search<'a> {
None => self.perform_sort(NoopDistinct, matching_words, criteria), None => self.perform_sort(NoopDistinct, matching_words, criteria),
Some(name) => { Some(name) => {
let field_ids_map = self.index.fields_ids_map(self.rtxn)?; let field_ids_map = self.index.fields_ids_map(self.rtxn)?;
let id = match field_ids_map.id(name) {
field_ids_map.id(name).ok_or_else(|| FieldIdMapMissingEntry::FieldName { Some(fid) => {
field_name: name.to_string(), let distinct = FacetDistinct::new(fid, self.index, self.rtxn);
process: "distinct attribute", self.perform_sort(distinct, matching_words, criteria)
})?; }
let distinct = FacetDistinct::new(id, self.index, self.rtxn); None => Ok(SearchResult::default()),
self.perform_sort(distinct, matching_words, criteria) }
} }
} }
} }

View File

@ -8,7 +8,7 @@ use roaring::RoaringBitmap;
use serde_json::Value; use serde_json::Value;
use super::ClearDocuments; use super::ClearDocuments;
use crate::error::{FieldIdMapMissingEntry, InternalError, UserError}; use crate::error::{InternalError, UserError};
use crate::heed_codec::facet::FacetStringLevelZeroValueCodec; use crate::heed_codec::facet::FacetStringLevelZeroValueCodec;
use crate::heed_codec::CboRoaringBitmapCodec; use crate::heed_codec::CboRoaringBitmapCodec;
use crate::index::{db_name, main_key}; use crate::index::{db_name, main_key};
@ -82,11 +82,13 @@ impl<'t, 'u, 'i> DeleteDocuments<'t, 'u, 'i> {
key: Some(main_key::PRIMARY_KEY_KEY), key: Some(main_key::PRIMARY_KEY_KEY),
} }
})?; })?;
let id_field =
fields_ids_map.id(primary_key).ok_or_else(|| FieldIdMapMissingEntry::FieldName { // If we can't find the id of the primary key it means that the database
field_name: primary_key.to_string(), // is empty and it should be safe to return that we deleted 0 documents.
process: "DeleteDocuments::execute", let id_field = match fields_ids_map.id(primary_key) {
})?; Some(field) => field,
None => return Ok(0),
};
let Index { let Index {
env: _env, env: _env,

View File

@ -235,15 +235,9 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> {
fn update_displayed(&mut self) -> Result<bool> { fn update_displayed(&mut self) -> Result<bool> {
match self.displayed_fields { match self.displayed_fields {
Setting::Set(ref fields) => { Setting::Set(ref fields) => {
let mut fields_ids_map = self.index.fields_ids_map(self.wtxn)?;
// fields are deduplicated, only the first occurrence is taken into account // fields are deduplicated, only the first occurrence is taken into account
let names: Vec<_> = fields.iter().unique().map(String::as_str).collect(); let names: Vec<_> = fields.iter().unique().map(String::as_str).collect();
for name in names.iter() {
fields_ids_map.insert(name).ok_or(UserError::AttributeLimitReached)?;
}
self.index.put_displayed_fields(self.wtxn, &names)?; self.index.put_displayed_fields(self.wtxn, &names)?;
self.index.put_fields_ids_map(self.wtxn, &fields_ids_map)?;
} }
Setting::Reset => { Setting::Reset => {
self.index.delete_displayed_fields(self.wtxn)?; self.index.delete_displayed_fields(self.wtxn)?;
@ -256,11 +250,7 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> {
fn update_distinct_field(&mut self) -> Result<bool> { fn update_distinct_field(&mut self) -> Result<bool> {
match self.distinct_field { match self.distinct_field {
Setting::Set(ref attr) => { Setting::Set(ref attr) => {
let mut fields_ids_map = self.index.fields_ids_map(self.wtxn)?;
fields_ids_map.insert(attr).ok_or(UserError::AttributeLimitReached)?;
self.index.put_distinct_field(self.wtxn, &attr)?; self.index.put_distinct_field(self.wtxn, &attr)?;
self.index.put_fields_ids_map(self.wtxn, &fields_ids_map)?;
} }
Setting::Reset => { Setting::Reset => {
self.index.delete_distinct_field(self.wtxn)?; self.index.delete_distinct_field(self.wtxn)?;
@ -388,14 +378,11 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> {
fn update_filterable(&mut self) -> Result<()> { fn update_filterable(&mut self) -> Result<()> {
match self.filterable_fields { match self.filterable_fields {
Setting::Set(ref fields) => { Setting::Set(ref fields) => {
let mut fields_ids_map = self.index.fields_ids_map(self.wtxn)?;
let mut new_facets = HashSet::new(); let mut new_facets = HashSet::new();
for name in fields { for name in fields {
fields_ids_map.insert(name).ok_or(UserError::AttributeLimitReached)?;
new_facets.insert(name.clone()); new_facets.insert(name.clone());
} }
self.index.put_filterable_fields(self.wtxn, &new_facets)?; self.index.put_filterable_fields(self.wtxn, &new_facets)?;
self.index.put_fields_ids_map(self.wtxn, &fields_ids_map)?;
} }
Setting::Reset => { Setting::Reset => {
self.index.delete_filterable_fields(self.wtxn)?; self.index.delete_filterable_fields(self.wtxn)?;
@ -408,17 +395,12 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> {
fn update_criteria(&mut self) -> Result<()> { fn update_criteria(&mut self) -> Result<()> {
match self.criteria { match self.criteria {
Setting::Set(ref fields) => { Setting::Set(ref fields) => {
let mut fields_ids_map = self.index.fields_ids_map(self.wtxn)?;
let mut new_criteria = Vec::new(); let mut new_criteria = Vec::new();
for name in fields { for name in fields {
let criterion: Criterion = name.parse()?; let criterion: Criterion = name.parse()?;
if let Some(name) = criterion.field_name() {
fields_ids_map.insert(name).ok_or(UserError::AttributeLimitReached)?;
}
new_criteria.push(criterion); new_criteria.push(criterion);
} }
self.index.put_criteria(self.wtxn, &new_criteria)?; self.index.put_criteria(self.wtxn, &new_criteria)?;
self.index.put_fields_ids_map(self.wtxn, &fields_ids_map)?;
} }
Setting::Reset => { Setting::Reset => {
self.index.delete_criteria(self.wtxn)?; self.index.delete_criteria(self.wtxn)?;