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

Add iterators to BooleanBuffer and NullBuffer #3901

Merged
merged 3 commits into from
Mar 23, 2023

Conversation

tustvold
Copy link
Contributor

Which issue does this PR close?

Part of #3880

Rationale for this change

Continues to flesh out the buffer abstractions

What changes are included in this PR?

Are there any user-facing changes?

@github-actions github-actions bot added the arrow Changes to the arrow crate label Mar 22, 2023
@@ -164,6 +165,21 @@ impl BooleanBuffer {
pub fn into_inner(self) -> Buffer {
self.buffer
}

/// Returns an iterator over the bits in this [`BooleanBuffer`]
pub fn iter(&self) -> BitIterator<'_> {
Copy link
Contributor

Choose a reason for hiding this comment

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

👌 nice

}

/// Returns a [`BitIndexIterator`] over the valid indices in this [`NullBuffer`]
pub fn valid_indices(&self) -> BitIndexIterator<'_> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the reason this is called valid_indices but the equivalent is called set_indices in BooleanBuffer is to highlight that a "Null buffer" in arrow is really a validity buffer (1 means non null)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I figured that it might avoid some confusion perhaps

self.buffer.iter()
}

/// Returns a [`BitIndexIterator`] over the valid indices in this [`NullBuffer`]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Returns a [`BitIndexIterator`] over the valid indices in this [`NullBuffer`]
/// Returns a [`BitIndexIterator`] over the valid indices in this [`NullBuffer`].
///
/// Valid indices indicate that the corresponding value is non null

@@ -114,6 +114,23 @@ impl NullBuffer {
Self::new(self.buffer.slice(offset, len))
}

/// Returns an iterator over the bits in this [`NullBuffer`]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Returns an iterator over the bits in this [`NullBuffer`]
/// Returns an iterator over the bits in this [`NullBuffer`]
///
/// * values of `1` indicate valid indices (the corresponding value is non null)
/// * values of `0` indicate invalid indices (the corresponding value is null)

self.buffer.set_indices()
}

/// Returns a [`BitSliceIterator`] yielding contiguous ranges of valid indices
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Returns a [`BitSliceIterator`] yielding contiguous ranges of valid indices
/// Returns a [`BitSliceIterator`] yielding contiguous ranges of valid indices
///
/// Valid indices indicate that the corresponding value is non null

@tustvold tustvold merged commit 1a42f4c into apache:master Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants