fix(format): read v1 chunk B-tree key offsets as 8 bytes
A type-1 (raw data chunk) B-tree key holds the chunk size, the filter mask and one offset per dimension, and those offsets are always 8 bytes: they are dataset coordinates, not file addresses. The reader used the superblock's size-of-offsets for them, so in a file with 4-byte offsets every key was misparsed. Unfiltered chunked datasets read as zeros (with stray bytes where a misread address landed on data) and filtered ones failed with "deflate: truncated stream". Only the sibling and child addresses follow size-of-offsets now. The unit-test B-tree builder wrote keys the same wrong way, which is why its tests passed; it now matches the format. Regression: h5py_four_byte_offsets_chunked_reads (h5py, set_sizes(4, 4) and (4, 8); 1-D and 2-D, unfiltered and gzip) and the unit test collect_chunks_with_four_byte_addresses. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
osobh
@@ -223,6 +223,10 @@ pub fn collect_chunk_info(
|
||||
collect_chunk_info_inner(file_data, btree_address, ndims, offset_size, length_size, 0)
|
||||
}
|
||||
|
||||
/// Width of each chunk offset in a v1 chunk B-tree key, independent of the
|
||||
/// file's size-of-offsets.
|
||||
const CHUNK_KEY_OFFSET_SIZE: u8 = 8;
|
||||
|
||||
/// Maximum recursion depth for chunk B-tree traversal (malformed/cyclic data
|
||||
/// protection), matching `btree_v1.rs`'s `MAX_BTREE_DEPTH`.
|
||||
const MAX_CHUNK_BTREE_DEPTH: usize = 64;
|
||||
@@ -260,8 +264,14 @@ fn collect_chunk_info_inner(
|
||||
|
||||
let mut pos = offset + 8 + os * 2; // skip left/right sibling
|
||||
|
||||
// Key size: chunk_size(4) + filter_mask(4) + ndims * offset_size
|
||||
let key_size = 4 + 4 + ndims * os;
|
||||
// Key: chunk_size(4) + filter_mask(4) + one offset per dimension. The
|
||||
// offsets are always 8 bytes each — they are dataset coordinates, not file
|
||||
// addresses, so they do not follow the superblock's size-of-offsets (only
|
||||
// the sibling and child addresses do).
|
||||
let key_size = ndims
|
||||
.checked_mul(CHUNK_KEY_OFFSET_SIZE as usize)
|
||||
.and_then(|n| n.checked_add(8))
|
||||
.ok_or_else(|| FormatError::ChunkedReadError("chunk key too large".into()))?;
|
||||
|
||||
if node_level == 0 {
|
||||
// Leaf node: keys and children interleaved
|
||||
@@ -287,8 +297,8 @@ fn collect_chunk_info_inner(
|
||||
let mut offsets = Vec::with_capacity(ndims);
|
||||
let mut kp = pos + 8;
|
||||
for _ in 0..ndims {
|
||||
offsets.push(read_offset(file_data, kp, offset_size)?);
|
||||
kp += os;
|
||||
offsets.push(read_offset(file_data, kp, CHUNK_KEY_OFFSET_SIZE)?);
|
||||
kp += CHUNK_KEY_OFFSET_SIZE as usize;
|
||||
}
|
||||
pos += key_size;
|
||||
|
||||
@@ -1592,7 +1602,8 @@ mod tests {
|
||||
} else {
|
||||
0
|
||||
};
|
||||
write_offset(&mut buf, off, offset_size);
|
||||
// Key offsets are always 8 bytes (they are coordinates).
|
||||
write_offset(&mut buf, off, 8);
|
||||
}
|
||||
// Child: address
|
||||
write_offset(&mut buf, chunk.address, offset_size);
|
||||
@@ -1602,7 +1613,7 @@ mod tests {
|
||||
buf.extend_from_slice(&0u32.to_le_bytes()); // chunk_size
|
||||
buf.extend_from_slice(&0u32.to_le_bytes()); // filter_mask
|
||||
for _ in 0..ndims {
|
||||
write_offset(&mut buf, u64::MAX, offset_size);
|
||||
write_offset(&mut buf, u64::MAX, 8);
|
||||
}
|
||||
|
||||
buf
|
||||
@@ -1680,6 +1691,37 @@ mod tests {
|
||||
assert_eq!(result[2].address, 0x300);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_chunks_with_four_byte_addresses() {
|
||||
// Sibling and child addresses are 4 bytes; the key offsets stay 8.
|
||||
let ndims = 3;
|
||||
let os: u8 = 4;
|
||||
let chunks = vec![
|
||||
ChunkInfo {
|
||||
chunk_size: 80,
|
||||
filter_mask: 2,
|
||||
offsets: vec![0, 5, 0],
|
||||
address: 0x1000,
|
||||
},
|
||||
ChunkInfo {
|
||||
chunk_size: 96,
|
||||
filter_mask: 0,
|
||||
offsets: vec![8, 10, 0],
|
||||
address: 0x2000,
|
||||
},
|
||||
];
|
||||
let btree = build_chunk_btree_leaf(&chunks, ndims, os);
|
||||
assert_eq!(btree.len(), 8 + 2 * 4 + 2 * (8 + 3 * 8 + 4) + (8 + 3 * 8));
|
||||
let result = collect_chunk_info(&btree, 0, ndims, os, os).unwrap();
|
||||
assert_eq!(result.len(), 2);
|
||||
for (got, want) in result.iter().zip(&chunks) {
|
||||
assert_eq!(got.offsets, want.offsets);
|
||||
assert_eq!(got.address, want.address);
|
||||
assert_eq!(got.chunk_size, want.chunk_size);
|
||||
assert_eq!(got.filter_mask, want.filter_mask);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_empty_btree() {
|
||||
let ndims = 2;
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
//! Chunked-read regressions against files written by h5py / libhdf5.
|
||||
//!
|
||||
//! Each test builds its input with h5py (or uses a small committed fixture
|
||||
//! when h5py cannot produce the feature) and compares clawhdf5's read with the
|
||||
//! known contents. Tests are skipped if python3 with h5py is not available,
|
||||
//! unless `CLAWHDF5_REQUIRE_INTEROP=1`.
|
||||
|
||||
use std::process::Command;
|
||||
|
||||
use clawhdf5::File;
|
||||
|
||||
/// The Python interpreter to drive interop checks with (see
|
||||
/// `h5py_interop_tests.rs`).
|
||||
fn python() -> String {
|
||||
std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string())
|
||||
}
|
||||
|
||||
fn interop_required() -> bool {
|
||||
std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1")
|
||||
}
|
||||
|
||||
fn python_available() -> bool {
|
||||
Command::new(python())
|
||||
.args(["-c", "import h5py, numpy"])
|
||||
.output()
|
||||
.map(|o| o.status.success())
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
macro_rules! skip_if_no_python {
|
||||
() => {
|
||||
if !python_available() {
|
||||
assert!(
|
||||
!interop_required(),
|
||||
"CLAWHDF5_REQUIRE_INTEROP=1 but python3 with h5py is not available"
|
||||
);
|
||||
eprintln!("SKIP: python3 with h5py not available");
|
||||
return;
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
fn run_python(script: &str) {
|
||||
let output = Command::new(python())
|
||||
.args(["-c", script])
|
||||
.output()
|
||||
.expect("failed to run python3");
|
||||
if !output.status.success() {
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
panic!("Python script failed:\nSTDOUT: {stdout}\nSTDERR: {stderr}");
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Files with 4-byte addresses (superblock size-of-offsets = 4)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// The chunk B-tree (v1, type 1) stores each chunk offset in its keys as a
|
||||
/// fixed 8-byte value whatever the file's size-of-offsets. Reading them with
|
||||
/// the offset width misparsed every key in a 4-byte-offset file: unfiltered
|
||||
/// datasets came back as zeros and filtered ones failed to inflate.
|
||||
#[test]
|
||||
fn h5py_four_byte_offsets_chunked_reads() {
|
||||
skip_if_no_python!();
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("sizes_4.h5");
|
||||
let p = path.display().to_string();
|
||||
run_python(&format!(
|
||||
r#"
|
||||
import h5py, numpy as np
|
||||
for name, lengths in (("{p}", 4), ("{p}.l8", 8)):
|
||||
fcpl = h5py.h5p.create(h5py.h5p.FILE_CREATE)
|
||||
fcpl.set_sizes(4, lengths)
|
||||
fid = h5py.h5f.create(name.encode(), h5py.h5f.ACC_TRUNC, fcpl=fcpl)
|
||||
with h5py.File(fid) as f:
|
||||
f.create_dataset("plain", data=np.arange(100.0), chunks=(10,))
|
||||
f.create_dataset("gzip", data=np.arange(100.0), chunks=(10,), compression="gzip")
|
||||
f.create_dataset("grid", data=np.arange(35 * 13, dtype="<i4").reshape(35, 13),
|
||||
chunks=(8, 5))
|
||||
f.create_dataset("grid_gzip", data=np.arange(35 * 13, dtype="<i4").reshape(35, 13),
|
||||
chunks=(8, 5), compression="gzip", shuffle=True)
|
||||
"#
|
||||
));
|
||||
|
||||
let expect: Vec<f64> = (0..100).map(f64::from).collect();
|
||||
let grid: Vec<i32> = (0..35 * 13).collect();
|
||||
for name in [p.clone(), format!("{p}.l8")] {
|
||||
let file = File::open(&name).unwrap();
|
||||
for ds in ["plain", "gzip"] {
|
||||
assert_eq!(
|
||||
file.dataset(ds).unwrap().read_f64().unwrap(),
|
||||
expect,
|
||||
"{name}:{ds}"
|
||||
);
|
||||
}
|
||||
for ds in ["grid", "grid_gzip"] {
|
||||
assert_eq!(
|
||||
file.dataset(ds).unwrap().read_i32().unwrap(),
|
||||
grid,
|
||||
"{name}:{ds}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user