refactor(encoding): align block codecs with miniblock - #8324
Conversation
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The codec-owned descriptor direction is worthwhile, but stable writers must preserve the selected compression configuration byte-for-byte. Keep that configuration with the selected codec and add exact descriptor regressions for omitted Zstd levels and explicit LZ4 levels.
Please mark this PR with the breaking-change label.
| Ok(LanceBuffer::from(compressed)) | ||
| Ok(( | ||
| Some(LanceBuffer::from(compressed)), | ||
| ProtobufUtils21::wrapped(self.compressor.config(), inner_encoding)?, |
There was a problem hiding this comment.
Serializing self.compressor.config() changes stable General descriptors from the selected configuration. With explicit Zstd and no level, construction normalizes None to runtime level 0 and this emits Some(0); LZ4 does the inverse for explicit levels because its runtime config() always returns None. Keep the originally selected CompressionConfig with this codec and serialize that exact value.
Reproducer
I ran this addable regression at 2228153208f842a2411316688c6c10916ce6b07b:
#[test]
#[cfg(feature = "zstd")]
fn gate_reproducer_general_block_preserves_absent_zstd_level() {
let mut params = CompressionParams::new();
params.columns.insert(
"dict_values".to_string(),
CompressionFieldParams {
compression: Some("zstd".to_string()),
compression_level: None,
..Default::default()
},
);
let strategy = strategy(TestEncoding::StructuralU32, params);
let field = create_test_field("dict_values", DataType::FixedSizeBinary(3));
let data = create_fixed_width_block(24, 1024);
let compressor = strategy.create_block_compressor(&field, &data).unwrap();
let (_, encoding) = compressor.compress(data).unwrap();
let Some(Compression::General(general)) = encoding.compression.as_ref() else {
panic!("expected general compression");
};
assert_eq!(general.compression.as_ref().unwrap().level, None);
}cargo test -p lance-encoding gate_reproducer_general_block_preserves_absent_zstd_level --lib
Expected None; observed Some(0).
Stack 1 of 10 for Lance generic block sequence compression. Builds on merged #8038.
This refactors block compression to follow the existing mini-block architecture: concrete codecs own descriptor construction, payload framing, and validation, while the shared strategy only dispatches. It removes the previous plan/factory shape so malformed or unsupported codec trees fail at the concrete codec boundary.
This layer introduces no protobuf variants or production selector changes. Stable Lance 2.0–2.2 bytes and reader behavior remain unchanged.
Stack navigation