diff --git a/milli/src/search/criteria/asc_desc.rs b/milli/src/search/criteria/asc_desc.rs index 193e9c942..50bb6798b 100644 --- a/milli/src/search/criteria/asc_desc.rs +++ b/milli/src/search/criteria/asc_desc.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use std::mem::take; -use anyhow::bail; +use anyhow::{bail, Context as _}; use itertools::Itertools; use log::debug; use ordered_float::OrderedFloat; @@ -13,12 +13,13 @@ use crate::heed_codec::facet::{FieldDocIdFacetI64Codec, FieldDocIdFacetF64Codec} 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::{FieldsIdsMap, FieldId, Index}; use super::{Criterion, CriterionResult}; pub struct AscDesc<'t> { index: &'t Index, rtxn: &'t heed::RoTxn<'t>, + field_name: String, field_id: FieldId, facet_type: FacetType, ascending: bool, @@ -35,11 +36,10 @@ impl<'t> AscDesc<'t> { rtxn: &'t heed::RoTxn, query_tree: Option, candidates: Option, - field_id: FieldId, - facet_type: FacetType, + field_name: String, ) -> anyhow::Result { - Self::initial(index, rtxn, query_tree, candidates, field_id, facet_type, true) + Self::initial(index, rtxn, query_tree, candidates, field_name, true) } pub fn initial_desc( @@ -47,33 +47,30 @@ impl<'t> AscDesc<'t> { rtxn: &'t heed::RoTxn, query_tree: Option, candidates: Option, - field_id: FieldId, - facet_type: FacetType, + field_name: String, ) -> anyhow::Result { - Self::initial(index, rtxn, query_tree, candidates, field_id, facet_type, false) + Self::initial(index, rtxn, query_tree, candidates, field_name, false) } pub fn asc( index: &'t Index, rtxn: &'t heed::RoTxn, parent: Box, - field_id: FieldId, - facet_type: FacetType, + field_name: String, ) -> anyhow::Result { - Self::new(index, rtxn, parent, field_id, facet_type, true) + Self::new(index, rtxn, parent, field_name, true) } pub fn desc( index: &'t Index, rtxn: &'t heed::RoTxn, parent: Box, - field_id: FieldId, - facet_type: FacetType, + field_name: String, ) -> anyhow::Result { - Self::new(index, rtxn, parent, field_id, facet_type, false) + Self::new(index, rtxn, parent, field_name, false) } fn initial( @@ -81,11 +78,14 @@ impl<'t> AscDesc<'t> { rtxn: &'t heed::RoTxn, query_tree: Option, candidates: Option, - field_id: FieldId, - facet_type: FacetType, + field_name: String, ascending: bool, ) -> anyhow::Result { + let fields_ids_map = index.fields_ids_map(rtxn)?; + let faceted_fields = index.faceted_fields(rtxn)?; + let (field_id, facet_type) = field_id_facet_type(&fields_ids_map, &faceted_fields, &field_name)?; + let faceted_candidates = index.faceted_documents_ids(rtxn, field_id)?; let candidates = match &query_tree { Some(qt) => { @@ -102,6 +102,7 @@ impl<'t> AscDesc<'t> { Ok(AscDesc { index, rtxn, + field_name, field_id, facet_type, ascending, @@ -117,14 +118,18 @@ impl<'t> AscDesc<'t> { index: &'t Index, rtxn: &'t heed::RoTxn, parent: Box, - field_id: FieldId, - facet_type: FacetType, + field_name: String, ascending: bool, ) -> anyhow::Result { + let fields_ids_map = index.fields_ids_map(rtxn)?; + let faceted_fields = index.faceted_fields(rtxn)?; + let (field_id, facet_type) = field_id_facet_type(&fields_ids_map, &faceted_fields, &field_name)?; + Ok(AscDesc { index, rtxn, + field_name, field_id, facet_type, ascending, @@ -140,8 +145,8 @@ impl<'t> AscDesc<'t> { impl<'t> Criterion for AscDesc<'t> { fn next(&mut self) -> anyhow::Result> { loop { - debug!("Facet {} iteration ({:?})", - if self.ascending { "Asc" } else { "Desc" }, self.candidates, + debug!("Facet {}({}) iteration ({:?})", + if self.ascending { "Asc" } else { "Desc" }, self.field_name, self.candidates, ); match &mut self.candidates { @@ -197,6 +202,21 @@ impl<'t> Criterion for AscDesc<'t> { } } +fn field_id_facet_type( + fields_ids_map: &FieldsIdsMap, + faceted_fields: &HashMap, + field: &str, +) -> anyhow::Result<(FieldId, FacetType)> +{ + let id = fields_ids_map.id(field).with_context(|| { + format!("field {:?} isn't registered", field) + })?; + let facet_type = faceted_fields.get(field).with_context(|| { + format!("field {:?} isn't faceted", field) + })?; + Ok((id, *facet_type)) +} + fn facet_ordered( index: &Index, rtxn: &heed::RoTxn, diff --git a/milli/src/search/criteria/mod.rs b/milli/src/search/criteria/mod.rs index 0dcaa5a69..b1119e221 100644 --- a/milli/src/search/criteria/mod.rs +++ b/milli/src/search/criteria/mod.rs @@ -122,18 +122,6 @@ impl<'t> CriteriaBuilder<'t> { { use crate::criterion::Criterion as Name; - let fields_ids_map = self.index.fields_ids_map(&self.rtxn)?; - let faceted_fields = self.index.faceted_fields(&self.rtxn)?; - let field_id_facet_type = |field: &str| -> anyhow::Result<(FieldId, FacetType)> { - let id = fields_ids_map.id(field).with_context(|| { - format!("field {:?} isn't registered", field) - })?; - let facet_type = faceted_fields.get(field).with_context(|| { - format!("field {:?} isn't faceted", field) - })?; - Ok((id, *facet_type)) - }; - let mut criterion = None as Option>; for name in self.index.criteria(&self.rtxn)? { criterion = Some(match criterion.take() { @@ -141,14 +129,8 @@ impl<'t> CriteriaBuilder<'t> { Name::Typo => Box::new(Typo::new(self, father)), Name::Words => Box::new(Words::new(self, father)), Name::Proximity => Box::new(Proximity::new(self, father)), - Name::Asc(field) => { - let (id, facet_type) = field_id_facet_type(&field)?; - Box::new(AscDesc::asc(&self.index, &self.rtxn, father, id, facet_type)?) - }, - Name::Desc(field) => { - let (id, facet_type) = field_id_facet_type(&field)?; - Box::new(AscDesc::desc(&self.index, &self.rtxn, father, id, facet_type)?) - }, + Name::Asc(field) => Box::new(AscDesc::asc(&self.index, &self.rtxn, father, field)?), + Name::Desc(field) => Box::new(AscDesc::desc(&self.index, &self.rtxn, father, field)?), _otherwise => father, }, None => match name { @@ -156,12 +138,10 @@ impl<'t> CriteriaBuilder<'t> { Name::Words => Box::new(Words::initial(self, query_tree.take(), facet_candidates.take())), Name::Proximity => Box::new(Proximity::initial(self, query_tree.take(), facet_candidates.take())), Name::Asc(field) => { - let (id, facet_type) = field_id_facet_type(&field)?; - Box::new(AscDesc::initial_asc(&self.index, &self.rtxn, query_tree.take(), facet_candidates.take(), id, facet_type)?) + Box::new(AscDesc::initial_asc(&self.index, &self.rtxn, query_tree.take(), facet_candidates.take(), field)?) }, Name::Desc(field) => { - let (id, facet_type) = field_id_facet_type(&field)?; - Box::new(AscDesc::initial_desc(&self.index, &self.rtxn, query_tree.take(), facet_candidates.take(), id, facet_type)?) + Box::new(AscDesc::initial_desc(&self.index, &self.rtxn, query_tree.take(), facet_candidates.take(), field)?) }, _otherwise => continue, },