From 892a416211511b3ac7e988c24bd40df7ea99f511 Mon Sep 17 00:00:00 2001
From: Chris Gillespie <6572184+gillespiecd@users.noreply.github.com>
Date: Tue, 29 Sep 2020 23:49:40 -0700
Subject: [PATCH] Move BTreeMap to IndexMap to preserve order (#2617)
---
crates/nu-data/src/base/shape.rs | 73 +++++++++++++++++++----
crates/nu-protocol/src/type_shape.rs | 86 ++++++++++++++--------------
2 files changed, 105 insertions(+), 54 deletions(-)
diff --git a/crates/nu-data/src/base/shape.rs b/crates/nu-data/src/base/shape.rs
index 0616c4c7e7..32f7f003bb 100644
--- a/crates/nu-data/src/base/shape.rs
+++ b/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)
+ }
+}
diff --git a/crates/nu-protocol/src/type_shape.rs b/crates/nu-protocol/src/type_shape.rs
index 9810908ea2..6d8c0efd0c 100644
--- a/crates/nu-protocol/src/type_shape.rs
+++ b/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 std::collections::BTreeMap;
+use serde::{Deserialize, Serialize};
+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: BTreeMap<Column, Type>,
+ map: IndexMap<Column, Type>,
}
-impl Serialize for Row {
- fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
- where
- S: serde::Serializer,
- {
- serializer.collect_map(self.map.iter())
+#[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<&Type>>().hash(state);
}
}
-impl<'de> Deserialize<'de> for Row {
- fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
- where
- D: Deserializer<'de>,
- {
- struct RowVisitor;
+impl PartialOrd for Row {
+ /// Compare two dictionaries for sort ordering
+ fn partial_cmp(&self, other: &Row) -> Option<Ordering> {
+ let this: Vec<&Column> = self.map.keys().collect();
+ let that: Vec<&Column> = other.map.keys().collect();
- impl<'de> serde::de::Visitor<'de> for RowVisitor {
- type Value = Row;
- 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);
- }
- }
- }
- }
+ if this != that {
+ return this.partial_cmp(&that);
}
- 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());