Skip to content

Commit f9f09a0

Browse files
rapourclaude
andcommitted
docs: tighten the Elias-Fano binding's comments
Shorten the comments added with the recent fixes and benchmark resize. The low-bits width check no longer claims that a wider slot bleeds into the high part: the codec masks low bits on both read paths, so the real risk is low parts silently masked off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: rapour <reza.apour@gmail.com>
1 parent fab1b57 commit f9f09a0

4 files changed

Lines changed: 11 additions & 17 deletions

File tree

‎encodings/elias-fano/benches/elias_fano.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,8 @@ fn encoded(shape: Shape) -> EliasFanoArray {
8787
elias_fano_encode(array.as_ref().as_::<Primitive>(), &mut ctx).unwrap()
8888
}
8989

90-
/// Probe count held fixed across shapes, so the reported figure is comparable. Each probe runs a
91-
/// whole `execute_scalar`, so this is a quarter of the codec benchmark's count to stay well inside
92-
/// the per-iteration budget.
90+
/// Probes per iteration, fixed across shapes so the figures compare. Each runs a whole
91+
/// `execute_scalar`, hence a quarter of the codec benchmark's count.
9392
const PROBES: usize = 1024;
9493

9594
fn random_indices() -> Vec<usize> {

‎encodings/elias-fano/src/array.rs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -610,11 +610,9 @@ fn validate_parts(
610610
"Elias-Fano low-bits child must be {LOWER_DTYPE}, got {}",
611611
lower.dtype()
612612
);
613-
// A bit-packed slot's global width is metadata, so this is free to check and is the only part
614-
// of the low bits checkable at all. Narrower is legal — a rewrite may repack tighter. Wider is
615-
// not: the reader ORs the low bits in under `lower_width`, so anything above bleeds into the
616-
// high part. Blocked widths live in a child array rather than metadata, so a blocked slot is
617-
// accepted like any other integer child.
613+
// A globally packed slot's width is metadata, so it is free to check. Narrower is legal, since
614+
// a rewrite may repack tighter; wider could hold low parts the reader would mask off. Blocked
615+
// widths live in a child array, so a blocked slot is accepted like any other integer child.
618616
if let Some(packed) = lower.as_opt::<BitPacked>()
619617
&& let BitWidthsView::Global(bit_width) = packed.bit_widths()
620618
{

‎encodings/elias-fano/src/compress.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,8 @@ pub(crate) fn elias_fano_decompress(
137137
let start = ef::position_of_rank(bits, samples1, first_rank).map_err(malformed)?;
138138
let end =
139139
ef::position_of_rank(bits, samples1, first_rank + len as u64 - 1).map_err(malformed)? + 1;
140-
// Validation checks the sample table's ranges and order, not its agreement with the upper
141-
// array, so a corrupt table can place the last rank's bit before the first's. Raise rather than
142-
// let `window_words` assert on the reversed window.
140+
// Validation does not check samples against the upper array, so a corrupt table can put the
141+
// last rank before the first. Raise instead of letting `window_words` assert.
143142
vortex_ensure!(
144143
start < end,
145144
"Elias-Fano sample table disagrees with the upper array"

‎encodings/elias-fano/src/tests.rs‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -670,17 +670,15 @@ fn test_rejects_corrupt_samples(#[case] poison: u64) -> VortexResult<()> {
670670
Ok(())
671671
}
672672

673-
/// A one-sample that is in range and in order passes validation even when it points at the wrong
674-
/// bit, since samples are not checked against the upper array. Decoding a slice across it then
675-
/// finds its last rank's bit before its first's, which must raise rather than reach the assert in
676-
/// `window_words`.
673+
/// A misplaced one-sample passes validation, since samples are not checked against the upper
674+
/// array. Decoding a slice across it must raise, not panic.
677675
#[test]
678676
fn test_decompress_raises_on_misplaced_sample() -> VortexResult<()> {
679677
let encoded = encode(&sorted_values(5000, 1 << 30, 0xDEFACED))?;
680678
let mut ctx = SESSION.create_execution_ctx();
681679

682-
// The first one-sample stands for rank 256. Its lowest legal position is 257, well below where
683-
// rank 256 really sits in a sparse sequence, so a select for 256 now lands on an earlier bit.
680+
// The first one-sample stands for rank 256; 257 is its lowest legal position, well before
681+
// rank 256's real bit.
684682
let seam = usize::try_from(encoded.num_samples0())? * size_of::<u64>();
685683
let mut poisoned = encoded.samples_buffer().clone().into_mut();
686684
poisoned[seam..seam + size_of::<u64>()].copy_from_slice(&257u64.to_le_bytes());

0 commit comments

Comments
 (0)