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

bucket_seed should be a member of Bucket #246

Open
edongashi opened this issue Mar 21, 2022 · 4 comments
Open

bucket_seed should be a member of Bucket #246

edongashi opened this issue Mar 21, 2022 · 4 comments

Comments

@edongashi
Copy link
Member

Also compute_bucket_seed is out of place in anonymization.c. It should be moved to common.c.

@cristianberneanu
Copy link
Collaborator

cristianberneanu commented Mar 21, 2022

Also compute_bucket_seed is out of place in anonymization.c. It should be moved to common.c.

Why is it out of place in anonymization.c?
Even if we don't want it there, common.c doesn't seem like the right place for it. Maybe noise_layers.c or bucket.c?

@edongashi
Copy link
Member Author

edongashi commented Mar 21, 2022

Why is it out of place in anonymization.c?

anonymization.c should not reference Bucket or BucketDesc. In my view those are runtime concepts and the rewriter is a "compile" time process.

common.c doesn't seem like the right place for it

I saw that eval_low_count is there, which is used by bucket scan and star bucket. The function will be needed in the same places because that is where we instantiate buckets. I'm not strictly against moving it, but it felt a bit awkward.

@edongashi
Copy link
Member Author

If computing the seed multiple times is costly, worth implementing for the sake of speedup.

@edongashi
Copy link
Member Author

Low priority because it has little performance gains.

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

No branches or pull requests

2 participants