Merge pull request #125 from Qrlew/replace_btree_set_in_union

Replace `BTreeSet` in `super_union` and `super_intersection` methods
Qrlew · Sep 22, 2023 · 805b659 · 805b659
2 parents f1324ca + b37d1a7
commit 805b659
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 19 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Aded
 - conversion DataType -> Value for Expr::Function [MR122](https://github.com/Qrlew/qrlew/pull/122)
+- replace `BTreeSet` in methods `super_union` and `super_intersection` of `data_type::Struct` and `data_type::Union`
+in order keep the inserting order (`BTreeSet` reorder keys by alphanumeric order) [MR125](https://github.com/Qrlew/qrlew/pull/125)
+
 ### Fixed
 - in protection use `PEPRelation::try_from(..)` instead of `PEPRelation(..)`
 

diff --git a/src/data_type/mod.rs b/src/data_type/mod.rs
@@ -1048,10 +1048,21 @@ impl Variant for Struct {
     }
 
     fn super_union(&self, other: &Self) -> Result<Self> {
-        let self_fields: BTreeSet<String> = self.fields.iter().map(|(f, _)| f.clone()).collect();
-        let other_fields: BTreeSet<String> = other.fields.iter().map(|(f, _)| f.clone()).collect();
-        let fields = self_fields.intersection(&other_fields);
-        fields
+        let self_fields: Vec<String> = self
+            .fields
+            .iter()
+            .map(|(f, _)| f.clone())
+            .unique()
+            .collect();
+        let other_fields: Vec<String> = other
+            .fields
+            .iter()
+            .map(|(f, _)| f.clone())
+            .unique()
+            .collect();
+        self_fields
+            .into_iter()
+            .chain(other_fields.into_iter())
             .into_iter()
             .map(|f| {
                 Ok((
@@ -1063,10 +1074,21 @@ impl Variant for Struct {
     }
 
     fn super_intersection(&self, other: &Self) -> Result<Self> {
-        let self_fields: BTreeSet<String> = self.fields.iter().map(|(f, _)| f.clone()).collect();
-        let other_fields: BTreeSet<String> = other.fields.iter().map(|(f, _)| f.clone()).collect();
-        let fields = self_fields.union(&other_fields);
-        fields
+        let self_fields: Vec<String> = self
+            .fields
+            .iter()
+            .map(|(f, _)| f.clone())
+            .unique()
+            .collect();
+        let other_fields: Vec<String> = other
+            .fields
+            .iter()
+            .map(|(f, _)| f.clone())
+            .unique()
+            .collect();
+        self_fields
+            .into_iter()
+            .chain(other_fields.into_iter())
             .into_iter()
             .map(|f| {
                 Ok((
@@ -1337,10 +1359,21 @@ impl Variant for Union {
     }
 
     fn super_union(&self, other: &Self) -> Result<Self> {
-        let self_fields: BTreeSet<String> = self.fields.iter().map(|(f, _)| f.clone()).collect();
-        let other_fields: BTreeSet<String> = other.fields.iter().map(|(f, _)| f.clone()).collect();
-        let fields = self_fields.union(&other_fields);
-        fields
+        let self_fields: Vec<String> = self
+            .fields
+            .iter()
+            .map(|(f, _)| f.clone())
+            .unique()
+            .collect();
+        let other_fields: Vec<String> = other
+            .fields
+            .iter()
+            .map(|(f, _)| f.clone())
+            .unique()
+            .collect();
+        self_fields
+            .into_iter()
+            .chain(other_fields.into_iter())
             .into_iter()
             .map(|f| {
                 Ok((
@@ -1352,10 +1385,21 @@ impl Variant for Union {
     }
 
     fn super_intersection(&self, other: &Self) -> Result<Self> {
-        let self_fields: BTreeSet<String> = self.fields.iter().map(|(f, _)| f.clone()).collect();
-        let other_fields: BTreeSet<String> = other.fields.iter().map(|(f, _)| f.clone()).collect();
-        let fields = self_fields.intersection(&other_fields);
-        fields
+        let self_fields: Vec<String> = self
+            .fields
+            .iter()
+            .map(|(f, _)| f.clone())
+            .unique()
+            .collect();
+        let other_fields: Vec<String> = other
+            .fields
+            .iter()
+            .map(|(f, _)| f.clone())
+            .unique()
+            .collect();
+        self_fields
+            .into_iter()
+            .chain(other_fields.into_iter())
             .into_iter()
             .map(|f| {
                 Ok((
@@ -3507,10 +3551,10 @@ mod tests {
         let c = a.clone().and(b.clone());
         let true_c = Struct::default()
             .and(("0", DataType::float()))
+            .and(("a", DataType::float_values([1., 2., 3.])))
             .and(("1", DataType::float()))
             .and(("2", DataType::float()))
             .and(("3", DataType::float()))
-            .and(("a", DataType::float_values([1., 2., 3.])))
             .and(("b", DataType::integer()))
             .and(("c", DataType::float()))
             .and(("d", DataType::float()));
@@ -3540,10 +3584,10 @@ mod tests {
             e,
             Struct::default()
                 .and(("0", DataType::float()))
+                .and(("a", DataType::float_values([1., 2., 3.])))
                 .and(("1", DataType::float()))
                 .and(("2", DataType::float()))
                 .and(("3", DataType::float()))
-                .and(("a", DataType::float_values([1., 2., 3.])))
                 .and(("b", DataType::integer()))
                 .and(("c", DataType::float()))
                 .and(("d", DataType::float()))
@@ -3936,7 +3980,7 @@ mod tests {
     }
 
     #[test]
-    fn test_struct_intersection() {
+    fn test_struct_intersection_and_union() {
         let left = DataType::unit()
             & ("a", DataType::integer_interval(0, 10))
             & ("b", DataType::integer_interval(-5, 0))
@@ -3954,6 +3998,37 @@ mod tests {
         println!("union = {}", left.super_union(&right).unwrap());
         assert!(left.is_subset_of(&left.super_union(&right).unwrap()));
         assert!(right.is_subset_of(&left.super_union(&right).unwrap()));
+
+        // struct of struct
+        let dt = DataType::structured([
+            (
+                "B",
+                DataType::structured([
+                    ("c", DataType::integer_interval(1, 8)),
+                    ("b", DataType::integer_interval(-5, 20)),
+                ]),
+            ),
+            (
+                "A",
+                DataType::structured([
+                    ("c", DataType::integer_min(0)),
+                    ("d", DataType::integer_interval(-1, 5)),
+                    ("a", DataType::integer_interval(-2, 25)),
+                ]),
+            ),
+        ]);
+        println!("\nintersection = {}", dt.super_intersection(&dt).unwrap());
+        println!("union = {}", dt.super_union(&dt).unwrap());
+        // check that the order of the keys is conserved
+        let true_schema = crate::relation::Schema::from(dt.clone());
+        assert_eq!(
+            crate::relation::Schema::from(dt.super_intersection(&dt).unwrap()),
+            true_schema
+        );
+        assert_eq!(
+            crate::relation::Schema::from(dt.super_union(&dt).unwrap()),
+            true_schema
+        );
     }
 
     #[test]