feat(bloom): introduce BloomFilterInvertedView for inverted queries - #271
feat(bloom): introduce BloomFilterInvertedView for inverted queries#271v0id-X wants to merge 4 commits into
Conversation
| /// * Constant space usage | ||
| /// | ||
| /// These guarantees hold until [`invert()`](Self::invert) is called; see its documentation. | ||
| /// These guarantees hold unless inverted via [`invert()`](Self::invert). |
There was a problem hiding this comment.
This is no longer relevant because the inverted view is now a brand-new read-only view. It is no longer a BloomFilter.
| /// Updates are disallowed on an inverted view to prevent unsound filter states. An inverted | ||
| /// view can be converted back into an updatable [`BloomFilter`] via | ||
| /// [`invert()`](BloomFilterInvertedView::invert) or | ||
| /// [`into_filter()`](BloomFilterInvertedView::into_filter). |
There was a problem hiding this comment.
I think this can be a document of BloomFilterInvertedView rather than details on the method.
There was a problem hiding this comment.
Agreed. I'll trim the method documentation down to its core behavior and move the details regarding disallowed mutations and reinversion up to the BloomFilterInvertedView struct documentation.
| /// Re-inverts the view back into an updatable [`BloomFilter`]. | ||
| /// | ||
| /// Inverting twice restores the original bit state and filter guarantees. | ||
| /// | ||
| /// # Examples | ||
| /// | ||
| /// ``` | ||
| /// use datasketches::bloom::BloomFilterBuilder; | ||
| /// | ||
| /// let mut filter = BloomFilterBuilder::with_accuracy(100, 0.01) | ||
| /// .build() | ||
| /// .unwrap(); | ||
| /// filter.insert("apple"); | ||
| /// | ||
| /// let inverted = filter.invert(); | ||
| /// let restored = inverted.invert(); | ||
| /// assert!(restored.contains(&"apple")); | ||
| /// ``` | ||
| pub fn invert(self) -> BloomFilter { | ||
| self.into_filter() | ||
| } | ||
|
|
||
| /// Converts this inverted view back into an updatable [`BloomFilter`] by | ||
| /// inverting the bits again. | ||
| /// | ||
| /// Equivalent to [`invert()`](Self::invert). | ||
| pub fn into_filter(mut self) -> BloomFilter { | ||
| for word in &mut self.inner.bit_array { | ||
| *word = !*word; | ||
| } | ||
| self.inner.num_bits_set = self.inner.capacity() as u64 - self.inner.num_bits_set; | ||
| self.inner | ||
| } |
There was a problem hiding this comment.
I don't think we need alias here. Please think of it by yourself rather than just blindly follow the issue description.
There was a problem hiding this comment.
That makes a lot more sense. Symmetrical .invert() (filter.invert().invert()) is way cleaner anyway. Dropping into_filter and keeping just invert(self) -> BloomFilter.
| /// Returns a reference to the underlying [`BloomFilter`] representation. | ||
| pub fn as_filter(&self) -> &BloomFilter { | ||
| &self.inner | ||
| } |
There was a problem hiding this comment.
I thought about it, this makes the abstraction weaker risking misuse. Removing it entirely.
| All significant changes to this project will be documented in this file. | ||
|
|
||
| ## Unreleased | ||
| - feat(bloom): make post-invert semantics observable via `BloomFilterInvertedView` (#270, #271) |
There was a problem hiding this comment.
Please follow the changelog format like other entries.
There was a problem hiding this comment.
My bad, will fix the formatting to match the rest of the file.
|
All review comments addressed and updated:
|
2 participants
Closes #270