Skip to content

Commit a542cbd

Browse files
authored
Remove DeltaScheme from default BtrBlocksCompressor (#9967)
This should be included once we add filtering Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent fdc54cd commit a542cbd

4 files changed

Lines changed: 27 additions & 7 deletions

File tree

‎vortex-btrblocks/src/builder.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,7 @@ pub const ALL_SCHEMES: &[&dyn Scheme] = &[
3636
&integer::RunEndScheme,
3737
&integer::SequenceScheme,
3838
&integer::IntRLEScheme,
39-
// Prefer all other schemes above delta, for now (since its slower to decompress).
40-
&integer::DeltaScheme::new(1.25),
39+
// Delta is omitted here: see [`DELTA_SCHEME`].
4140
////////////////////////////////////////////////////////////////////////////////////////////////
4241
// Float schemes.
4342
////////////////////////////////////////////////////////////////////////////////////////////////
@@ -66,6 +65,13 @@ pub const ALL_SCHEMES: &[&dyn Scheme] = &[
6665
&temporal::TemporalScheme,
6766
];
6867

68+
/// Delta, kept out of [`ALL_SCHEMES`] because it is slower to decompress than the schemes that
69+
/// would otherwise win. Callers that want it opt in with
70+
/// [`with_new_scheme`](BtrBlocksCompressorBuilder::with_new_scheme).
71+
///
72+
/// TODO(robert): Return it to [`ALL_SCHEMES`] once we have scheme filtering.
73+
pub static DELTA_SCHEME: integer::DeltaScheme = integer::DeltaScheme::new(1.25);
74+
6975
/// Builder for creating configured [`BtrBlocksCompressor`] instances.
7076
///
7177
/// By default, all schemes in [`ALL_SCHEMES`] are enabled in a deterministic order. Feature-gated

‎vortex-btrblocks/src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ mod trace_tests;
7878
// Btrblocks-specific exports.
7979
pub use builder::ALL_SCHEMES;
8080
pub use builder::BtrBlocksCompressorBuilder;
81+
pub use builder::DELTA_SCHEME;
8182
pub use canonical_compressor::BtrBlocksCompressor;
8283
pub use schemes::patches::compress_patches;
8384
pub use vortex_compressor::CascadingCompressor;

‎vortex-btrblocks/src/schemes/integer/scheme_selection_tests.rs‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ use vortex_session::VortexSession;
2828
use vortex_sparse::Sparse;
2929

3030
use crate::BtrBlocksCompressor;
31+
use crate::BtrBlocksCompressorBuilder;
32+
use crate::DELTA_SCHEME;
3133
static SESSION: LazyLock<VortexSession> = LazyLock::new(vortex_array::array_session);
3234

3335
#[test]
@@ -174,7 +176,9 @@ fn test_delta_compressed() -> VortexResult<()> {
174176
.collect();
175177
let array = PrimitiveArray::new(Buffer::copy_from(&values), Validity::NonNullable);
176178

177-
let btr = BtrBlocksCompressor::default();
179+
let btr = BtrBlocksCompressorBuilder::default()
180+
.with_new_scheme(&DELTA_SCHEME)
181+
.build();
178182
let compressed = btr.compress(
179183
&array.clone().into_array(),
180184
&mut SESSION.create_execution_ctx(),
@@ -212,7 +216,9 @@ fn test_delta_compressed_unaligned_length() -> VortexResult<()> {
212216
.collect();
213217
let array = PrimitiveArray::new(Buffer::copy_from(&values), Validity::NonNullable);
214218

215-
let btr = BtrBlocksCompressor::default();
219+
let btr = BtrBlocksCompressorBuilder::default()
220+
.with_new_scheme(&DELTA_SCHEME)
221+
.build();
216222
let compressed = btr.compress(
217223
&array.clone().into_array(),
218224
&mut SESSION.create_execution_ctx(),
@@ -239,7 +245,9 @@ fn test_delta_nullable_unaligned_sum() -> VortexResult<()> {
239245
let array =
240246
PrimitiveArray::from_option_iter(iter::once(None).chain((1i32..=100_000).map(Some)));
241247

242-
let btr = BtrBlocksCompressor::default();
248+
let btr = BtrBlocksCompressorBuilder::default()
249+
.with_new_scheme(&DELTA_SCHEME)
250+
.build();
243251
let compressed = btr.compress(&array.clone().into_array(), &mut ctx)?;
244252
assert!(
245253
compressed.is::<Delta>(),

‎vortex-btrblocks/src/trace_tests.rs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ use vortex_error::VortexResult;
5353
use vortex_mask::Mask;
5454
use vortex_session::VortexSession;
5555

56-
use crate::BtrBlocksCompressor;
56+
use crate::BtrBlocksCompressorBuilder;
57+
use crate::DELTA_SCHEME;
5758

5859
/// A session with the default Vortex encodings registered.
5960
///
@@ -124,8 +125,12 @@ fn lineitem() -> VortexResult<ArrayRef> {
124125
.from_arrow_record_batch(batch, &schema)
125126
}
126127

128+
/// Delta is opt-in, and these traces cover the delta-encoded FSST offsets, so enable it here.
127129
fn compressed_lineitem() -> VortexResult<ArrayRef> {
128-
BtrBlocksCompressor::default().compress(&lineitem()?, &mut execution_ctx())
130+
BtrBlocksCompressorBuilder::default()
131+
.with_new_scheme(&DELTA_SCHEME)
132+
.build()
133+
.compress(&lineitem()?, &mut execution_ctx())
129134
}
130135

131136
fn field(array: &ArrayRef, name: &str) -> VortexResult<ArrayRef> {

0 commit comments

Comments
 (0)