mbutrovich commented on code in PR #3454:
URL: https://github.com/apache/iceberg-rust/pull/3454#discussion_r4326434048


##########
crates/iceberg/src/delete_vector.rs:
##########
@@ -316,10 +460,284 @@ pub(crate) fn frame_dv_blob(vector: &[u8]) -> Vec<u8> {
     blob
 }
 
+/// Deterministic pseudo-random sets mixing single positions, short and long 
runs, runs across
+/// 2^16 and 2^32 boundaries, and empty bitmaps between used ones.
+#[cfg(test)]
+pub(crate) fn mixed_delete_vectors() -> Vec<DeleteVector> {
+    let mut state = 0x9E37_79B9_7F4A_7C15u64;
+    let mut rand = move || {
+        state ^= state << 13;
+        state ^= state >> 7;
+        state ^= state << 17;
+        state
+    };

Review Comment:
   Could we use a seeded `StdRng` here instead of a hand-written xorshift? 
`rand` is already a dev-dependency of the crate 
([`Cargo.toml`](https://github.com/apache/iceberg-rust/blob/0bc58a09bc21821e4e15e75593616d3597c4a22b/crates/iceberg/Cargo.toml#L94)).
 It would replace this closure and its copies in 
`test_ranges_skip_to_matches_model`, `test_advance_to_matches_model` and 
`test_build_deletes_row_selection_matches_model`. The modulo arithmetic at the 
call sites would also become ranges that say what they mean:
   
   ```rust
   let mut rng = StdRng::seed_from_u64(7);
   let run = match rng.random_range(0..5) {
       0 => 1,
       1 => 2,
       // one past the probe length: the rest comes from `next_range`
       2 => u64::from(RUN_PROBE_LEN) + 1,
       3 => rng.random_range(1..=40),
       _ => rng.random_range(1..=70_000),
   };
   ```



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to