Merge pull request #314 from meilisearch/fix-number-ord

Fix the ordering functions of the Number type

meilidb-core/src/database.rs

@@ -277,8 +277,9 @@ impl Database {
 mod tests {
     use super::*;
 
+    use crate::criterion::{self, CriteriaBuilder};
     use crate::update::{ProcessedUpdateResult, UpdateStatus};
-    use crate::DocumentId;
+    use crate::{Document, DocumentId};
     use serde::de::IgnoredAny;
     use std::sync::mpsc;
 
@@ -912,4 +913,99 @@ mod tests {
         let result = database.open_index("test");
         assert!(result.is_none());
     }
+
+    #[test]
+    fn check_number_ordering() {
+        let dir = tempfile::tempdir().unwrap();
+
+        let database = Database::open_or_create(dir.path()).unwrap();
+        let env = &database.env;
+
+        let (sender, receiver) = mpsc::sync_channel(100);
+        let update_fn = move |_name: &str, update: ProcessedUpdateResult| {
+            sender.send(update.update_id).unwrap()
+        };
+        let index = database.create_index("test").unwrap();
+
+        database.set_update_callback(Box::new(update_fn));
+
+        let schema = {
+            let data = r#"
+                identifier = "id"
+
+                [attributes."name"]
+                displayed = true
+                indexed = true
+
+                [attributes."release_date"]
+                displayed = true
+                ranked = true
+            "#;
+            toml::from_str(data).unwrap()
+        };
+
+        let mut writer = env.write_txn().unwrap();
+        let _update_id = index.schema_update(&mut writer, schema).unwrap();
+        writer.commit().unwrap();
+
+        let mut additions = index.documents_addition();
+
+        // DocumentId(7900334843754999545)
+        let doc1 = serde_json::json!({
+            "id": 123,
+            "name": "Kevin the first",
+            "release_date": -10000,
+        });
+
+        // DocumentId(8367468610878465872)
+        let doc2 = serde_json::json!({
+            "id": 234,
+            "name": "Kevin the second",
+            "release_date": 10000,
+        });
+
+        additions.update_document(doc1);
+        additions.update_document(doc2);
+
+        let mut writer = env.write_txn().unwrap();
+        let update_id = additions.finalize(&mut writer).unwrap();
+        writer.commit().unwrap();
+
+        // block until the transaction is processed
+        let _ = receiver.into_iter().find(|id| *id == update_id);
+
+        let reader = env.read_txn().unwrap();
+
+        let schema = index.main.schema(&reader).unwrap().unwrap();
+        let ranked_map = index.main.ranked_map(&reader).unwrap().unwrap();
+
+        let criteria = CriteriaBuilder::new()
+            .add(
+                criterion::SortByAttr::lower_is_better(&ranked_map, &schema, "release_date")
+                    .unwrap(),
+            )
+            .add(criterion::DocumentId)
+            .build();
+
+        let builder = index.query_builder_with_criteria(criteria);
+
+        let results = builder.query(&reader, "Kevin", 0..20).unwrap();
+        let mut iter = results.into_iter();
+
+        assert_matches!(
+            iter.next(),
+            Some(Document {
+                id: DocumentId(7900334843754999545),
+                ..
+            })
+        );
+        assert_matches!(
+            iter.next(),
+            Some(Document {
+                id: DocumentId(8367468610878465872),
+                ..
+            })
+        );
+        assert_matches!(iter.next(), None);
+    }
 }

meilidb-core/src/number.rs

@@ -1,3 +1,4 @@
+use std::cmp::Ordering;
 use std::fmt;
 use std::num::{ParseFloatError, ParseIntError};
 use std::str::FromStr;
@@ -5,7 +6,7 @@ use std::str::FromStr;
 use ordered_float::OrderedFloat;
 use serde::{Deserialize, Serialize};
 
-#[derive(Serialize, Deserialize, Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Serialize, Deserialize, Debug, Copy, Clone, Hash)]
 pub enum Number {
     Unsigned(u64),
     Signed(i64),
@@ -39,6 +40,50 @@ impl FromStr for Number {
     }
 }
 
+impl PartialEq for Number {
+    fn eq(&self, other: &Number) -> bool {
+        self.cmp(other) == Ordering::Equal
+    }
+}
+
+impl Eq for Number {}
+
+impl PartialOrd for Number {
+    fn partial_cmp(&self, other: &Number) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for Number {
+    fn cmp(&self, other: &Self) -> Ordering {
+        use Number::{Float, Signed, Unsigned};
+
+        match (*self, *other) {
+            (Unsigned(a), Unsigned(b)) => a.cmp(&b),
+            (Unsigned(a), Signed(b)) => {
+                if b < 0 {
+                    Ordering::Greater
+                } else {
+                    a.cmp(&(b as u64))
+                }
+            }
+            (Unsigned(a), Float(b)) => (OrderedFloat(a as f64)).cmp(&b),
+            (Signed(a), Unsigned(b)) => {
+                if a < 0 {
+                    Ordering::Less
+                } else {
+                    (a as u64).cmp(&b)
+                }
+            }
+            (Signed(a), Signed(b)) => a.cmp(&b),
+            (Signed(a), Float(b)) => OrderedFloat(a as f64).cmp(&b),
+            (Float(a), Unsigned(b)) => a.cmp(&OrderedFloat(b as f64)),
+            (Float(a), Signed(b)) => a.cmp(&OrderedFloat(b as f64)),
+            (Float(a), Float(b)) => a.cmp(&b),
+        }
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ParseNumberError {
     uint_error: ParseIntError,