From 16bd7b6d0deabbf508ad2d11394c26a8fedc7f0c Mon Sep 17 00:00:00 2001
From: WindSoilder <WindSoilder@outlook.com>
Date: Sat, 14 May 2022 19:04:09 +0800
Subject: [PATCH] Fix Value::Record compare logic, and pass uniq tests. (#5541)

* fix record compare logic

* add more comment
---
 crates/nu-command/tests/commands/uniq.rs |  4 ----
 crates/nu-protocol/src/value/mod.rs      | 28 ++++++++++++++++++++++--
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/crates/nu-command/tests/commands/uniq.rs b/crates/nu-command/tests/commands/uniq.rs
index 58c1bcbad4..6cf9155a80 100644
--- a/crates/nu-command/tests/commands/uniq.rs
+++ b/crates/nu-command/tests/commands/uniq.rs
@@ -61,8 +61,6 @@ fn uniq_values() {
     })
 }
 
-// FIXME: jt: needs more work
-#[ignore]
 #[test]
 fn nested_json_structures() {
     Playground::setup("uniq_test_3", |dirs, sandbox| {
@@ -127,8 +125,6 @@ fn nested_json_structures() {
     })
 }
 
-// FIXME: jt: needs more work
-#[ignore]
 #[test]
 fn uniq_when_keys_out_of_order() {
     let actual = nu!(
diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs
index df7452a697..b0aa0baea6 100644
--- a/crates/nu-protocol/src/value/mod.rs
+++ b/crates/nu-protocol/src/value/mod.rs
@@ -27,6 +27,7 @@ use crate::{did_you_mean, BlockId, Config, Span, Spanned, Type, VarId};
 
 use crate::ast::Operator;
 pub use custom_value::CustomValue;
+use std::iter;
 
 use crate::ShellError;
 
@@ -1287,9 +1288,17 @@ impl PartialOrd for Value {
                     vals: rhs_vals,
                     ..
                 } => {
-                    let result = lhs_cols.partial_cmp(rhs_cols);
+                    // reorder cols and vals to make more logically compare.
+                    // more genral, if two record have same col and values,
+                    // the order of cols shouldn't affect the equal property.
+                    let (lhs_cols_ordered, lhs_vals_ordered) =
+                        reorder_record_inner(lhs_cols, lhs_vals);
+                    let (rhs_cols_ordered, rhs_vals_ordered) =
+                        reorder_record_inner(rhs_cols, rhs_vals);
+
+                    let result = lhs_cols_ordered.partial_cmp(&rhs_cols_ordered);
                     if result == Some(Ordering::Equal) {
-                        lhs_vals.partial_cmp(rhs_vals)
+                        lhs_vals_ordered.partial_cmp(&rhs_vals_ordered)
                     } else {
                         result
                     }
@@ -2232,6 +2241,21 @@ impl Value {
     }
 }
 
+fn reorder_record_inner(cols: &[String], vals: &[Value]) -> (Vec<String>, Vec<Value>) {
+    let mut kv_pairs =
+        iter::zip(cols.to_owned(), vals.to_owned()).collect::<Vec<(String, Value)>>();
+    kv_pairs.sort_by(|a, b| {
+        a.0.partial_cmp(&b.0)
+            .expect("Columns should support compare")
+    });
+    let (mut cols, mut vals) = (vec![], vec![]);
+    for (col, val) in kv_pairs {
+        cols.push(col);
+        vals.push(val);
+    }
+    (cols, vals)
+}
+
 /// Create a Value::Record from a spanned hashmap
 impl From<Spanned<HashMap<String, Value>>> for Value {
     fn from(input: Spanned<HashMap<String, Value>>) -> Self {