Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added protection in dp_compile #129

Merged
merged 17 commits into from
Sep 28, 2023
Merged

Conversation

vserret
Copy link
Contributor

@vserret vserret commented Sep 26, 2023

No description provided.

@vserret vserret requested a review from ngrislain September 26, 2023 08:44
victoria de sainte agathe added 2 commits September 26, 2023 12:27
delta: f64,
) -> Result<(PEPRelation, PrivateQuery)> {
Ok(
if self.grouping_columns()? == vec![protected_entity_id.to_string()] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a special case ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the self is a PEPRelation so that we have added PE_ID in the grouping columns
And by definition we cannot reveal the keys with only one PE_ID value

Expr::eq(
Expr::qcol(self.name().to_string(), f.name().to_string()),
Expr::qcol(grouping_values.name().to_string(), f.name().to_string()),
pub fn protect_grouping_keys(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this function do ? pease document

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not protecting anything, what's the purpose of this ?

epsilon: f64,
delta: f64,
) -> Result<(PEPRelation, PrivateQuery)> {
let (protected_input, private_query) = PEPRelation::try_from(self.inputs()[0].clone())?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole point of having types for PEP and DP is to have compile time guarantees, you are defeating this point everywhere you use runtime failible conversions.

delta: f64,
) -> Result<(PEPRelation, PrivateQuery)> {
let (protected_input, private_query) = PEPRelation::try_from(self.inputs()[0].clone())?
.protect_grouping_keys(epsilon, delta)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that you are visiting the Relation without really doing it.
Are you sure your code will not process the input of an autojoin twice ?

) -> Result<(PEPRelation, PrivateQuery)> {
let (left_dp_relation, left_private_query) =
PEPRelation::try_from(self.inputs()[0].clone())?
.protect_grouping_keys(epsilon, delta)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure you are not processing the same relation twice?
Can you add a test for this case ?

@ngrislain ngrislain merged commit 7608dcd into main Sep 28, 2023
1 check failed
@vserret vserret deleted the add_tau_thrsolding_into_dp_compilation branch October 13, 2023 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants