You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "laskoviymishka (via GitHub)" <gi...@apache.org> on 2026/09/15 15:24:26 UTC

[PR] feat(iceberg): serialize DV-s to the deletion-vector-v1 blob layout [iceberg-rust]

laskoviymishka opened a new pull request, #3232:
URL: https://github.com/apache/iceberg-rust/pull/3232

   ## Which issue does this PR close?
   
   Add DeleteVector::serialize, which produces a deletion-vector-v1 blob that DeleteVector::deserialize reads back (#2580). It frames the roaring treemap as [length][magic][vector][crc], matching what deserialize reads: the roaring portable 64-bit directory (a little-endian bitmap count followed by ascending u32-keyed bitmaps), a big-endian length prefix covering the magic and vector, and a big-endian CRC-32 over the same bytes. The vector is written as-is with sparse-key encoding rather than run-length-encoded, so for the same positions the bytes need not match Iceberg-Java's BitmapPositionDeleteIndex, which run-optimizes and writes dense keys; both encodings are spec-conformant and mutually decodable. This emits the raw blob only; wrapping it in a Puffin Blob with the snapshot and sequence-number properties is left to a later change.
   
   Fold the framing into a single implementation shared by serialize and the tests, so the encode paths no longer duplicate the layout: the test-only frame_dv_blob and the encode_dv_blob helpers now delegate to serialize, and the length prefix uses a checked u32 conversion instead of a truncating cast. Round-trip tests cover an empty vector, a single position, a dense single-container run, positions spanning multiple 2^32 containers, and a run-optimized dense range, plus golden assertions on the magic offset, the length prefix, and CRC validation, and a byte-stability test that re-serializes a decoded blob to guard against container-type normalization on the round trip.
   


-- 
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@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] feat(iceberg): serialize DV-s to the deletion-vector-v1 blob layout [iceberg-rust]

Posted by "anoopj (via GitHub)" <gi...@apache.org>.
anoopj commented on code in PR #3232:
URL: https://github.com/apache/iceberg-rust/pull/3232#discussion_r4031925837


##########
crates/iceberg/src/delete_vector.rs:
##########
@@ -309,15 +352,17 @@ impl BitOrAssign for DeleteVector {
     }
 }
 
-// Reproduces Iceberg-Java's `deletion-vector-v1` framing so tests can round-trip through
-// `deserialize` without a Java writer, and so other test modules can build blob fixtures.
-// Cross-implementation golden fixtures produced by Iceberg-Java are tracked separately; this
-// only checks that our decode matches our encode.
-#[cfg(test)]
-pub(crate) fn frame_dv_blob(vector: &[u8]) -> Vec<u8> {
+// The single implementation of the `deletion-vector-v1` framing: it prepends the big-endian
+// length prefix and magic, appends the big-endian CRC-32 over the magic and vector, and is shared
+// by `serialize` and by the tests that craft raw roaring directories `serialize` can never emit.
+// Only reachable through `serialize` (dead in non-test builds) and the tests, hence `allow(unused)`.
+#[allow(unused)]
+fn frame_dv_blob(vector: &[u8]) -> Vec<u8> {
     let body_len = DV_MAGIC_BYTES + vector.len();
     let mut blob = Vec::with_capacity(DV_LENGTH_PREFIX_BYTES + body_len + DV_CRC_BYTES);
-    blob.extend_from_slice(&(body_len as u32).to_be_bytes());
+    let body_len =
+        u32::try_from(body_len).expect("deletion-vector-v1 body length exceeds u32::MAX");

Review Comment:
   That is a good catch. But should we do a panic here? why not return an error? I agree it is highly unlikely to get to this though, but it would be good to avoid panics. 



-- 
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@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] feat(iceberg): serialize DV-s to the deletion-vector-v1 blob layout [iceberg-rust]

Posted by "Stefan-Dienst (via GitHub)" <gi...@apache.org>.
Stefan-Dienst commented on code in PR #3232:
URL: https://github.com/apache/iceberg-rust/pull/3232#discussion_r4019482384


##########
crates/iceberg/src/delete_vector.rs:
##########
@@ -402,41 +441,122 @@ mod tests {
         positions
     }
 
+    // Serializes `dv`, asserts the blob carries well-formed `deletion-vector-v1` framing, and that
+    // decoding it recovers the same set of positions. This is the round-trip property:
+    // `deserialize(serialize(dv))` yields the same positions as `dv`.
+    fn assert_roundtrips(dv: &DeleteVector) {
+        let blob = dv.serialize();
+
+        // The magic sits immediately after the 4-byte length prefix.
+        assert_eq!(
+            &blob[DV_LENGTH_PREFIX_BYTES..DV_LENGTH_PREFIX_BYTES + DV_MAGIC_BYTES],
+            &DV_MAGIC,
+            "magic must follow the length prefix"
+        );
+        // The length prefix counts the magic and vector, i.e. everything but itself and the CRC.
+        let declared = u32::from_be_bytes(blob[..DV_LENGTH_PREFIX_BYTES].try_into().unwrap());
+        assert_eq!(
+            declared as usize,
+            blob.len() - DV_LENGTH_PREFIX_BYTES - DV_CRC_BYTES,
+            "length prefix must cover the magic and vector"
+        );
+
+        // Decoding validates the CRC and reconstructs the vector.
+        let decoded = DeleteVector::deserialize(&blob).expect("serialized blob must decode");
+        assert_eq!(decoded.len(), dv.len());
+        assert_eq!(sorted(&decoded), sorted(dv));
+    }
+
+    #[test]
+    fn test_serialize_roundtrip_empty() {
+        assert_roundtrips(&DeleteVector::default());
+    }
+
+    #[test]
+    fn test_serialize_roundtrip_single_position() {
+        assert_roundtrips(&dv_of([42]));
+    }
+
     #[test]
-    fn test_deserialize_roundtrip_empty() {
-        let blob = encode_dv_blob(&DeleteVector::default());
-        assert_eq!(DeleteVector::deserialize(&blob).unwrap().len(), 0);
+    fn test_serialize_roundtrip_small() {
+        assert_roundtrips(&dv_of([0u64, 5, 100, 1000]));
     }
 
+    // Many positions within a single 2^32 container, so the directory holds exactly one bitmap.
     #[test]
-    fn test_deserialize_roundtrip_small() {
-        let positions = [0u64, 5, 100, 1000];
-        let dv = DeleteVector::deserialize(&encode_dv_blob(&dv_of(positions))).unwrap();
-        assert_eq!(sorted(&dv), positions);
+    fn test_serialize_roundtrip_many_in_one_container() {
+        assert_roundtrips(&dv_of((0..5_000).map(|i| i * 3)));
     }
 
+    // Positions spanning multiple 2^32 containers, so the directory holds several keyed bitmaps
+    // that must be emitted in ascending key order.
     #[test]
-    fn test_deserialize_roundtrip_spanning_64bit_keys() {
-        let positions = [1u64, 1 << 33, (1 << 33) + 5, 1 << 34];
-        let dv = DeleteVector::deserialize(&encode_dv_blob(&dv_of(positions))).unwrap();
-        assert_eq!(sorted(&dv), positions);
+    fn test_serialize_roundtrip_spanning_64bit_keys() {
+        assert_roundtrips(&dv_of([1u64, (1 << 32) + 5, (1 << 33) + 9]));
     }
 
     // Java run-optimizes every deletion vector before writing, so real blobs carry RUN
-    // containers, which use the SERIAL_COOKIE roaring layout. Force that layout so decode
+    // containers, which use the SERIAL_COOKIE roaring layout. Force that layout so the round trip
     // exercises the run-container path rather than only array and bitmap containers.
     #[test]
-    fn test_deserialize_roundtrip_run_optimized() {
+    fn test_serialize_roundtrip_run_optimized_dense_range() {
         let mut dv = dv_of(0..10_000);
         assert!(
             dv.inner.optimize(),
             "expected a dense range to run-length encode"
         );
-        let decoded = DeleteVector::deserialize(&encode_dv_blob(&dv)).unwrap();
-        assert_eq!(decoded.len(), 10_000);
-        let positions = sorted(&decoded);
-        assert_eq!(positions.first(), Some(&0));
-        assert_eq!(positions.last(), Some(&9_999));
+        assert_roundtrips(&dv);
+    }
+
+    // Spells out the `deletion-vector-v1` framing with literal offsets and magic bytes, guarding
+    // against a drift in the layout that a round trip through our own decoder could mask.
+    #[test]
+    fn test_serialize_golden_framing() {
+        let blob = dv_of([1u64, 2, 3]).serialize();
+
+        assert_eq!(&blob[4..8], &[0xD1, 0xD3, 0x39, 0x64]);
+        let declared = u32::from_be_bytes(blob[..4].try_into().unwrap());
+        // Everything but the length prefix and the trailing CRC is covered by `declared`.
+        let framing_overhead = DV_LENGTH_PREFIX_BYTES + DV_CRC_BYTES;

Review Comment:
   question: Just out of curiosity, why didn't you hard code `8` here instead of using the constants? Wouldn't you want to make this test as independent as possible?



-- 
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@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] feat(iceberg): serialize DV-s to the deletion-vector-v1 blob layout [iceberg-rust]

Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on code in PR #3232:
URL: https://github.com/apache/iceberg-rust/pull/3232#discussion_r4020320674


##########
crates/iceberg/src/delete_vector.rs:
##########
@@ -402,41 +441,122 @@ mod tests {
         positions
     }
 
+    // Serializes `dv`, asserts the blob carries well-formed `deletion-vector-v1` framing, and that
+    // decoding it recovers the same set of positions. This is the round-trip property:
+    // `deserialize(serialize(dv))` yields the same positions as `dv`.
+    fn assert_roundtrips(dv: &DeleteVector) {
+        let blob = dv.serialize();
+
+        // The magic sits immediately after the 4-byte length prefix.
+        assert_eq!(
+            &blob[DV_LENGTH_PREFIX_BYTES..DV_LENGTH_PREFIX_BYTES + DV_MAGIC_BYTES],
+            &DV_MAGIC,
+            "magic must follow the length prefix"
+        );
+        // The length prefix counts the magic and vector, i.e. everything but itself and the CRC.
+        let declared = u32::from_be_bytes(blob[..DV_LENGTH_PREFIX_BYTES].try_into().unwrap());
+        assert_eq!(
+            declared as usize,
+            blob.len() - DV_LENGTH_PREFIX_BYTES - DV_CRC_BYTES,
+            "length prefix must cover the magic and vector"
+        );
+
+        // Decoding validates the CRC and reconstructs the vector.
+        let decoded = DeleteVector::deserialize(&blob).expect("serialized blob must decode");
+        assert_eq!(decoded.len(), dv.len());
+        assert_eq!(sorted(&decoded), sorted(dv));
+    }
+
+    #[test]
+    fn test_serialize_roundtrip_empty() {
+        assert_roundtrips(&DeleteVector::default());
+    }
+
+    #[test]
+    fn test_serialize_roundtrip_single_position() {
+        assert_roundtrips(&dv_of([42]));
+    }
+
     #[test]
-    fn test_deserialize_roundtrip_empty() {
-        let blob = encode_dv_blob(&DeleteVector::default());
-        assert_eq!(DeleteVector::deserialize(&blob).unwrap().len(), 0);
+    fn test_serialize_roundtrip_small() {
+        assert_roundtrips(&dv_of([0u64, 5, 100, 1000]));
     }
 
+    // Many positions within a single 2^32 container, so the directory holds exactly one bitmap.
     #[test]
-    fn test_deserialize_roundtrip_small() {
-        let positions = [0u64, 5, 100, 1000];
-        let dv = DeleteVector::deserialize(&encode_dv_blob(&dv_of(positions))).unwrap();
-        assert_eq!(sorted(&dv), positions);
+    fn test_serialize_roundtrip_many_in_one_container() {
+        assert_roundtrips(&dv_of((0..5_000).map(|i| i * 3)));
     }
 
+    // Positions spanning multiple 2^32 containers, so the directory holds several keyed bitmaps
+    // that must be emitted in ascending key order.
     #[test]
-    fn test_deserialize_roundtrip_spanning_64bit_keys() {
-        let positions = [1u64, 1 << 33, (1 << 33) + 5, 1 << 34];
-        let dv = DeleteVector::deserialize(&encode_dv_blob(&dv_of(positions))).unwrap();
-        assert_eq!(sorted(&dv), positions);
+    fn test_serialize_roundtrip_spanning_64bit_keys() {
+        assert_roundtrips(&dv_of([1u64, (1 << 32) + 5, (1 << 33) + 9]));
     }
 
     // Java run-optimizes every deletion vector before writing, so real blobs carry RUN
-    // containers, which use the SERIAL_COOKIE roaring layout. Force that layout so decode
+    // containers, which use the SERIAL_COOKIE roaring layout. Force that layout so the round trip
     // exercises the run-container path rather than only array and bitmap containers.
     #[test]
-    fn test_deserialize_roundtrip_run_optimized() {
+    fn test_serialize_roundtrip_run_optimized_dense_range() {
         let mut dv = dv_of(0..10_000);
         assert!(
             dv.inner.optimize(),
             "expected a dense range to run-length encode"
         );
-        let decoded = DeleteVector::deserialize(&encode_dv_blob(&dv)).unwrap();
-        assert_eq!(decoded.len(), 10_000);
-        let positions = sorted(&decoded);
-        assert_eq!(positions.first(), Some(&0));
-        assert_eq!(positions.last(), Some(&9_999));
+        assert_roundtrips(&dv);
+    }
+
+    // Spells out the `deletion-vector-v1` framing with literal offsets and magic bytes, guarding
+    // against a drift in the layout that a round trip through our own decoder could mask.
+    #[test]
+    fn test_serialize_golden_framing() {
+        let blob = dv_of([1u64, 2, 3]).serialize();
+
+        assert_eq!(&blob[4..8], &[0xD1, 0xD3, 0x39, 0x64]);
+        let declared = u32::from_be_bytes(blob[..4].try_into().unwrap());
+        // Everything but the length prefix and the trailing CRC is covered by `declared`.
+        let framing_overhead = DV_LENGTH_PREFIX_BYTES + DV_CRC_BYTES;

Review Comment:
   to make this a bit more self-explanatory, just having 8 means not much, but this expression has meaning, alternative would be just a comment i guess.



-- 
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@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] feat(iceberg): serialize DV-s to the deletion-vector-v1 blob layout [iceberg-rust]

Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on code in PR #3232:
URL: https://github.com/apache/iceberg-rust/pull/3232#discussion_r4046931692


##########
crates/iceberg/src/delete_vector.rs:
##########
@@ -309,15 +352,17 @@ impl BitOrAssign for DeleteVector {
     }
 }
 
-// Reproduces Iceberg-Java's `deletion-vector-v1` framing so tests can round-trip through
-// `deserialize` without a Java writer, and so other test modules can build blob fixtures.
-// Cross-implementation golden fixtures produced by Iceberg-Java are tracked separately; this
-// only checks that our decode matches our encode.
-#[cfg(test)]
-pub(crate) fn frame_dv_blob(vector: &[u8]) -> Vec<u8> {
+// The single implementation of the `deletion-vector-v1` framing: it prepends the big-endian
+// length prefix and magic, appends the big-endian CRC-32 over the magic and vector, and is shared
+// by `serialize` and by the tests that craft raw roaring directories `serialize` can never emit.
+// Only reachable through `serialize` (dead in non-test builds) and the tests, hence `allow(unused)`.
+#[allow(unused)]
+fn frame_dv_blob(vector: &[u8]) -> Vec<u8> {
     let body_len = DV_MAGIC_BYTES + vector.len();
     let mut blob = Vec::with_capacity(DV_LENGTH_PREFIX_BYTES + body_len + DV_CRC_BYTES);
-    blob.extend_from_slice(&(body_len as u32).to_be_bytes());
+    let body_len =
+        u32::try_from(body_len).expect("deletion-vector-v1 body length exceeds u32::MAX");

Review Comment:
   agree, fixed!



-- 
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@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] feat(iceberg): serialize DV-s to the deletion-vector-v1 blob layout [iceberg-rust]

Posted by "JeonDaehong (via GitHub)" <gi...@apache.org>.
JeonDaehong commented on code in PR #3232:
URL: https://github.com/apache/iceberg-rust/pull/3232#discussion_r4181995275


##########
crates/iceberg/src/delete_vector.rs:
##########
@@ -122,6 +122,52 @@ impl DeleteVector {
 
         Ok(DeleteVector { inner })
     }
+
+    /// Serializes this `DeleteVector` into a `deletion-vector-v1` Puffin blob that
+    /// [`DeleteVector::deserialize`] reads back, conforming to the Iceberg Puffin
+    /// `deletion-vector-v1` layout:
+    ///
+    /// ```text
+    /// [length: u32 big-endian][magic: D1 D3 39 64][vector][crc: u32 big-endian]
+    /// ```
+    ///
+    /// `length` counts the magic and vector bytes (not itself or the CRC). The CRC-32 is computed
+    /// over the magic and vector. `vector` is the roaring bitmap in the portable 64-bit format: a
+    /// directory of 32-bit key / 32-bit roaring bitmap pairs ordered by unsigned comparison of the
+    /// keys, one bitmap per key. `roaring`'s `RoaringTreemap::serialize_into` emits exactly that
+    /// directory (a `u64` little-endian count followed by ascending `u32`-keyed bitmaps), which the
+    /// round-trip tests verify against [`decode_roaring_directory`].
+    ///
+    /// The roaring vector is written as-is, not run-length-encoded, using sparse-key encoding: only
+    /// the non-empty 32-bit containers are emitted. For the same logical set of positions these
+    /// bytes therefore need not match Iceberg-Java's `BitmapPositionDeleteIndex` output, which
+    /// run-optimizes the bitmap and writes dense keys. Both encodings are `deletion-vector-v1`
+    /// spec-conformant and decode to the same positions, so each side reads back what the other
+    /// writes. Callers that want size-optimal, Java-like blobs should `optimize()` the underlying
+    /// roaring bitmap (`RoaringTreemap::optimize`) before serializing.
+    ///
+    /// This produces the raw blob only; wrapping it in a Puffin `Blob` with the associated
+    /// snapshot and sequence-number properties is handled separately.
+    ///
+    /// # Errors
+    ///
+    /// Returns [`ErrorKind::DataInvalid`] if the framed body (the magic plus the vector) would
+    /// exceed `u32::MAX` bytes, since the `deletion-vector-v1` length prefix is a `u32`; reaching
+    /// this requires a deletion vector of roughly 4 GiB, far larger than any realistic set of row
+    /// positions. Returns [`ErrorKind::Unexpected`] if writing the roaring treemap into the
+    /// in-memory buffer fails, which does not happen under Rust's allocator model (an allocation
+    /// failure aborts rather than returning an error); it guards only against a hypothetical future
+    /// change to `roaring`'s API that would make `serialize_into` fallible for an in-memory `Vec`.
+    // Nothing in non-test crate code calls this yet (the delete-vector write path wires it in
+    // later), and `delete_vector` is a private module, so it otherwise reads as dead code.
+    #[allow(unused)]
+    pub fn serialize(&self) -> Result<Vec<u8>> {

Review Comment:
   The spec says the vector supports positive 64-bit positions ("the most significant bit must be 0"), but insert accepts any u64 and serialize doesn't check, so a position >= 2^63 would be written as a key >= 2^31. Iceberg Java rejects that on read (RoaringPositionBitmap.readKey requires key <= Integer.MAX_VALUE - 1), so the "mutually decodable" note wouldn't hold for that range.
   
   Real row positions won't get there, so this is mostly defensive, but would it make sense for serialize to return an error when the highest key has its top bit set, like the length check below? That would keep us from writing a blob other implementations can't read.



-- 
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@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] feat(iceberg): serialize DV-s to the deletion-vector-v1 blob layout [iceberg-rust]

Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka closed pull request #3232: feat(iceberg): serialize DV-s to the deletion-vector-v1 blob layout
URL: https://github.com/apache/iceberg-rust/pull/3232


-- 
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@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] feat(iceberg): serialize DV-s to the deletion-vector-v1 blob layout [iceberg-rust]

Posted by "dina-kar (via GitHub)" <gi...@apache.org>.
dina-kar commented on PR #3232:
URL: https://github.com/apache/iceberg-rust/pull/3232#issuecomment-6331420866

   @laskoviymishka, would you mind if we finished this PR? It hasn't changed since mid-September, and the deletion-vector writer in #2580 builds on it.
   
   If you're fine with that, I'd open a PR that keeps your two commits as they are, with you as their author. On top of them it would add:
   
   - the fix for the conflict with #3107;
   - the rustdoc fix mentioned above;
   - the `public-api.txt` entry;
   - a test against Spark-written deletion vectors.
   
   I'd also handle the open review point about positions of 2^63 and above. If you'd rather keep going yourself, that's great too. Just say so and I'll help with review and tests instead.
   


-- 
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@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] feat(iceberg): serialize DV-s to the deletion-vector-v1 blob layout [iceberg-rust]

Posted by "dina-kar (via GitHub)" <gi...@apache.org>.
dina-kar commented on PR #3232:
URL: https://github.com/apache/iceberg-rust/pull/3232#issuecomment-6321054868

   Thanks for working on this. We need deletion-vector writes in iceberg-rust, and we'd like to build a `DeletionVectorWriter` on top of this PR rather than duplicate it. The writer would emit the Puffin file and its `DataFile` entries (`referenced_data_file`, `content_offset`, `content_size_in_bytes`). I've described the plan on #2580.
   
   Some things that might help here:
   
   - We have DV tables written by Spark 4.0.4 with Iceberg 1.12.0: unpartitioned and partitioned tables, `DELETE`, merge-on-read `UPDATE` and `MERGE`, and a v2 table with position deletes upgraded to v3. For every Spark-written blob in them, we can check that `deserialize` → `serialize` → `deserialize` returns the same positions, and that Spark and Trino read the blobs this PR writes once they're committed. If useful, I can contribute those checks as tests, either here or in a follow-up PR, whichever you prefer.
   - The branch currently conflicts with `main`: `crates/iceberg/src/test_utils.rs` was moved in #3107.
   - Your suggestion to `optimize()` before serializing matches what we saw: Spark's blobs are run-optimized.
   
   Our own builds carry this PR's two commits unchanged, so nothing here needs to change on our account.
   


-- 
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@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] feat(iceberg): serialize DV-s to the deletion-vector-v1 blob layout [iceberg-rust]

Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka commented on PR #3232:
URL: https://github.com/apache/iceberg-rust/pull/3232#issuecomment-6373347876

   @dina-kar - i'm fine with that


-- 
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@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] feat(iceberg): serialize DV-s to the deletion-vector-v1 blob layout [iceberg-rust]

Posted by "dina-kar (via GitHub)" <gi...@apache.org>.
dina-kar commented on PR #3232:
URL: https://github.com/apache/iceberg-rust/pull/3232#issuecomment-6373372879

    Thanks @laskoviymishka


-- 
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@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org