Merge pull request #124 from Qrlew/fix_protection

Use `PEPRelation::try_from(..)` instead of just `PEPRelation(..)`

CHANGELOG.md

@@ -6,7 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+
+### Aded
 - conversion DataType -> Value for Expr::Function [MR122](https://github.com/Qrlew/qrlew/pull/122)
+### Fixed
+- in protection use `PEPRelation::try_from(..)` instead of `PEPRelation(..)`
 
 ## [0.3.1] - 2023-09-16
 ### Fixed

src/differential_privacy/mod.rs

@@ -14,7 +14,7 @@ use crate::{
     display::Dot,
     expr::{self, aggregate, AggregateColumn, Expr},
     hierarchy::Hierarchy,
-    protection::PEPRelation,
+    protection::{self, PEPRelation},
     relation::{field::Field, transforms, Map, Reduce, Relation, Variant as _},
     DataType, Ready,
 };
@@ -59,6 +59,11 @@ impl From<transforms::Error> for Error {
         Error::Other(err.to_string())
     }
 }
+impl From<protection::Error> for Error {
+    fn from(err: protection::Error) -> Self {
+        Error::Other(err.to_string())
+    }
+}
 
 impl error::Error for Error {}
 pub type Result<T> = result::Result<T, Error>;
@@ -118,7 +123,8 @@ impl PEPRelation {
         let protected_entity_weight = self.protected_entity_weight().to_string();
         match Relation::from(self) {
             Relation::Map(map) => {
-                let dp_input = PEPRelation(map.input().clone()).dp_compile(epsilon, delta)?;
+                let dp_input =
+                    PEPRelation::try_from(map.input().clone())?.dp_compile(epsilon, delta)?;
                 Ok(DPRelation(
                     Map::builder()
                         .with(map)

src/protection/mod.rs

@@ -82,6 +82,23 @@ impl From<PEPRelation> for Relation {
     }
 }
 
+impl TryFrom<Relation> for PEPRelation {
+    type Error = Error;
+
+    fn try_from(value: Relation) -> Result<Self> {
+        if value.is_pep() {
+            Ok(PEPRelation(value))
+        } else {
+            Err(Error::NotProtectedEntityPreserving(
+                format!(
+                    "Cannot convert to PEPRelation a relation that does not contains both {} and {} columns. \nGot: {}",
+                    PE_ID, PE_WEIGHT, value.schema().iter().map(|f| f.name()).collect::<Vec<_>>().join(",")
+                )
+            ))
+        }
+    }
+}
+
 impl Deref for PEPRelation {
     type Target = Relation;
 
@@ -90,6 +107,16 @@ impl Deref for PEPRelation {
     }
 }
 
+impl Relation {
+    pub fn is_pep(&self) -> bool {
+        if self.schema().field(PE_ID).is_err() || self.schema().field(PE_WEIGHT).is_err() {
+            false
+        } else {
+            true
+        }
+    }
+}
+
 /// A visitor to compute Relation protection
 #[derive(Clone, Debug)]
 pub struct ProtectVisitor<F: Fn(&Table) -> Result<PEPRelation>> {
@@ -113,18 +140,18 @@ pub fn protect_visitor_from_exprs<'a>(
     protected_entity: &'a [(&'a Table, Expr)],
     strategy: Strategy,
 ) -> ProtectVisitor<impl Fn(&Table) -> Result<PEPRelation> + 'a> {
-    ProtectVisitor::new(
-        move |table: &Table| match protected_entity
-            .iter()
-            .find_map(|(t, e)| (table == *t).then(|| e.clone()))
-        {
-            Some(expr) => Ok(PEPRelation(
-                Relation::from(table.clone()).identity_with_field(PE_ID, expr.clone()),
-            )),
-            None => Err(Error::unprotected_table(table)),
-        },
-        strategy,
-    )
+    let protect_tables = move |table: &Table| match protected_entity
+        .iter()
+        .find_map(|(t, e)| (table == *t).then(|| e.clone()))
+    {
+        Some(expr) => PEPRelation::try_from(
+            Relation::from(table.clone())
+                .identity_with_field(PE_ID, expr.clone())
+                .insert_field(1, PE_WEIGHT, Expr::val(1)),
+        ),
+        None => Err(Error::unprotected_table(table)),
+    };
+    ProtectVisitor::new(protect_tables, strategy)
 }
 
 /// Build a visitor from exprs
@@ -133,59 +160,60 @@ pub fn protect_visitor_from_field_paths<'a>(
     protected_entity: &'a [(&'a str, &'a [(&'a str, &'a str, &'a str)], &'a str)],
     strategy: Strategy,
 ) -> ProtectVisitor<impl Fn(&Table) -> Result<PEPRelation> + 'a> {
-    ProtectVisitor::new(
-        move |table: &Table| match protected_entity
-            .iter()
-            .find(|(tab, _path, _field)| table.name() == relations[*tab].name())
-        {
-            Some((_tab, path, field)) => Ok(PEPRelation(
-                Relation::from(table.clone())
-                    .with_field_path(relations, path, field, PE_ID)
-                    .map_fields(|n, e| {
-                        if n == PE_ID {
-                            Expr::md5(Expr::cast_as_text(e))
-                        } else {
-                            e
-                        }
-                    }),
-            )),
-            None => Err(Error::unprotected_table(table)),
-        }, //TODO fix MD5 here
-        strategy,
-    )
+    let protect_tables = move |table: &Table| match protected_entity
+        .iter()
+        .find(|(tab, _path, _field)| table.name() == relations[*tab].name())
+    {
+        Some((_tab, path, field)) => PEPRelation::try_from(
+            Relation::from(table.clone())
+                .with_field_path(relations, path, field, PE_ID)
+                .map_fields(|n, e| {
+                    if n == PE_ID {
+                        Expr::md5(Expr::cast_as_text(e))
+                    } else {
+                        e
+                    }
+                })
+                .insert_field(1, PE_WEIGHT, Expr::val(1)),
+        ),
+        None => Err(Error::unprotected_table(table)),
+    }; //TODO fix MD5 here
+    ProtectVisitor::new(protect_tables, strategy)
 }
 
 impl<'a, F: Fn(&Table) -> Result<PEPRelation>> Visitor<'a, Result<PEPRelation>>
     for ProtectVisitor<F>
 {
     fn table(&self, table: &'a Table) -> Result<PEPRelation> {
-        Ok(PEPRelation(
+        PEPRelation::try_from(
             Relation::from((self.protect_tables)(table)?)
                 .insert_field(1, PE_WEIGHT, Expr::val(1))
                 // We preserve the name
                 .with_name(format!("{}{}", PROTECTION_PREFIX, table.name())),
-        ))
+        )
     }
 
     fn map(&self, map: &'a Map, input: Result<PEPRelation>) -> Result<PEPRelation> {
-        let builder = Relation::map()
+        let relation: Relation = Relation::map()
             .with((PE_ID, Expr::col(PE_ID)))
             .with((PE_WEIGHT, Expr::col(PE_WEIGHT)))
             .with(map.clone())
-            .input(Relation::from(input?));
-        Ok(PEPRelation(builder.build()))
+            .input(Relation::from(input?))
+            .build();
+        PEPRelation::try_from(relation)
     }
 
     fn reduce(&self, reduce: &'a Reduce, input: Result<PEPRelation>) -> Result<PEPRelation> {
         match self.strategy {
             Strategy::Soft => Err(Error::not_protected_entity_preserving(reduce)),
             Strategy::Hard => {
-                let builder = Relation::reduce()
+                let relation: Relation = Relation::reduce()
                     .with_group_by_column(PE_ID)
                     .with((PE_WEIGHT, AggregateColumn::sum(PE_WEIGHT)))
                     .with(reduce.clone())
-                    .input(Relation::from(input?));
-                Ok(PEPRelation(builder.build()))
+                    .input(Relation::from(input?))
+                    .build();
+                PEPRelation::try_from(relation)
             }
         }
     }
@@ -269,9 +297,9 @@ impl<'a, F: Fn(&Table) -> Result<PEPRelation>> Visitor<'a, Result<PEPRelation>>
                         b.with((n, Expr::col(n)))
                     }
                 });
-                let builder = builder.input(Rc::new(join.into()));
+                let relation: Relation = builder.input(Rc::new(join.into())).build();
 
-                Ok(PEPRelation(builder.build()))
+                PEPRelation::try_from(relation)
             }
         }
     }
@@ -282,17 +310,18 @@ impl<'a, F: Fn(&Table) -> Result<PEPRelation>> Visitor<'a, Result<PEPRelation>>
         left: Result<PEPRelation>,
         right: Result<PEPRelation>,
     ) -> Result<PEPRelation> {
-        let builder = Relation::set()
+        let relation: Relation = Relation::set()
             .name(set.name())
             .operator(set.operator().clone())
             .quantifier(set.quantifier().clone())
             .left(Relation::from(left?))
-            .right(Relation::from(right?));
-        Ok(PEPRelation(builder.build()))
+            .right(Relation::from(right?))
+            .build();
+        PEPRelation::try_from(relation)
     }
 
     fn values(&self, values: &'a Values) -> Result<PEPRelation> {
-        Ok(PEPRelation(Relation::Values(values.clone())))
+        PEPRelation::try_from(Relation::Values(values.clone()))
     }
 }