From 1c6d9bdc6a98faccd70e1e69b00d3a885d80ae24 Mon Sep 17 00:00:00 2001
From: Paul Masurel <paul@quickwit.io>
Date: Fri, 7 Jan 2022 20:31:26 +0900
Subject: [PATCH] Comparison of Value based on serialization. (#1250)

---
 src/schema/document.rs    | 21 ++++++++++++++++----
 src/schema/field_value.rs |  2 +-
 src/schema/value.rs       | 42 +--------------------------------------
 3 files changed, 19 insertions(+), 46 deletions(-)

diff --git a/src/schema/document.rs b/src/schema/document.rs
index b69f706c4..ecf9eec38 100644
--- a/src/schema/document.rs
+++ b/src/schema/document.rs
@@ -3,6 +3,8 @@ use crate::tokenizer::PreTokenizedString;
 use crate::DateTime;
 use common::BinarySerializable;
 use common::VInt;
+use std::collections::HashMap;
+use std::collections::HashSet;
 use std::io::{self, Read, Write};
 use std::mem;
 
@@ -30,10 +32,21 @@ impl From<Vec<FieldValue>> for Document {
 impl PartialEq for Document {
     fn eq(&self, other: &Document) -> bool {
         // super slow, but only here for tests
-        let mut self_field_values: Vec<&_> = self.field_values.iter().collect();
-        let mut other_field_values: Vec<&_> = other.field_values.iter().collect();
-        self_field_values.sort();
-        other_field_values.sort();
+        let convert_to_comparable_map = |field_values: &[FieldValue]| {
+            let mut field_value_set: HashMap<Field, HashSet<String>> = Default::default();
+            for field_value in field_values.iter() {
+                let json_val = serde_json::to_string(field_value.value()).unwrap();
+                field_value_set
+                    .entry(field_value.field())
+                    .or_default()
+                    .insert(json_val);
+            }
+            field_value_set
+        };
+        let self_field_values: HashMap<Field, HashSet<String>> =
+            convert_to_comparable_map(&self.field_values);
+        let other_field_values: HashMap<Field, HashSet<String>> =
+            convert_to_comparable_map(&other.field_values);
         self_field_values.eq(&other_field_values)
     }
 }
diff --git a/src/schema/field_value.rs b/src/schema/field_value.rs
index 4dbe15afb..80160afca 100644
--- a/src/schema/field_value.rs
+++ b/src/schema/field_value.rs
@@ -4,7 +4,7 @@ use common::BinarySerializable;
 use std::io::{self, Read, Write};
 
 /// `FieldValue` holds together a `Field` and its `Value`.
-#[derive(Debug, Clone, Ord, PartialEq, Eq, PartialOrd, serde::Serialize, serde::Deserialize)]
+#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
 pub struct FieldValue {
     field: Field,
     value: Value,
diff --git a/src/schema/value.rs b/src/schema/value.rs
index 73f643a89..add1995d4 100644
--- a/src/schema/value.rs
+++ b/src/schema/value.rs
@@ -3,7 +3,7 @@ use crate::tokenizer::PreTokenizedString;
 use crate::DateTime;
 use serde::de::Visitor;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
-use std::{cmp::Ordering, fmt};
+use std::fmt;
 
 /// Value represents the value of a any field.
 /// It is an enum over all over all of the possible field type.
@@ -28,46 +28,6 @@ pub enum Value {
 }
 
 impl Eq for Value {}
-impl PartialOrd for Value {
-    fn partial_cmp(&self, other: &Value) -> Option<Ordering> {
-        Some(self.cmp(other))
-    }
-}
-impl Ord for Value {
-    fn cmp(&self, other: &Self) -> Ordering {
-        match (self, other) {
-            (Value::Str(l), Value::Str(r)) => l.cmp(r),
-            (Value::PreTokStr(l), Value::PreTokStr(r)) => l.cmp(r),
-            (Value::U64(l), Value::U64(r)) => l.cmp(r),
-            (Value::I64(l), Value::I64(r)) => l.cmp(r),
-            (Value::Date(l), Value::Date(r)) => l.cmp(r),
-            (Value::Facet(l), Value::Facet(r)) => l.cmp(r),
-            (Value::Bytes(l), Value::Bytes(r)) => l.cmp(r),
-            (Value::F64(l), Value::F64(r)) => {
-                match (l.is_nan(), r.is_nan()) {
-                    (false, false) => l.partial_cmp(r).unwrap(), // only fail on NaN
-                    (true, true) => Ordering::Equal,
-                    (true, false) => Ordering::Less, // we define NaN as less than -∞
-                    (false, true) => Ordering::Greater,
-                }
-            }
-            (Value::Str(_), _) => Ordering::Less,
-            (_, Value::Str(_)) => Ordering::Greater,
-            (Value::PreTokStr(_), _) => Ordering::Less,
-            (_, Value::PreTokStr(_)) => Ordering::Greater,
-            (Value::U64(_), _) => Ordering::Less,
-            (_, Value::U64(_)) => Ordering::Greater,
-            (Value::I64(_), _) => Ordering::Less,
-            (_, Value::I64(_)) => Ordering::Greater,
-            (Value::F64(_), _) => Ordering::Less,
-            (_, Value::F64(_)) => Ordering::Greater,
-            (Value::Date(_), _) => Ordering::Less,
-            (_, Value::Date(_)) => Ordering::Greater,
-            (Value::Facet(_), _) => Ordering::Less,
-            (_, Value::Facet(_)) => Ordering::Greater,
-        }
-    }
-}
 
 impl Serialize for Value {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>