You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "james-willis (via GitHub)" <gi...@apache.org> on 2026/06/06 03:29:36 UTC
[PR] feat(raster-functions): zero-copy band-data in RS_EnsureLoaded [sedona-db]
james-willis opened a new pull request, #917:
URL: https://github.com/apache/sedona-db/pull/917
(no comment)
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(rust/sedona-raster,rust/sedona-raster-functions): zero-copy band-data in RS_EnsureLoaded [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot merged PR #917:
URL: https://github.com/apache/sedona-db/pull/917
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-functions): zero-copy band-data in RS_EnsureLoaded [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #917:
URL: https://github.com/apache/sedona-db/pull/917#discussion_r3375965912
##########
rust/sedona-raster/src/builder.rs:
##########
@@ -431,6 +439,75 @@ impl RasterBuilder {
&mut self.band_data
}
+ /// Append the current band's data as a **zero-copy** view into an existing
+ /// Arrow [`Buffer`], rather than copying bytes via `append_value`.
+ ///
+ /// `buffer` is attached to the output `BinaryViewArray` as a shared data
+ /// block (a refcount bump, never a copy) and a single view row referencing
+ /// `[offset, offset + len)` is appended. Buffers are de-duplicated by data
+ /// pointer, so handing the same backing buffer for many bands attaches it
+ /// once and points every view at it.
+ ///
+ /// Bytes shorter than the 12-byte inline threshold are stored inline (a
+ /// tiny copy) instead, since the `BinaryViewArray` layout requires `len <=
+ /// 12` views to be inline and a block-referencing view of that size is
+ /// non-canonical (it fails array validation on roundtrip). Band data is
+ /// realistically always larger, so this is the degenerate path.
+ ///
+ /// Counts as the one data value for the current band (see [`finish_band`]).
+ ///
+ /// [`finish_band`]: Self::finish_band
+ pub fn append_band_data_buffer(
+ &mut self,
+ buffer: &Buffer,
+ offset: u32,
+ len: u32,
+ ) -> Result<(), ArrowError> {
+ if len <= 12 {
+ self.band_data
+ .append_value(&buffer.as_slice()[offset as usize..(offset + len) as usize]);
+ return Ok(());
+ }
+ let block = match self.band_data_blocks.get(&(buffer.as_ptr() as usize)) {
+ Some(&idx) => idx,
+ None => {
+ // `clone` bumps the buffer's refcount; the bytes are not copied.
+ let idx = self.band_data.append_block(buffer.clone());
+ self.band_data_blocks.insert(buffer.as_ptr() as usize, idx);
+ idx
+ }
+ };
+ self.band_data.try_append_view(block, offset, len)
+ }
+
+ /// Append the current band's data by copying row `row` of `src` through —
+ /// zero-copy when the row's bytes are block-backed (shares the backing
+ /// `Buffer` via [`append_band_data_buffer`]), with a small copy only for
+ /// inline views (≤ 12 bytes, which live in the view itself and have no
+ /// backing buffer).
+ ///
+ /// Counts as the one data value for the current band (see [`finish_band`]).
+ ///
+ /// [`finish_band`]: Self::finish_band
+ pub fn append_band_data_from(
+ &mut self,
+ src: &BinaryViewArray,
+ row: usize,
+ ) -> Result<(), ArrowError> {
+ // BYTE_VIEW u128 layout: [len: u32][prefix: u32][buffer_index: u32][offset: u32].
+ // A `len <= 12` view stores its bytes inline (no backing buffer).
+ let view = src.views()[row];
+ let len = view as u32;
+ if len <= 12 {
+ self.band_data.append_value(src.value(row));
+ Ok(())
+ } else {
+ let buffer_index = (view >> 64) as u32;
+ let offset = (view >> 96) as u32;
+ self.append_band_data_buffer(&src.data_buffers()[buffer_index as usize], offset, len)
+ }
Review Comment:
No, it doesnt handle the inline case, attaching the block from the source, or getting the block/offset/len
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-functions): zero-copy band-data in RS_EnsureLoaded [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #917:
URL: https://github.com/apache/sedona-db/pull/917#discussion_r3375648944
##########
rust/sedona-raster/src/builder.rs:
##########
@@ -431,6 +439,75 @@ impl RasterBuilder {
&mut self.band_data
}
+ /// Append the current band's data as a **zero-copy** view into an existing
+ /// Arrow [`Buffer`], rather than copying bytes via `append_value`.
+ ///
+ /// `buffer` is attached to the output `BinaryViewArray` as a shared data
+ /// block (a refcount bump, never a copy) and a single view row referencing
+ /// `[offset, offset + len)` is appended. Buffers are de-duplicated by data
+ /// pointer, so handing the same backing buffer for many bands attaches it
+ /// once and points every view at it.
+ ///
+ /// Bytes shorter than the 12-byte inline threshold are stored inline (a
+ /// tiny copy) instead, since the `BinaryViewArray` layout requires `len <=
+ /// 12` views to be inline and a block-referencing view of that size is
+ /// non-canonical (it fails array validation on roundtrip). Band data is
+ /// realistically always larger, so this is the degenerate path.
+ ///
+ /// Counts as the one data value for the current band (see [`finish_band`]).
+ ///
+ /// [`finish_band`]: Self::finish_band
+ pub fn append_band_data_buffer(
+ &mut self,
+ buffer: &Buffer,
+ offset: u32,
+ len: u32,
+ ) -> Result<(), ArrowError> {
+ if len <= 12 {
+ self.band_data
+ .append_value(&buffer.as_slice()[offset as usize..(offset + len) as usize]);
+ return Ok(());
+ }
+ let block = match self.band_data_blocks.get(&(buffer.as_ptr() as usize)) {
+ Some(&idx) => idx,
+ None => {
+ // `clone` bumps the buffer's refcount; the bytes are not copied.
+ let idx = self.band_data.append_block(buffer.clone());
+ self.band_data_blocks.insert(buffer.as_ptr() as usize, idx);
+ idx
+ }
+ };
+ self.band_data.try_append_view(block, offset, len)
+ }
+
+ /// Append the current band's data by copying row `row` of `src` through —
+ /// zero-copy when the row's bytes are block-backed (shares the backing
+ /// `Buffer` via [`append_band_data_buffer`]), with a small copy only for
+ /// inline views (≤ 12 bytes, which live in the view itself and have no
+ /// backing buffer).
+ ///
+ /// Counts as the one data value for the current band (see [`finish_band`]).
+ ///
+ /// [`finish_band`]: Self::finish_band
+ pub fn append_band_data_from(
+ &mut self,
+ src: &BinaryViewArray,
+ row: usize,
+ ) -> Result<(), ArrowError> {
+ // BYTE_VIEW u128 layout: [len: u32][prefix: u32][buffer_index: u32][offset: u32].
+ // A `len <= 12` view stores its bytes inline (no backing buffer).
+ let view = src.views()[row];
+ let len = view as u32;
+ if len <= 12 {
+ self.band_data.append_value(src.value(row));
+ Ok(())
+ } else {
+ let buffer_index = (view >> 64) as u32;
+ let offset = (view >> 96) as u32;
+ self.append_band_data_buffer(&src.data_buffers()[buffer_index as usize], offset, len)
+ }
Review Comment:
I believe `try_append_view()` will do this for you without hardcoding the struct layout?
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-functions): zero-copy band-data in RS_EnsureLoaded [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #917:
URL: https://github.com/apache/sedona-db/pull/917#discussion_r3376429269
##########
rust/sedona-raster/src/builder.rs:
##########
@@ -431,6 +444,81 @@ impl RasterBuilder {
&mut self.band_data
}
+ /// Append the current band's data as a **zero-copy** view into an existing
+ /// Arrow [`Buffer`], rather than copying bytes via `append_value`.
+ ///
+ /// `buffer` is attached to the output `BinaryViewArray` as a shared data
+ /// block (a refcount bump, never a copy) and a single view row referencing
+ /// `[offset, offset + len)` is appended. Buffers are de-duplicated by data
+ /// pointer, so handing the same backing buffer for many bands attaches it
+ /// once and points every view at it.
+ ///
+ /// Bytes at or under the inline threshold ([`MAX_INLINE_VIEW_LEN`]) are
+ /// stored inline (a tiny copy) instead, since the `BinaryViewArray` layout
+ /// requires such views to be inline and a block-referencing view of that
+ /// size is non-canonical (it fails array validation on roundtrip). Band
+ /// data is realistically always larger, so this is the degenerate path.
+ ///
+ /// Counts as the one data value for the current band (see [`finish_band`]).
+ ///
+ /// [`finish_band`]: Self::finish_band
+ pub fn append_band_data_buffer(
+ &mut self,
+ buffer: &Buffer,
+ offset: u32,
+ len: u32,
+ ) -> Result<(), ArrowError> {
+ if len <= MAX_INLINE_VIEW_LEN {
+ self.band_data
+ .append_value(&buffer.as_slice()[offset as usize..(offset + len) as usize]);
+ return Ok(());
+ }
+ let block = match self.band_data_blocks.get(&(buffer.as_ptr() as usize)) {
+ Some(&idx) => idx,
+ None => {
+ // `clone` bumps the buffer's refcount; the bytes are not copied.
+ let idx = self.band_data.append_block(buffer.clone());
+ self.band_data_blocks.insert(buffer.as_ptr() as usize, idx);
+ idx
+ }
+ };
+ self.band_data.try_append_view(block, offset, len)
+ }
+
+ /// Append the current band's data by copying row `row` of `src` through —
+ /// zero-copy when the row's bytes are block-backed (shares the backing
+ /// `Buffer` via [`append_band_data_buffer`]), with a small copy only for
+ /// inline views (≤ [`MAX_INLINE_VIEW_LEN`] bytes, which live in the view
+ /// itself and have no backing buffer).
+ ///
+ /// Counts as the one data value for the current band (see [`finish_band`]).
+ ///
+ /// [`finish_band`]: Self::finish_band
+ pub fn append_band_data_from(
+ &mut self,
+ src: &BinaryViewArray,
+ row: usize,
+ ) -> Result<(), ArrowError> {
+ // Arrow BYTE_VIEW layout (u128, little-endian fields), fixed by the
+ // columnar format spec:
+ // bits 0..32 length
+ // bits 32..64 prefix
+ // bits 64..96 buffer_index
+ // bits 96..128 offset
+ // A view of `length <= MAX_INLINE_VIEW_LEN` stores its bytes inline and
+ // has no backing buffer to share.
+ let view = src.views()[row];
+ let len = view as u32;
+ if len <= MAX_INLINE_VIEW_LEN {
+ self.band_data.append_value(src.value(row));
+ Ok(())
+ } else {
+ let buffer_index = (view >> 64) as u32;
+ let offset = (view >> 96) as u32;
+ self.append_band_data_buffer(&src.data_buffers()[buffer_index as usize], offset, len)
+ }
Review Comment:
Might be nice if this also used the ByteView but this works too so let's go with it 🙂
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org