Off by 1 in slice of OffsetsBuffer

[kylebarron]

The implementations of slice on OffsetsBuffer calls slice on the underlying Buffer directly:

arrow2/src/offset.rs

Lines 443 to 459 in 2ecd3e8

	/// Slices this [`OffsetsBuffer`].
	/// # Panics
	/// Panics if `offset + length` is larger than `len`
	/// or `length == 0`.
	#[inline]
	pub fn slice(&mut self, offset: usize, length: usize) {
	assert!(length > 0);
	self.0.slice(offset, length);
	}
	/// Slices this [`OffsetsBuffer`] starting at `offset`.
	/// # Safety
	/// The caller must ensure `offset + length <= self.len()`
	#[inline]
	pub unsafe fn slice_unchecked(&mut self, offset: usize, length: usize) {
	self.0.slice_unchecked(offset, length);
	}

But this is incorrect because then a slice with length 1 returns an OffsetsBuffer with only one value in the buffer.

I'd argue that either the documentation could be better that this is slicing the raw buffer and not the offsets that a slice of values of that length would have. Or the code could add the +1 itself.

[ritchie46]

I don't think this is incorrect? It slices the buffer? It doesn't do any semantic handling.

[kylebarron]

I found it confusing because at least in a couple places, the API does do semantic handling instead of raw buffer operations. E.g. with_capacity reserves space for n + 1 elements, so the input number refers to the number of values, not the number of offsets.

In any case, it's probably too disruptive to change the API now, so a docstring improvement would probably be best