mqy commented on a change in pull request #9193:
URL: https://github.com/apache/arrow/pull/9193#discussion_r558865481
##########
File path: rust/arrow/src/array/data.rs
##########
@@ -219,17 +219,15 @@ impl ArrayData {
///
/// Panics if `offset + length > self.len()`.
pub fn slice(&self, offset: usize, length: usize) -> ArrayData {
- assert!((offset + length) <= self.len());
-
- let mut new_data = self.clone();
-
- new_data.len = length;
- new_data.offset = offset + self.offset;
-
- new_data.null_count =
- count_nulls(new_data.null_buffer(), new_data.offset, new_data.len);
+ self.copy_range(offset, length)
+ }
- new_data
+ fn copy_range(&self, offset: usize, length: usize) -> ArrayData {
+ assert!((offset + length) <= self.len());
+ let arrays = vec![self];
Review comment:
Perhaps this hints the right direction:
- Possible performance speedup because no longer clone all array data and
use only part of it (think about the total length is large and the slice length
very small)
- Make the resulting ArrayData self-contained, thus less chances to mystery
bugs. In this case, perhaps it's better to rename `slice` as `copy_range` to
avoid mis-understanding.
The problem is: there are still 5 failures due to "not yet implemented" or
"not supported"
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
