Skip to content

Commit 0310216

Browse files
committed
style(compress): clarify stored width selection contracts
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
1 parent c7776c9 commit 0310216

7 files changed

Lines changed: 28 additions & 10 deletions

File tree

‎vortex-array/src/arrays/narrow/encoding.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,10 @@ impl NarrowArray {
5656
if !ptype.is_int() || ptype.byte_width() == 1 {
5757
return Ok(array);
5858
}
59+
5960
let bounds = min_max(array.as_ref(), ctx, NumericalAggregateOpts::default())?;
6061
let storage_type = if let Some(bounds) = bounds {
61-
let smallest = if ptype.is_signed_int() {
62+
let (min_type, max_type) = if ptype.is_signed_int() {
6263
(
6364
PType::min_signed_ptype_for_value(i64::try_from(&bounds.min)?),
6465
PType::min_signed_ptype_for_value(i64::try_from(&bounds.max)?),
@@ -69,16 +70,17 @@ impl NarrowArray {
6970
PType::min_unsigned_ptype_for_value(u64::try_from(&bounds.max)?),
7071
)
7172
};
72-
if smallest.0.byte_width() >= smallest.1.byte_width() {
73-
smallest.0
73+
if min_type.byte_width() >= max_type.byte_width() {
74+
min_type
7475
} else {
75-
smallest.1
76+
max_type
7677
}
7778
} else if ptype.is_signed_int() {
7879
PType::I8
7980
} else {
8081
PType::U8
8182
};
83+
8284
if storage_type.byte_width() >= ptype.byte_width() {
8385
return Ok(array);
8486
}

‎vortex-btrblocks/tests/narrow.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

44
//! Preview gating and codec cascades retain Narrow's logical dtype on the wire.
5+
//!
6+
//! Core and CUDA presets exclude Narrow wrappers. Serialization round trips also cover signed
7+
//! internal buffers whose dtypes are selected by the compressor.
58
69
#![cfg(test)]
710

@@ -57,7 +60,7 @@ fn session(preview: bool) -> VortexResult<VortexSession> {
5760
#[rstest]
5861
#[case::core(false)]
5962
#[case::preview(true)]
60-
fn compression_roundtrip(#[case] preview: bool) -> VortexResult<()> {
63+
fn test_compression_roundtrip(#[case] preview: bool) -> VortexResult<()> {
6164
let session = session(preview)?;
6265
let mut ctx = session.create_execution_ctx();
6366
let input = PrimitiveArray::from_iter((0..8192u64).map(|i| i % 128)).into_array();
@@ -95,7 +98,7 @@ fn compression_roundtrip(#[case] preview: bool) -> VortexResult<()> {
9598
}
9699

97100
#[test]
98-
fn cuda_preset_excludes_narrow() -> VortexResult<()> {
101+
fn test_cuda_preset_excludes_narrow() -> VortexResult<()> {
99102
let session = session(true)?;
100103
let mut ctx = session.create_execution_ctx();
101104
let input = PrimitiveArray::from_iter((0..8192u64).map(|i| i % 128)).into_array();
@@ -110,7 +113,7 @@ fn cuda_preset_excludes_narrow() -> VortexResult<()> {
110113
}
111114

112115
#[test]
113-
fn internal_signed_buffers_roundtrip() -> VortexResult<()> {
116+
fn test_internal_signed_buffers_roundtrip() -> VortexResult<()> {
114117
let session = session(false)?;
115118
vortex_fsst::initialize(&session);
116119
#[cfg(feature = "pco")]

‎vortex-compressor/src/compressor/cascade.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ impl CascadingCompressor {
9393
{
9494
return NarrowArray::encode(primitive.into_owned(), exec_ctx);
9595
}
96+
9697
return Ok(child.clone());
9798
}
9899

@@ -134,11 +135,13 @@ impl CascadingCompressor {
134135
if compressed.dtype() == &dtype {
135136
return Ok(compressed);
136137
}
138+
137139
if let Some(constant) = compressed.as_opt::<Constant>() {
138140
return Ok(
139141
ConstantArray::new(constant.scalar().cast(&dtype)?, len).into_array()
140142
);
141143
}
144+
142145
Ok(NarrowArray::try_new(compressed, dtype)?.into_array())
143146
}
144147
Canonical::Decimal(decimal) => {

‎vortex-compressor/src/compressor/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ impl CascadingCompressor {
8181
/// disabled by default and does not consume a level of the codec cascade budget.
8282
pub fn with_narrow_integers(mut self, enabled: bool) -> Self {
8383
self.narrow_integers = enabled;
84+
8485
self
8586
}
8687
}

‎vortex-compressor/src/compressor/tests/mod.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

44
//! Compression selection, cascades, and logical dtype preservation.
5+
//!
6+
//! Scheme fixtures isolate selection and exclusion rules. Narrow cases separately check the
7+
//! optional width reduction before codec selection.
58
69
mod narrow;
710

‎vortex-compressor/src/compressor/tests/narrow.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

44
//! Integer narrowing precedes codec selection and preserves the input dtype.
5+
//!
6+
//! These cases use a compressor without schemes to isolate width selection. Constants and values
7+
//! that need the full logical width exercise paths that do not require a Narrow wrapper.
58
69
use rstest::rstest;
710
use vortex_array::IntoArray;
@@ -21,7 +24,7 @@ use crate::CascadingCompressor;
2124
#[rstest]
2225
#[case::disabled(false)]
2326
#[case::enabled(true)]
24-
fn integer_width(#[case] enabled: bool) -> VortexResult<()> {
27+
fn test_integer_width(#[case] enabled: bool) -> VortexResult<()> {
2528
let mut ctx = array_session().create_execution_ctx();
2629
let input = buffer![-128i64, 0, 127].into_array();
2730
let compressor = CascadingCompressor::new(vec![]).with_narrow_integers(enabled);
@@ -44,7 +47,7 @@ fn integer_width(#[case] enabled: bool) -> VortexResult<()> {
4447
#[rstest]
4548
#[case::constant(PrimitiveArray::from_iter([127i64; 1024]))]
4649
#[case::all_null(PrimitiveArray::from_option_iter([None::<i64>; 3]))]
47-
fn constant_dtype(#[case] input: PrimitiveArray) -> VortexResult<()> {
50+
fn test_constant_dtype(#[case] input: PrimitiveArray) -> VortexResult<()> {
4851
let mut ctx = array_session().create_execution_ctx();
4952
let input = input.into_array();
5053
let compressor = CascadingCompressor::new(vec![]).with_narrow_integers(true);
@@ -58,7 +61,7 @@ fn constant_dtype(#[case] input: PrimitiveArray) -> VortexResult<()> {
5861
}
5962

6063
#[test]
61-
fn full_width_values() -> VortexResult<()> {
64+
fn test_full_width_values() -> VortexResult<()> {
6265
let mut ctx = array_session().create_execution_ctx();
6366
let input = buffer![i64::MIN, i64::MAX].into_array();
6467
let result = CascadingCompressor::new(vec![])

‎vortex-edition/src/declarations/preview/v2026_10.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

44
//! The October 2026 preview edition adding Narrow integer arrays.
5+
//!
6+
//! This declaration permits Narrow on the wire for sessions that enable it. Earlier preview
7+
//! and core editions retain their existing membership.
58
69
use crate::Edition;
710
use crate::EditionDeclaration;

0 commit comments

Comments
 (0)