From b0c0490e857be4e9cd36ad74514e000e6c50158f Mon Sep 17 00:00:00 2001
From: Kerollmops <clement@meilisearch.com>
Date: Tue, 1 Jun 2021 15:48:38 +0200
Subject: [PATCH] Make sure that we can add a Asc/Desc field without it being
 filterable

---
 milli/src/criterion.rs       | 11 +++++------
 milli/src/update/settings.rs | 25 ++++++++++++++++++++++---
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/milli/src/criterion.rs b/milli/src/criterion.rs
index 1d7326db7..81a2878b3 100644
--- a/milli/src/criterion.rs
+++ b/milli/src/criterion.rs
@@ -1,5 +1,5 @@
-use std::collections::HashSet;
 use std::fmt;
+use std::str::FromStr;
 
 use anyhow::{Context, bail};
 use regex::Regex;
@@ -30,8 +30,10 @@ pub enum Criterion {
     Desc(String),
 }
 
-impl Criterion {
-    pub fn from_str(faceted_attributes: &HashSet<String>, txt: &str) -> anyhow::Result<Criterion> {
+impl FromStr for Criterion {
+    type Err = anyhow::Error;
+
+    fn from_str(txt: &str) -> Result<Criterion, Self::Err> {
         match txt {
             "words" => Ok(Criterion::Words),
             "typo" => Ok(Criterion::Typo),
@@ -42,9 +44,6 @@ impl Criterion {
                 let caps = ASC_DESC_REGEX.captures(text).with_context(|| format!("unknown criterion name: {}", text))?;
                 let order = caps.get(1).unwrap().as_str();
                 let field_name = caps.get(2).unwrap().as_str();
-                faceted_attributes.get(field_name).with_context(|| {
-                    format!("Can't use {:?} as a criterion as it isn't a faceted field.", field_name)
-                })?;
                 match order {
                     "asc" => Ok(Criterion::Asc(field_name.to_string())),
                     "desc" => Ok(Criterion::Desc(field_name.to_string())),
diff --git a/milli/src/update/settings.rs b/milli/src/update/settings.rs
index 10b6b8cbe..eba14a17c 100644
--- a/milli/src/update/settings.rs
+++ b/milli/src/update/settings.rs
@@ -9,7 +9,6 @@ use rayon::ThreadPool;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
 use crate::{FieldsIdsMap, Index};
-use crate::criterion::Criterion;
 use crate::update::{ClearDocuments, IndexDocuments, UpdateIndexingStep};
 use crate::update::index_documents::{IndexDocumentsMethod, Transform};
 
@@ -402,10 +401,9 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> {
     fn update_criteria(&mut self) -> anyhow::Result<()> {
         match self.criteria {
             Setting::Set(ref fields) => {
-                let filterable_fields = self.index.filterable_fields(&self.wtxn)?;
                 let mut new_criteria = Vec::new();
                 for name in fields {
-                    let criterion = Criterion::from_str(&filterable_fields, &name)?;
+                    let criterion = name.parse()?;
                     new_criteria.push(criterion);
                 }
                 self.index.put_criteria(self.wtxn, &new_criteria)?;
@@ -446,6 +444,7 @@ mod tests {
     use maplit::{btreeset, hashmap, hashset};
     use big_s::S;
 
+    use crate::{Criterion, FilterCondition};
     use crate::update::{IndexDocuments, UpdateFormat};
 
     use super::*;
@@ -858,4 +857,24 @@ mod tests {
         assert!(index.primary_key(&rtxn).unwrap().is_none());
         assert_eq!(vec![Criterion::Asc("toto".to_string())], index.criteria(&rtxn).unwrap());
     }
+
+    #[test]
+    fn setting_not_filterable_cant_filter() {
+        let path = tempfile::tempdir().unwrap();
+        let mut options = EnvOpenOptions::new();
+        options.map_size(10 * 1024 * 1024); // 10 MB
+        let index = Index::new(options, &path).unwrap();
+
+        // Set all the settings except searchable
+        let mut wtxn = index.write_txn().unwrap();
+        let mut builder = Settings::new(&mut wtxn, &index, 0);
+        builder.set_displayed_fields(vec!["hello".to_string()]);
+        // It is only Asc(toto), there is a facet database but it is denied to filter with toto.
+        builder.set_criteria(vec!["asc(toto)".to_string()]);
+        builder.execute(|_, _| ()).unwrap();
+        wtxn.commit().unwrap();
+
+        let rtxn = index.read_txn().unwrap();
+        FilterCondition::from_str(&rtxn, &index, "toto = 32").unwrap_err();
+    }
 }