mbutrovich commented on code in PR #11782:
URL: https://github.com/apache/arrow-rs/pull/11782#discussion_r4342170147
##########
arrow-select/src/coalesce/byte_view.rs:
##########
@@ -121,13 +121,22 @@ impl<B: ByteViewType> InProgressByteViewArray<B> {
self.completed.push(buffer);
}
- fn append_views_by_filter(&mut self, views: &[u128], filter:
&FilterPredicate) {
+ /// Append the `views` selected by `filter` to `self.views`.
+ ///
+ /// # Safety
+ ///
+ /// Every row selected by `filter` must be less than `views.len()`, i.e.
+ /// `filter.filter_len() <= views.len()`.
+ unsafe fn append_views_by_filter(&mut self, views: &[u128], filter:
&FilterPredicate) {
let selected_count = filter.count();
Review Comment:
Should this check its precondition with a `debug_assert!`?
```suggestion
unsafe fn append_views_by_filter(&mut self, views: &[u128], filter:
&FilterPredicate) {
debug_assert!(filter.filter_len() <= views.len());
let selected_count = filter.count();
```
I don't see this as a pattern in the repo currently (enforcing `unsafe`
invariants with `debug_assert`) so more of a question than a suggestion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
