596: Filter operators: NOT + IN[..] r=irevoire a=loiclec

# Pull Request

## What does this PR do?
Implements the changes described in https://github.com/meilisearch/meilisearch/issues/2580
It is based on top of #556 

Co-authored-by: Loïc Lecrenier <loic@meilisearch.com>
This commit is contained in:
bors[bot] 2022-08-18 11:24:32 +00:00 committed by GitHub
commit afc10acd19
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 686 additions and 549 deletions

View file

@ -89,52 +89,44 @@ impl<'a> Filter<'a> {
I: IntoIterator<Item = Either<J, &'a str>>,
J: IntoIterator<Item = &'a str>,
{
let mut ands: Option<FilterCondition> = None;
let mut ands = vec![];
for either in array {
match either {
Either::Left(array) => {
let mut ors = None;
let mut ors = vec![];
for rule in array {
if let Some(filter) = Self::from_str(rule.as_ref())? {
let condition = filter.condition;
ors = match ors.take() {
Some(ors) => {
Some(FilterCondition::Or(Box::new(ors), Box::new(condition)))
}
None => Some(condition),
};
ors.push(filter.condition);
}
}
if let Some(rule) = ors {
ands = match ands.take() {
Some(ands) => {
Some(FilterCondition::And(Box::new(ands), Box::new(rule)))
}
None => Some(rule),
};
if ors.len() > 1 {
ands.push(FilterCondition::Or(ors));
} else if ors.len() == 1 {
ands.push(ors.pop().unwrap());
}
}
Either::Right(rule) => {
if let Some(filter) = Self::from_str(rule.as_ref())? {
let condition = filter.condition;
ands = match ands.take() {
Some(ands) => {
Some(FilterCondition::And(Box::new(ands), Box::new(condition)))
}
None => Some(condition),
};
ands.push(filter.condition);
}
}
}
}
let and = if ands.is_empty() {
return Ok(None);
} else if ands.len() == 1 {
ands.pop().unwrap()
} else {
FilterCondition::And(ands)
};
if let Some(token) = ands.as_ref().and_then(|fc| fc.token_at_depth(MAX_FILTER_DEPTH)) {
if let Some(token) = and.token_at_depth(MAX_FILTER_DEPTH) {
return Err(token.as_external_error(FilterError::TooDeep).into());
}
Ok(ands.map(|ands| Self { condition: ands }))
Ok(Some(Self { condition: and }))
}
pub fn from_str(expression: &'a str) -> Result<Option<Self>> {
@ -284,14 +276,6 @@ impl<'a> Filter<'a> {
let exist = index.exists_faceted_documents_ids(rtxn, field_id)?;
return Ok(exist);
}
Condition::NotExists => {
let all_ids = index.documents_ids(rtxn)?;
let exist = Self::evaluate_operator(rtxn, index, field_id, &Condition::Exists)?;
let notexist = all_ids - exist;
return Ok(notexist);
}
Condition::Equal(val) => {
let (_original_value, string_docids) = strings_db
.get(rtxn, &(field_id, &val.value().to_lowercase()))?
@ -317,11 +301,10 @@ impl<'a> Filter<'a> {
return Ok(string_docids | number_docids);
}
Condition::NotEqual(val) => {
let all_numbers_ids = index.number_faceted_documents_ids(rtxn, field_id)?;
let all_strings_ids = index.string_faceted_documents_ids(rtxn, field_id)?;
let operator = Condition::Equal(val.clone());
let docids = Self::evaluate_operator(rtxn, index, field_id, &operator)?;
return Ok((all_numbers_ids | all_strings_ids) - docids);
let all_ids = index.documents_ids(rtxn)?;
return Ok(all_ids - docids);
}
};
@ -367,6 +350,39 @@ impl<'a> Filter<'a> {
filterable_fields: &HashSet<String>,
) -> Result<RoaringBitmap> {
match &self.condition {
FilterCondition::Not(f) => {
let all_ids = index.documents_ids(rtxn)?;
let selected = Self::inner_evaluate(
&(f.as_ref().clone()).into(),
rtxn,
index,
filterable_fields,
)?;
return Ok(all_ids - selected);
}
FilterCondition::In { fid, els } => {
if crate::is_faceted(fid.value(), filterable_fields) {
let field_ids_map = index.fields_ids_map(rtxn)?;
if let Some(fid) = field_ids_map.id(fid.value()) {
let mut bitmap = RoaringBitmap::new();
for el in els {
let op = Condition::Equal(el.clone());
let el_bitmap = Self::evaluate_operator(rtxn, index, fid, &op)?;
bitmap |= el_bitmap;
}
Ok(bitmap)
} else {
Ok(RoaringBitmap::new())
}
} else {
return Err(fid.as_external_error(FilterError::AttributeNotFilterable {
attribute: fid.value(),
filterable_fields: filterable_fields.clone(),
}))?;
}
}
FilterCondition::Condition { fid, op } => {
if crate::is_faceted(fid.value(), filterable_fields) {
let field_ids_map = index.fields_ids_map(rtxn)?;
@ -397,38 +413,38 @@ impl<'a> Filter<'a> {
}
}
}
FilterCondition::Or(lhs, rhs) => {
let lhs = Self::inner_evaluate(
&(lhs.as_ref().clone()).into(),
rtxn,
index,
filterable_fields,
)?;
let rhs = Self::inner_evaluate(
&(rhs.as_ref().clone()).into(),
rtxn,
index,
filterable_fields,
)?;
Ok(lhs | rhs)
}
FilterCondition::And(lhs, rhs) => {
let lhs = Self::inner_evaluate(
&(lhs.as_ref().clone()).into(),
rtxn,
index,
filterable_fields,
)?;
if lhs.is_empty() {
return Ok(lhs);
FilterCondition::Or(subfilters) => {
let mut bitmap = RoaringBitmap::new();
for f in subfilters {
bitmap |=
Self::inner_evaluate(&(f.clone()).into(), rtxn, index, filterable_fields)?;
}
Ok(bitmap)
}
FilterCondition::And(subfilters) => {
let mut subfilters_iter = subfilters.iter();
if let Some(first_subfilter) = subfilters_iter.next() {
let mut bitmap = Self::inner_evaluate(
&(first_subfilter.clone()).into(),
rtxn,
index,
filterable_fields,
)?;
for f in subfilters_iter {
if bitmap.is_empty() {
return Ok(bitmap);
}
bitmap &= Self::inner_evaluate(
&(f.clone()).into(),
rtxn,
index,
filterable_fields,
)?;
}
Ok(bitmap)
} else {
Ok(RoaringBitmap::new())
}
let rhs = Self::inner_evaluate(
&(rhs.as_ref().clone()).into(),
rtxn,
index,
filterable_fields,
)?;
Ok(lhs & rhs)
}
FilterCondition::GeoLowerThan { point, radius } => {
if filterable_fields.contains("_geo") {
@ -467,17 +483,6 @@ impl<'a> Filter<'a> {
}))?;
}
}
FilterCondition::GeoGreaterThan { point, radius } => {
let result = Self::inner_evaluate(
&FilterCondition::GeoLowerThan { point: point.clone(), radius: radius.clone() }
.into(),
rtxn,
index,
filterable_fields,
)?;
let geo_faceted_doc_ids = index.geo_faceted_documents_ids(rtxn)?;
Ok(geo_faceted_doc_ids - result)
}
}
}
}
@ -732,12 +737,10 @@ mod tests {
}
}
let error = Filter::from_str(&filter_string).unwrap_err();
assert!(
error.to_string().starts_with("Too many filter conditions"),
"{}",
error.to_string()
);
// Note: the filter used to be rejected for being too deep, but that is
// no longer the case
let filter = Filter::from_str(&filter_string).unwrap();
assert!(filter.is_some());
}
#[test]