Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Commit

Permalink
Use the Error enum everywhere in the project
Browse files Browse the repository at this point in the history
  • Loading branch information
Kerollmops committed Jun 14, 2021
1 parent ca78cb5 commit 312c2d1
Show file tree
Hide file tree
Showing 35 changed files with 385 additions and 300 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion milli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ authors = ["Kerollmops <clement@meilisearch.com>"]
edition = "2018"

[dependencies]
anyhow = "1.0.38"
bstr = "0.2.15"
byteorder = "1.4.2"
chrono = { version = "0.4.19", features = ["serde"] }
Expand Down
11 changes: 7 additions & 4 deletions milli/src/criterion.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::fmt;
use std::str::FromStr;

use anyhow::{Context, bail};
use regex::Regex;
use serde::{Serialize, Deserialize};
use once_cell::sync::Lazy;

use crate::error::{Error, UserError};

static ASC_DESC_REGEX: Lazy<Regex> = Lazy::new(|| {
Regex::new(r#"(asc|desc)\(([\w_-]+)\)"#).unwrap()
});
Expand Down Expand Up @@ -41,7 +42,7 @@ impl Criterion {
}

impl FromStr for Criterion {
type Err = anyhow::Error;
type Err = Error;

fn from_str(txt: &str) -> Result<Criterion, Self::Err> {
match txt {
Expand All @@ -51,13 +52,15 @@ impl FromStr for Criterion {
"attribute" => Ok(Criterion::Attribute),
"exactness" => Ok(Criterion::Exactness),
text => {
let caps = ASC_DESC_REGEX.captures(text).with_context(|| format!("unknown criterion name: {}", text))?;
let caps = ASC_DESC_REGEX.captures(text).ok_or_else(|| {
UserError::InvalidCriterionName { name: text.to_string() }
})?;
let order = caps.get(1).unwrap().as_str();
let field_name = caps.get(2).unwrap().as_str();
match order {
"asc" => Ok(Criterion::Asc(field_name.to_string())),
"desc" => Ok(Criterion::Desc(field_name.to_string())),
otherwise => bail!("unknown criterion name: {}", otherwise),
text => return Err(UserError::InvalidCriterionName { name: text.to_string() }.into()),
}
},
}
Expand Down
22 changes: 11 additions & 11 deletions milli/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ use std::borrow::Cow;
use std::collections::{HashMap, HashSet};
use std::path::Path;

use anyhow::Context;
use chrono::{DateTime, Utc};
use heed::{Database, PolyDatabase, RoTxn, RwTxn};
use heed::types::*;
use roaring::RoaringBitmap;

use crate::error::UserError;
use crate::{Criterion, default_criteria, FacetDistribution, FieldsDistribution, Search};
use crate::{BEU32, DocumentId, ExternalDocumentsIds, FieldId};
use crate::{BEU32, DocumentId, ExternalDocumentsIds, FieldId, Result};
use crate::{
BEU32StrCodec, BoRoaringBitmapCodec, CboRoaringBitmapCodec,
ObkvCodec, RoaringBitmapCodec, RoaringBitmapLenCodec, StrLevelPositionCodec, StrStrU8Codec,
Expand Down Expand Up @@ -84,7 +84,7 @@ pub struct Index {
}

impl Index {
pub fn new<P: AsRef<Path>>(mut options: heed::EnvOpenOptions, path: P) -> anyhow::Result<Index> {
pub fn new<P: AsRef<Path>>(mut options: heed::EnvOpenOptions, path: P) -> Result<Index> {
options.max_dbs(14);

let env = options.open(path)?;
Expand Down Expand Up @@ -173,7 +173,7 @@ impl Index {
}

/// Returns the number of documents indexed in the database.
pub fn number_of_documents(&self, rtxn: &RoTxn) -> anyhow::Result<u64> {
pub fn number_of_documents(&self, rtxn: &RoTxn) -> Result<u64> {
let count = self.main.get::<_, Str, RoaringBitmapLenCodec>(rtxn, DOCUMENTS_IDS_KEY)?;
Ok(count.unwrap_or_default())
}
Expand Down Expand Up @@ -215,7 +215,7 @@ impl Index {

/// Returns the external documents ids map which associate the external ids
/// with the internal ids (i.e. `u32`).
pub fn external_documents_ids<'t>(&self, rtxn: &'t RoTxn) -> anyhow::Result<ExternalDocumentsIds<'t>> {
pub fn external_documents_ids<'t>(&self, rtxn: &'t RoTxn) -> Result<ExternalDocumentsIds<'t>> {
let hard = self.main.get::<_, Str, ByteSlice>(rtxn, HARD_EXTERNAL_DOCUMENTS_IDS_KEY)?;
let soft = self.main.get::<_, Str, ByteSlice>(rtxn, SOFT_EXTERNAL_DOCUMENTS_IDS_KEY)?;
let hard = match hard {
Expand Down Expand Up @@ -504,7 +504,7 @@ impl Index {
}

/// Returns the FST which is the words dictionary of the engine.
pub fn words_fst<'t>(&self, rtxn: &'t RoTxn) -> anyhow::Result<fst::Set<Cow<'t, [u8]>>> {
pub fn words_fst<'t>(&self, rtxn: &'t RoTxn) -> Result<fst::Set<Cow<'t, [u8]>>> {
match self.main.get::<_, Str, ByteSlice>(rtxn, WORDS_FST_KEY)? {
Some(bytes) => Ok(fst::Set::new(bytes)?.map_data(Cow::Borrowed)?),
None => Ok(fst::Set::default().map_data(Cow::Owned)?),
Expand All @@ -521,7 +521,7 @@ impl Index {
self.main.delete::<_, Str>(wtxn, STOP_WORDS_KEY)
}

pub fn stop_words<'t>(&self, rtxn: &'t RoTxn) -> anyhow::Result<Option<fst::Set<&'t [u8]>>> {
pub fn stop_words<'t>(&self, rtxn: &'t RoTxn) -> Result<Option<fst::Set<&'t [u8]>>> {
match self.main.get::<_, Str, ByteSlice>(rtxn, STOP_WORDS_KEY)? {
Some(bytes) => Ok(Some(fst::Set::new(bytes)?)),
None => Ok(None),
Expand Down Expand Up @@ -555,7 +555,7 @@ impl Index {
}

/// Returns the FST which is the words prefixes dictionnary of the engine.
pub fn words_prefixes_fst<'t>(&self, rtxn: &'t RoTxn) -> anyhow::Result<fst::Set<Cow<'t, [u8]>>> {
pub fn words_prefixes_fst<'t>(&self, rtxn: &'t RoTxn) -> Result<fst::Set<Cow<'t, [u8]>>> {
match self.main.get::<_, Str, ByteSlice>(rtxn, WORDS_PREFIXES_FST_KEY)? {
Some(bytes) => Ok(fst::Set::new(bytes)?.map_data(Cow::Borrowed)?),
None => Ok(fst::Set::default().map_data(Cow::Owned)?),
Expand All @@ -577,13 +577,13 @@ impl Index {
&self,
rtxn: &'t RoTxn,
ids: impl IntoIterator<Item=DocumentId>,
) -> anyhow::Result<Vec<(DocumentId, obkv::KvReader<'t>)>>
) -> Result<Vec<(DocumentId, obkv::KvReader<'t>)>>
{
let mut documents = Vec::new();

for id in ids {
let kv = self.documents.get(rtxn, &BEU32::new(id))?
.with_context(|| format!("Could not find document {}", id))?;
.ok_or_else(|| UserError::UnknownInternalDocumentId { document_id: id })?;
documents.push((id, kv));
}

Expand All @@ -594,7 +594,7 @@ impl Index {
pub fn all_documents<'t>(
&self,
rtxn: &'t RoTxn,
) -> anyhow::Result<impl Iterator<Item = heed::Result<(DocumentId, obkv::KvReader<'t>)>>> {
) -> Result<impl Iterator<Item = heed::Result<(DocumentId, obkv::KvReader<'t>)>>> {
Ok(self
.documents
.iter(rtxn)?
Expand Down
16 changes: 11 additions & 5 deletions milli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ pub mod update;
use std::borrow::Cow;
use std::collections::HashMap;
use std::hash::BuildHasherDefault;
use std::result::Result as StdResult;

use anyhow::Context;
use fxhash::{FxHasher32, FxHasher64};
use serde_json::{Map, Value};

pub use self::criterion::{Criterion, default_criteria};
pub use self::error::Error;
pub use self::external_documents_ids::ExternalDocumentsIds;
pub use self::fields_ids_map::FieldsIdsMap;
pub use self::heed_codec::{BEU32StrCodec, StrStrU8Codec, StrLevelPositionCodec, ObkvCodec, FieldIdWordCountCodec};
Expand All @@ -30,6 +31,8 @@ pub use self::index::Index;
pub use self::search::{Search, FacetDistribution, FilterCondition, SearchResult, MatchingWords};
pub use self::tree_level::TreeLevel;

pub type Result<T> = std::result::Result<T, error::Error>;

pub type FastMap4<K, V> = HashMap<K, V, BuildHasherDefault<FxHasher32>>;
pub type FastMap8<K, V> = HashMap<K, V, BuildHasherDefault<FxHasher64>>;
pub type SmallString32 = smallstr::SmallString<[u8; 32]>;
Expand All @@ -44,21 +47,24 @@ pub type FieldId = u8;
pub type Position = u32;
pub type FieldsDistribution = HashMap<String, u64>;

type MergeFn = for<'a> fn(&[u8], &[Cow<'a, [u8]>]) -> anyhow::Result<Vec<u8>>;
type MergeFn<E> = for<'a> fn(&[u8], &[Cow<'a, [u8]>]) -> StdResult<Vec<u8>, E>;

/// Transform a raw obkv store into a JSON Object.
pub fn obkv_to_json(
displayed_fields: &[FieldId],
fields_ids_map: &FieldsIdsMap,
obkv: obkv::KvReader,
) -> anyhow::Result<Map<String, Value>>
) -> Result<Map<String, Value>>
{
displayed_fields.iter()
.copied()
.flat_map(|id| obkv.get(id).map(|value| (id, value)))
.map(|(id, value)| {
let name = fields_ids_map.name(id).context("unknown obkv field id")?;
let value = serde_json::from_slice(value)?;
let name = fields_ids_map.name(id).ok_or(error::FieldIdMapMissingEntry::FieldId {
field_id: id,
from_db_name: "documents",
})?;
let value = serde_json::from_slice(value).map_err(error::InternalError::SerdeJson)?;
Ok((name.to_owned(), value))
})
.collect()
Expand Down
21 changes: 12 additions & 9 deletions milli/src/search/criteria/asc_desc.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use std::mem::take;

use anyhow::Context;
use itertools::Itertools;
use log::debug;
use ordered_float::OrderedFloat;
use roaring::RoaringBitmap;

use crate::error::FieldIdMapMissingEntry;
use crate::search::criteria::{resolve_query_tree, CriteriaBuilder};
use crate::search::facet::FacetIter;
use crate::search::query_tree::Operation;
use crate::{FieldId, Index};
use crate::{FieldId, Index, Result};
use super::{Criterion, CriterionParameters, CriterionResult};

/// Threshold on the number of candidates that will make
Expand All @@ -36,7 +36,7 @@ impl<'t> AscDesc<'t> {
rtxn: &'t heed::RoTxn,
parent: Box<dyn Criterion + 't>,
field_name: String,
) -> anyhow::Result<Self> {
) -> Result<Self> {
Self::new(index, rtxn, parent, field_name, true)
}

Expand All @@ -45,7 +45,7 @@ impl<'t> AscDesc<'t> {
rtxn: &'t heed::RoTxn,
parent: Box<dyn Criterion + 't>,
field_name: String,
) -> anyhow::Result<Self> {
) -> Result<Self> {
Self::new(index, rtxn, parent, field_name, false)
}

Expand All @@ -55,11 +55,14 @@ impl<'t> AscDesc<'t> {
parent: Box<dyn Criterion + 't>,
field_name: String,
ascending: bool,
) -> anyhow::Result<Self> {
) -> Result<Self> {
let fields_ids_map = index.fields_ids_map(rtxn)?;
let field_id = fields_ids_map
.id(&field_name)
.with_context(|| format!("field {:?} isn't registered", field_name))?;
.ok_or_else(|| FieldIdMapMissingEntry::FieldName {
field_name: field_name.clone(),
from_db_name: "asc-desc",
})?;

Ok(AscDesc {
index,
Expand All @@ -79,7 +82,7 @@ impl<'t> AscDesc<'t> {

impl<'t> Criterion for AscDesc<'t> {
#[logging_timer::time("AscDesc::{}")]
fn next(&mut self, params: &mut CriterionParameters) -> anyhow::Result<Option<CriterionResult>> {
fn next(&mut self, params: &mut CriterionParameters) -> Result<Option<CriterionResult>> {
// remove excluded candidates when next is called, instead of doing it in the loop.
self.allowed_candidates -= params.excluded_candidates;

Expand Down Expand Up @@ -162,7 +165,7 @@ fn facet_ordered<'t>(
field_id: FieldId,
ascending: bool,
candidates: RoaringBitmap,
) -> anyhow::Result<Box<dyn Iterator<Item = heed::Result<RoaringBitmap>> + 't>> {
) -> Result<Box<dyn Iterator<Item = heed::Result<RoaringBitmap>> + 't>> {
if candidates.len() <= CANDIDATES_THRESHOLD {
let iter = iterative_facet_ordered_iter(index, rtxn, field_id, ascending, candidates)?;
Ok(Box::new(iter.map(Ok)) as Box<dyn Iterator<Item = _>>)
Expand All @@ -186,7 +189,7 @@ fn iterative_facet_ordered_iter<'t>(
field_id: FieldId,
ascending: bool,
candidates: RoaringBitmap,
) -> anyhow::Result<impl Iterator<Item = RoaringBitmap> + 't> {
) -> Result<impl Iterator<Item = RoaringBitmap> + 't> {
let mut docids_values = Vec::with_capacity(candidates.len() as usize);
for docid in candidates.iter() {
let left = (field_id, docid, f64::MIN);
Expand Down
17 changes: 11 additions & 6 deletions milli/src/search/criteria/attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::mem::take;

use roaring::RoaringBitmap;

use crate::{TreeLevel, search::build_dfa};
use crate::{TreeLevel, Result, search::build_dfa};
use crate::search::criteria::Query;
use crate::search::query_tree::{Operation, QueryKind};
use crate::search::{word_derivations, WordDerivationsCache};
Expand Down Expand Up @@ -48,7 +48,7 @@ impl<'t> Attribute<'t> {

impl<'t> Criterion for Attribute<'t> {
#[logging_timer::time("Attribute::{}")]
fn next(&mut self, params: &mut CriterionParameters) -> anyhow::Result<Option<CriterionResult>> {
fn next(&mut self, params: &mut CriterionParameters) -> Result<Option<CriterionResult>> {
// remove excluded candidates when next is called, instead of doing it in the loop.
if let Some((_, _, allowed_candidates)) = self.state.as_mut() {
*allowed_candidates -= params.excluded_candidates;
Expand Down Expand Up @@ -224,7 +224,12 @@ struct QueryLevelIterator<'t, 'q> {
}

impl<'t, 'q> QueryLevelIterator<'t, 'q> {
fn new(ctx: &'t dyn Context<'t>, queries: &'q [Query], wdcache: &mut WordDerivationsCache) -> anyhow::Result<Option<Self>> {
fn new(
ctx: &'t dyn Context<'t>,
queries: &'q [Query],
wdcache: &mut WordDerivationsCache,
) -> Result<Option<Self>>
{
let mut inner = Vec::with_capacity(queries.len());
for query in queries {
match &query.kind {
Expand Down Expand Up @@ -471,7 +476,7 @@ fn initialize_query_level_iterators<'t, 'q>(
branches: &'q FlattenedQueryTree,
allowed_candidates: &RoaringBitmap,
wdcache: &mut WordDerivationsCache,
) -> anyhow::Result<BinaryHeap<Branch<'t, 'q>>> {
) -> Result<BinaryHeap<Branch<'t, 'q>>> {

let mut positions = BinaryHeap::with_capacity(branches.len());
for branch in branches {
Expand Down Expand Up @@ -521,7 +526,7 @@ fn set_compute_candidates<'t>(
branches: &FlattenedQueryTree,
allowed_candidates: &RoaringBitmap,
wdcache: &mut WordDerivationsCache,
) -> anyhow::Result<Option<RoaringBitmap>>
) -> Result<Option<RoaringBitmap>>
{
let mut branches_heap = initialize_query_level_iterators(ctx, branches, allowed_candidates, wdcache)?;
let lowest_level = TreeLevel::min_value();
Expand Down Expand Up @@ -573,7 +578,7 @@ fn linear_compute_candidates(
ctx: &dyn Context,
branches: &FlattenedQueryTree,
allowed_candidates: &RoaringBitmap,
) -> anyhow::Result<BTreeMap<u64, RoaringBitmap>>
) -> Result<BTreeMap<u64, RoaringBitmap>>
{
fn compute_candidate_rank(branches: &FlattenedQueryTree, words_positions: HashMap<String, RoaringBitmap>) -> u64 {
let mut min_rank = u64::max_value();
Expand Down
6 changes: 3 additions & 3 deletions milli/src/search/criteria/exactness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::search::criteria::{
CriterionResult,
resolve_query_tree,
};
use crate::TreeLevel;
use crate::{TreeLevel, Result};

pub struct Exactness<'t> {
ctx: &'t dyn Context<'t>,
Expand Down Expand Up @@ -45,7 +45,7 @@ impl<'t> Exactness<'t> {

impl<'t> Criterion for Exactness<'t> {
#[logging_timer::time("Exactness::{}")]
fn next(&mut self, params: &mut CriterionParameters) -> anyhow::Result<Option<CriterionResult>> {
fn next(&mut self, params: &mut CriterionParameters) -> Result<Option<CriterionResult>> {
// remove excluded candidates when next is called, instead of doing it in the loop.
if let Some(state) = self.state.as_mut() {
state.difference_with(params.excluded_candidates);
Expand Down Expand Up @@ -158,7 +158,7 @@ fn resolve_state(
ctx: &dyn Context,
state: State,
query: &[ExactQueryPart],
) -> anyhow::Result<(RoaringBitmap, Option<State>)>
) -> Result<(RoaringBitmap, Option<State>)>
{
use State::*;
match state {
Expand Down
3 changes: 2 additions & 1 deletion milli/src/search/criteria/final.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use log::debug;
use roaring::RoaringBitmap;

use crate::Result;
use crate::search::query_tree::Operation;
use crate::search::WordDerivationsCache;
use super::{resolve_query_tree, Criterion, CriterionResult, CriterionParameters, Context};
Expand Down Expand Up @@ -29,7 +30,7 @@ impl<'t> Final<'t> {
}

#[logging_timer::time("Final::{}")]
pub fn next(&mut self, excluded_candidates: &RoaringBitmap) -> anyhow::Result<Option<FinalResult>> {
pub fn next(&mut self, excluded_candidates: &RoaringBitmap) -> Result<Option<FinalResult>> {
debug!("Final iteration");
let excluded_candidates = &self.returned_candidates | excluded_candidates;
let mut criterion_parameters = CriterionParameters {
Expand Down
Loading

0 comments on commit 312c2d1

Please sign in to comment.