Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "binseq"
version = "0.9.2"
version = "0.9.3"
edition = "2024"
description = "A high efficiency binary format for sequencing data"
license = "MIT"
Expand Down
8 changes: 7 additions & 1 deletion src/cbq/core/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ impl ColumnarBlock {
self.num_records = 0;
self.current_size = 0;
self.num_npos = 0;
self.len_nef = 0;
}

// clear spans
Expand Down Expand Up @@ -432,6 +433,11 @@ impl ColumnarBlock {
}

/// Decompress all columns back to native representation
///
/// Note: `resize` can be only be used with `copy_decode` if passing
/// as `&mut [T]`. Passing a resized `&mut Vec<T>` will lead to an
/// append operation, not an overwrite. If passing `&mut Vec<T>`, the
/// `Vec` will be resized automatically by `copy_decode`.
pub fn decompress_columns(&mut self) -> Result<()> {
// decompress sequence lengths
{
Expand All @@ -450,7 +456,7 @@ impl ColumnarBlock {

// decompress npos
if !self.z_npos.is_empty() {
self.ef_bytes.resize(self.len_nef, 0);
self.ef_bytes.reserve(self.len_nef);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To ensure defensive programming and idempotency, self.ef_bytes should be cleared before calling copy_decode. Since copy_decode takes &mut self.ef_bytes as impl std::io::Write, it appends the decompressed bytes to the vector. If self.ef_bytes is not cleared first, any pre-existing data in the vector (for example, if decompress_columns is called multiple times or if the buffer wasn't fully cleared) will remain at the beginning of the vector, causing EliasFano::deserialize_from to read stale/invalid data or fail.

Suggested change
self.ef_bytes.reserve(self.len_nef);
self.ef_bytes.clear();
self.ef_bytes.reserve(self.len_nef);

copy_decode(self.z_npos.as_slice(), &mut self.ef_bytes)?;

let ef = EliasFano::deserialize_from(self.ef_bytes.as_slice())?;
Expand Down
48 changes: 48 additions & 0 deletions src/cbq/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,54 @@ mod tests {
Ok(())
}

/// Sequences containing `N`s, which drive the Elias-Fano `npos` column.
/// Blocks with no `N`s at all never populate `z_npos`, so a suite built
/// only from `sample_sequences` (all ACGT) never exercises this path.
fn sample_sequences_with_n(n_seq: usize, seq_len: usize) -> Vec<Vec<u8>> {
const BASES: [u8; 4] = [b'A', b'C', b'G', b'T'];
(0..n_seq)
.map(|i| {
(0..seq_len)
.map(|j| {
if i % 5 == 0 {
b'N'
} else {
BASES[(i as usize + j) % 4]
}
})
.collect::<Vec<u8>>()
})
.collect()
}

/// Round-trips sequences containing `N`s through a `ColumnarBlockWriter`
/// and back through a `Reader`, exercising the Elias-Fano `npos`
/// decompression path in [`ColumnarBlock::decompress_columns`].
#[test]
fn test_roundtrip_sequences_with_n() -> Result<()> {
// Small block size so N-bearing sequences span multiple blocks.
let block_size = 256;
let mut writer = ColumnarBlockWriter::new(Vec::new(), header(block_size))?;

let seqs = sample_sequences_with_n(1024, 100);
assert!(
seqs.iter().any(|s| s.contains(&b'N')),
"test fixture must actually contain N's to exercise npos"
);
for seq in &seqs {
writer.push(record(seq))?;
}
writer.finish()?;

let read_back = read_all_sequences(writer.inner);
assert_eq!(
read_back, seqs,
"round-trip mismatch for N-bearing sequences"
);

Ok(())
}

/// `ingest_completed` on a source with no completed blocks is a no-op for
/// the global writer and preserves the source's incomplete block.
#[test]
Expand Down
Loading