Move BTreeMap to IndexMap to preserve order (#2617)

crates/nu-data/src/base/shape.rs

@@ -1,21 +1,23 @@
 use bigdecimal::BigDecimal;
 use chrono::{DateTime, Utc};
+use indexmap::map::IndexMap;
 use nu_protocol::RangeInclusion;
 use nu_protocol::{format_primitive, ColumnPath, Dictionary, Primitive, UntaggedValue, Value};
 use nu_source::{b, DebugDocBuilder, PrettyDebug};
 use num_bigint::BigInt;
-use std::collections::BTreeMap;
+use serde::{Deserialize, Serialize};
+use std::cmp::Ordering;
 use std::fmt::Debug;
-use std::hash::Hash;
+use std::hash::{Hash, Hasher};
 use std::path::PathBuf;
 
-#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
+#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Deserialize, Serialize)]
 pub struct InlineRange {
     from: (InlineShape, RangeInclusion),
     to: (InlineShape, RangeInclusion),
 }
 
-#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
+#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Deserialize, Serialize)]
 pub enum InlineShape {
     Nothing,
     Int(BigInt),
@@ -32,7 +34,7 @@ pub enum InlineShape {
     Path(PathBuf),
     Binary(usize),
 
-    Row(BTreeMap<Column, InlineShape>),
+    Row(Row),
     Table(Vec<InlineShape>),
 
     // TODO: Block arguments
@@ -81,14 +83,14 @@ impl InlineShape {
     }
 
     pub fn from_dictionary(dictionary: &Dictionary) -> InlineShape {
-        let mut map = BTreeMap::new();
+        let mut map = IndexMap::new();
 
         for (key, value) in dictionary.entries.iter() {
             let column = Column::String(key.clone());
             map.insert(column, InlineShape::from_value(value));
         }
 
-        InlineShape::Row(map)
+        InlineShape::Row(Row { map })
     }
 
     pub fn from_table<'a>(table: impl IntoIterator<Item = &'a Value>) -> InlineShape {
@@ -193,16 +195,16 @@ impl PrettyDebug for FormatInlineShape {
                 "[",
                 b::kind("row")
                     + b::space()
-                    + if row.keys().len() <= 6 {
+                    + if row.map.keys().len() <= 6 {
                         b::intersperse(
-                            row.keys().map(|key| match key {
+                            row.map.keys().map(|key| match key {
                                 Column::String(string) => b::description(string),
                                 Column::Value => b::blank(),
                             }),
                             b::space(),
                         )
                     } else {
-                        b::description(format!("{} columns", row.keys().len()))
+                        b::description(format!("{} columns", row.map.keys().len()))
                     },
                 "]",
             )
@@ -250,7 +252,7 @@ impl GroupedValue for Vec<(usize, usize)> {
     }
 }
 
-#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
+#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Deserialize, Serialize)]
 pub enum Column {
     String(String),
     Value,
@@ -273,3 +275,52 @@ impl Into<Column> for &str {
         Column::String(self.to_string())
     }
 }
+
+/// A shape representation of the type of a row
+#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
+pub struct Row {
+    map: IndexMap<Column, InlineShape>,
+}
+
+#[allow(clippy::derive_hash_xor_eq)]
+impl Hash for Row {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        let mut entries = self.map.clone();
+        entries.sort_keys();
+        entries.keys().collect::<Vec<&Column>>().hash(state);
+        entries.values().collect::<Vec<&InlineShape>>().hash(state);
+    }
+}
+
+impl PartialOrd for Row {
+    fn partial_cmp(&self, other: &Row) -> Option<Ordering> {
+        let this: Vec<&Column> = self.map.keys().collect();
+        let that: Vec<&Column> = other.map.keys().collect();
+
+        if this != that {
+            return this.partial_cmp(&that);
+        }
+
+        let this: Vec<&InlineShape> = self.map.values().collect();
+        let that: Vec<&InlineShape> = self.map.values().collect();
+
+        this.partial_cmp(&that)
+    }
+}
+
+impl Ord for Row {
+    /// Compare two dictionaries for ordering
+    fn cmp(&self, other: &Row) -> Ordering {
+        let this: Vec<&Column> = self.map.keys().collect();
+        let that: Vec<&Column> = other.map.keys().collect();
+
+        if this != that {
+            return this.cmp(&that);
+        }
+
+        let this: Vec<&InlineShape> = self.map.values().collect();
+        let that: Vec<&InlineShape> = self.map.values().collect();
+
+        this.cmp(&that)
+    }
+}

crates/nu-protocol/src/type_shape.rs

@@ -9,11 +9,12 @@ use crate::value::primitive::Primitive;
 use crate::value::range::RangeInclusion;
 use crate::value::{UntaggedValue, Value};
 use derive_new::new;
+use indexmap::map::IndexMap;
 use nu_source::{b, DebugDoc, DebugDocBuilder, PrettyDebug};
-use serde::{Deserialize, Deserializer, Serialize};
+use serde::{Deserialize, Serialize};
-use std::collections::BTreeMap;
+use std::cmp::Ordering;
 use std::fmt::Debug;
-use std::hash::Hash;
+use std::hash::{Hash, Hasher};
 
 /// Representation of the type of ranges
 #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize, new)]
@@ -71,53 +72,52 @@ pub enum Type {
 }
 
 /// A shape representation of the type of a row
-#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, new)]
+#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
 pub struct Row {
-    #[new(default)]
+    map: IndexMap<Column, Type>,
-    map: BTreeMap<Column, Type>,
 }
 
-impl Serialize for Row {
+#[allow(clippy::derive_hash_xor_eq)]
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+impl Hash for Row {
-    where
+    fn hash<H: Hasher>(&self, state: &mut H) {
-        S: serde::Serializer,
+        let mut entries = self.map.clone();
-    {
+        entries.sort_keys();
-        serializer.collect_map(self.map.iter())
+        entries.keys().collect::<Vec<&Column>>().hash(state);
+        entries.values().collect::<Vec<&Type>>().hash(state);
     }
 }
 
-impl<'de> Deserialize<'de> for Row {
+impl PartialOrd for Row {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    /// Compare two dictionaries for sort ordering
-    where
+    fn partial_cmp(&self, other: &Row) -> Option<Ordering> {
-        D: Deserializer<'de>,
+        let this: Vec<&Column> = self.map.keys().collect();
-    {
+        let that: Vec<&Column> = other.map.keys().collect();
-        struct RowVisitor;
 
-        impl<'de> serde::de::Visitor<'de> for RowVisitor {
+        if this != that {
-            type Value = Row;
+            return this.partial_cmp(&that);
-            fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
-                write!(formatter, "a row")
-            }
-
-            fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
-            where
-                A: serde::de::MapAccess<'de>,
-            {
-                let mut new_map = BTreeMap::new();
-
-                loop {
-                    let entry = map.next_entry()?;
-
-                    match entry {
-                        None => return Ok(Row { map: new_map }),
-                        Some((key, value)) => {
-                            new_map.insert(key, value);
-                        }
-                    }
-                }
-            }
         }
-        deserializer.deserialize_map(RowVisitor)
+
+        let this: Vec<&Type> = self.map.values().collect();
+        let that: Vec<&Type> = self.map.values().collect();
+
+        this.partial_cmp(&that)
+    }
+}
+
+impl Ord for Row {
+    /// Compare two dictionaries for ordering
+    fn cmp(&self, other: &Row) -> Ordering {
+        let this: Vec<&Column> = self.map.keys().collect();
+        let that: Vec<&Column> = other.map.keys().collect();
+
+        if this != that {
+            return this.cmp(&that);
+        }
+
+        let this: Vec<&Type> = self.map.values().collect();
+        let that: Vec<&Type> = self.map.values().collect();
+
+        this.cmp(&that)
     }
 }
 
@@ -155,7 +155,7 @@ impl Type {
 
     /// Convert a dictionary into its corresponding row Type
     pub fn from_dictionary(dictionary: &Dictionary) -> Type {
-        let mut map = BTreeMap::new();
+        let mut map = IndexMap::new();
 
         for (key, value) in dictionary.entries.iter() {
             let column = Column::String(key.clone());