Skip to content

Commit aa5d235

Browse files
authored
Fix: missing patches in alp decompress (#6007)
Fixes: #5948 Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
1 parent 6df2004 commit aa5d235

File tree

2 files changed

+72
-1
lines changed

2 files changed

+72
-1
lines changed

encodings/alp/src/alp/array.rs

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,4 +737,73 @@ mod tests {
737737
}
738738
}
739739
}
740+
741+
/// Regression test for issue #5948: execute_decompress drops patches when chunk_offsets is
742+
/// None.
743+
///
744+
/// When patches exist but do NOT have chunk_offsets, the execute path incorrectly passes
745+
/// `None` to `decompress_unchunked_core` instead of the actual patches.
746+
///
747+
/// This can happen after file IO serialization/deserialization where chunk_offsets may not
748+
/// be preserved, or when building ALPArrays manually without chunk_offsets.
749+
#[test]
750+
fn test_execute_decompress_with_patches_no_chunk_offsets_regression_5948() {
751+
// Create an array with values that will produce patches. PI doesn't encode cleanly.
752+
let values: Vec<f64> = vec![1.0, 2.0, PI, 4.0, 5.0];
753+
let original = PrimitiveArray::from_iter(values);
754+
755+
// First encode normally to get a properly formed ALPArray with patches.
756+
let normally_encoded = alp_encode(&original, None).unwrap();
757+
assert!(
758+
normally_encoded.patches().is_some(),
759+
"Test requires patches to be present"
760+
);
761+
762+
let original_patches = normally_encoded.patches().unwrap();
763+
assert!(
764+
original_patches.chunk_offsets().is_some(),
765+
"Normal encoding should have chunk_offsets"
766+
);
767+
768+
// Rebuild the patches WITHOUT chunk_offsets to simulate deserialized patches.
769+
let patches_without_chunk_offsets = Patches::new(
770+
original_patches.array_len(),
771+
original_patches.offset(),
772+
original_patches.indices().clone(),
773+
original_patches.values().clone(),
774+
None, // NO chunk_offsets - this triggers the bug!
775+
);
776+
777+
// Build a new ALPArray with the same encoded data but patches without chunk_offsets.
778+
let alp_without_chunk_offsets = ALPArray::new(
779+
normally_encoded.encoded().clone(),
780+
normally_encoded.exponents(),
781+
Some(patches_without_chunk_offsets),
782+
);
783+
784+
// The legacy decompress_into_array path should work correctly.
785+
let result_legacy = decompress_into_array(alp_without_chunk_offsets.clone());
786+
let legacy_slice = result_legacy.as_slice::<f64>();
787+
788+
// Verify the legacy path produces correct values.
789+
assert!(
790+
(legacy_slice[2] - PI).abs() < 1e-10,
791+
"Legacy path should have PI at index 2, got {}",
792+
legacy_slice[2]
793+
);
794+
795+
// The execute path has the bug - it drops patches when chunk_offsets is None.
796+
let result_execute = {
797+
let mut ctx = SESSION.create_execution_ctx();
798+
execute_decompress(alp_without_chunk_offsets, &mut ctx).unwrap()
799+
};
800+
let execute_slice = result_execute.as_slice::<f64>();
801+
802+
// This assertion FAILS until the bug is fixed because execute_decompress drops patches.
803+
assert!(
804+
(execute_slice[2] - PI).abs() < 1e-10,
805+
"Execute path should have PI at index 2, but got {} (patches were dropped!)",
806+
execute_slice[2]
807+
);
808+
}
740809
}

encodings/alp/src/alp/decompress.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,9 @@ pub fn execute_decompress(array: ALPArray, ctx: &mut ExecutionCtx) -> VortexResu
8686
))
8787
} else {
8888
let encoded = encoded.execute::<PrimitiveArray>(ctx)?;
89-
Ok(decompress_unchunked_core(encoded, exponents, None, dtype))
89+
Ok(decompress_unchunked_core(
90+
encoded, exponents, patches, dtype,
91+
))
9092
}
9193
}
9294

0 commit comments

Comments
 (0)