Skip to content

Commit 71ff5e5

Browse files
authored
fix(encoding): tolerate legacy RLE chunks that overflow the declared value count (#7708)
#7376 made the RLE miniblock decoder reject chunks whose run lengths sum past the declared value count. The legacy (pre run-length-width) miniblock encoder rolled back to a power-of-2 checkpoint after a run had already crossed it, so existing files contain chunks that declare 2048 values but hold runs summing slightly more; the excess values are re-encoded at the start of the next chunk. The legacy decoder truncated the excess, so those files were valid when written — after #7376 they fail to read with: ``` Invalid user input: RLE decoding overflowed expected value count: produced at least 2080, expected 2048 ``` This restores the truncating behavior for miniblock decoding: clamp the final crossing run at the declared count and ignore trailing legacy runs, while still rejecting underflow and zero run lengths. Block payloads have no chunk boundaries, so an overflow there can only be corruption and remains a hard error. Current encoders emit exact chunks, so this only changes how legacy files are read. Includes regression tests for the legacy chunk-boundary overflow and the strict block path. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved run-length (RLE) decoding to cap results to the expected number of values, avoiding failures when runs exceed the requested range. * Enhanced compatibility with legacy data by clamping miniblock overflow at chunk boundaries and ensuring decoded output matches the expected length and values. * Updated miniblock test coverage to reflect the new overflow semantics, and added a regression test for boundary-crossing truncation. * Preserved strict overflow handling for full block payloads, which still reports an error. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent dda6cab commit 71ff5e5

1 file changed

Lines changed: 130 additions & 35 deletions

File tree

  • rust/lance-encoding/src/encodings/physical

rust/lance-encoding/src/encodings/physical/rle.rs

Lines changed: 130 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,7 +1281,12 @@ impl RleDecompressor {
12811281
}
12821282
}
12831283

1284-
fn decode_data(&self, data: Vec<LanceBuffer>, num_values: u64) -> Result<DataBlock> {
1284+
fn decode_data(
1285+
&self,
1286+
data: Vec<LanceBuffer>,
1287+
num_values: u64,
1288+
clamp_overflow: bool,
1289+
) -> Result<DataBlock> {
12851290
if num_values == 0 {
12861291
return Ok(DataBlock::FixedWidth(FixedWidthDataBlock {
12871292
bits_per_value: self.bits_per_value,
@@ -1308,10 +1313,30 @@ impl RleDecompressor {
13081313
self.decode_child_buffers(values_buffer, lengths_buffer)?;
13091314

13101315
let decoded_data = match self.bits_per_value {
1311-
8 => self.decode_generic::<u8>(&values_buffer, &lengths_buffer, num_values)?,
1312-
16 => self.decode_generic::<u16>(&values_buffer, &lengths_buffer, num_values)?,
1313-
32 => self.decode_generic::<u32>(&values_buffer, &lengths_buffer, num_values)?,
1314-
64 => self.decode_generic::<u64>(&values_buffer, &lengths_buffer, num_values)?,
1316+
8 => self.decode_generic::<u8>(
1317+
&values_buffer,
1318+
&lengths_buffer,
1319+
num_values,
1320+
clamp_overflow,
1321+
)?,
1322+
16 => self.decode_generic::<u16>(
1323+
&values_buffer,
1324+
&lengths_buffer,
1325+
num_values,
1326+
clamp_overflow,
1327+
)?,
1328+
32 => self.decode_generic::<u32>(
1329+
&values_buffer,
1330+
&lengths_buffer,
1331+
num_values,
1332+
clamp_overflow,
1333+
)?,
1334+
64 => self.decode_generic::<u64>(
1335+
&values_buffer,
1336+
&lengths_buffer,
1337+
num_values,
1338+
clamp_overflow,
1339+
)?,
13151340
_ => {
13161341
return Err(Error::invalid_input_source(
13171342
format!(
@@ -1398,6 +1423,7 @@ impl RleDecompressor {
13981423
values_buffer: &LanceBuffer,
13991424
lengths_buffer: &LanceBuffer,
14001425
num_values: u64,
1426+
clamp_overflow: bool,
14011427
) -> Result<LanceBuffer>
14021428
where
14031429
T: bytemuck::Pod + Copy + std::fmt::Debug + ArrowNativeType,
@@ -1450,8 +1476,21 @@ impl RleDecompressor {
14501476
format!("RLE num_values does not fit in usize: {num_values}").into(),
14511477
)
14521478
})?;
1453-
let mut decoded_value_count = 0usize;
1454-
for length_bytes in lengths.chunks_exact(length_size) {
1479+
// Legacy miniblock encoders rolled back to a power-of-2 checkpoint after a run
1480+
// had already crossed it, so a chunk's run lengths can sum past its declared
1481+
// value count (the excess values are re-encoded at the start of the next chunk).
1482+
// The pre-run-length-width decoder truncated the excess, so miniblock decoding
1483+
// clamps rather than rejects to keep those files readable. Block payloads never
1484+
// legitimately overflow, so they decode strictly.
1485+
let mut decoded: Vec<T> = Vec::new();
1486+
decoded
1487+
.try_reserve_exact(expected_value_count)
1488+
.map_err(|_| {
1489+
Error::invalid_input_source(
1490+
format!("RLE decoding cannot allocate {expected_value_count} values").into(),
1491+
)
1492+
})?;
1493+
for (value, length_bytes) in values.iter().zip(lengths.chunks_exact(length_size)) {
14551494
let length = self.run_length_width.read_length(length_bytes);
14561495
if length == 0 {
14571496
return Err(Error::invalid_input_source(
@@ -1463,36 +1502,35 @@ impl RleDecompressor {
14631502
format!("RLE run length does not fit in usize: {length}").into(),
14641503
)
14651504
})?;
1466-
decoded_value_count = decoded_value_count.checked_add(length).ok_or_else(|| {
1467-
Error::invalid_input_source("RLE run length sum overflowed usize".into())
1468-
})?;
1469-
if decoded_value_count > expected_value_count {
1470-
return Err(Error::invalid_input_source(
1471-
format!(
1472-
"RLE decoding overflowed expected value count: produced at least {}, expected {}",
1473-
decoded_value_count, expected_value_count
1474-
)
1475-
.into(),
1476-
));
1505+
let remaining = expected_value_count - decoded.len();
1506+
if length > remaining {
1507+
if !clamp_overflow {
1508+
return Err(Error::invalid_input_source(
1509+
format!(
1510+
"RLE decoding overflowed expected value count: produced at least {}, expected {}",
1511+
decoded.len() + length,
1512+
expected_value_count
1513+
)
1514+
.into(),
1515+
));
1516+
}
1517+
decoded.resize(expected_value_count, *value);
1518+
break;
14771519
}
1520+
decoded.resize(decoded.len() + length, *value);
14781521
}
14791522

1480-
if decoded_value_count != expected_value_count {
1523+
if decoded.len() != expected_value_count {
14811524
return Err(Error::invalid_input_source(
14821525
format!(
14831526
"RLE decoding produced {} values, expected {}",
1484-
decoded_value_count, expected_value_count
1527+
decoded.len(),
1528+
expected_value_count
14851529
)
14861530
.into(),
14871531
));
14881532
}
14891533

1490-
let mut decoded: Vec<T> = Vec::with_capacity(expected_value_count);
1491-
for (value, length_bytes) in values.iter().zip(lengths.chunks_exact(length_size)) {
1492-
let length = self.run_length_width.read_length(length_bytes) as usize;
1493-
decoded.resize(decoded.len() + length, *value);
1494-
}
1495-
14961534
trace!(
14971535
"RLE decoded {} {} values",
14981536
num_values,
@@ -1504,7 +1542,7 @@ impl RleDecompressor {
15041542

15051543
impl MiniBlockDecompressor for RleDecompressor {
15061544
fn decompress(&self, data: Vec<LanceBuffer>, num_values: u64) -> Result<DataBlock> {
1507-
self.decode_data(data, num_values)
1545+
self.decode_data(data, num_values, true)
15081546
}
15091547
}
15101548

@@ -1541,7 +1579,7 @@ impl BlockDecompressor for RleDecompressor {
15411579
let values_buffer = data.slice_with_length(values_start, values_size);
15421580
let lengths_buffer = data.slice_with_length(lengths_start, data.len() - lengths_start);
15431581

1544-
self.decode_data(vec![values_buffer, lengths_buffer], num_values)
1582+
self.decode_data(vec![values_buffer, lengths_buffer], num_values, false)
15451583
}
15461584
}
15471585

@@ -2096,7 +2134,7 @@ mod tests {
20962134
}
20972135

20982136
#[test]
2099-
fn test_rle_rejects_underflow_overflow_and_zero_lengths() {
2137+
fn test_rle_rejects_underflow_and_zero_lengths_and_clamps_overflow() {
21002138
let decompressor = RleDecompressor::with_run_length_width(32, RunLengthWidth::U16);
21012139
let value = LanceBuffer::from(1i32.to_le_bytes().to_vec());
21022140

@@ -2119,12 +2157,15 @@ mod tests {
21192157
],
21202158
5,
21212159
)
2122-
.unwrap_err();
2123-
assert!(
2124-
overflow
2125-
.to_string()
2126-
.contains("overflowed expected value count")
2127-
);
2160+
.unwrap();
2161+
match overflow {
2162+
DataBlock::FixedWidth(block) => {
2163+
assert_eq!(block.num_values, 5);
2164+
let decoded = block.data.borrow_to_typed_slice::<i32>();
2165+
assert_eq!(decoded.as_ref(), &[1i32; 5]);
2166+
}
2167+
_ => panic!("Expected FixedWidth block"),
2168+
}
21282169

21292170
let zero = MiniBlockDecompressor::decompress(
21302171
&decompressor,
@@ -2135,6 +2176,60 @@ mod tests {
21352176
assert!(zero.to_string().contains("zero run length"));
21362177
}
21372178

2179+
#[test]
2180+
fn test_block_rle_rejects_overflow() {
2181+
// Block payloads have no chunk boundaries, so run lengths summing past
2182+
// num_values can only be corruption and must stay a hard error.
2183+
let decompressor = RleDecompressor::with_run_length_width(32, RunLengthWidth::U16);
2184+
let values = 1i32.to_le_bytes();
2185+
let lengths = 6u16.to_le_bytes();
2186+
let mut payload = Vec::new();
2187+
payload.extend_from_slice(&(values.len() as u64).to_le_bytes());
2188+
payload.extend_from_slice(&values);
2189+
payload.extend_from_slice(&lengths);
2190+
2191+
let error = BlockDecompressor::decompress(&decompressor, LanceBuffer::from(payload), 5)
2192+
.unwrap_err();
2193+
assert!(matches!(&error, Error::InvalidInput { .. }));
2194+
assert!(
2195+
error
2196+
.to_string()
2197+
.contains("overflowed expected value count")
2198+
);
2199+
}
2200+
2201+
#[test]
2202+
fn test_rle_truncates_legacy_chunk_boundary_overflow() {
2203+
// Legacy encoders emitted chunks declaring 2048 values whose final run crossed
2204+
// the checkpoint boundary (e.g. run lengths summing to 2080); the excess values
2205+
// are duplicated at the start of the next chunk and must be ignored here.
2206+
let decompressor = RleDecompressor::with_run_length_width(32, RunLengthWidth::U16);
2207+
let mut values = Vec::new();
2208+
values.extend_from_slice(&7i32.to_le_bytes());
2209+
values.extend_from_slice(&8i32.to_le_bytes());
2210+
let mut lengths = Vec::new();
2211+
lengths.extend_from_slice(&2000u16.to_le_bytes());
2212+
lengths.extend_from_slice(&80u16.to_le_bytes());
2213+
2214+
let decoded = MiniBlockDecompressor::decompress(
2215+
&decompressor,
2216+
vec![LanceBuffer::from(values), LanceBuffer::from(lengths)],
2217+
2048,
2218+
)
2219+
.unwrap();
2220+
match decoded {
2221+
DataBlock::FixedWidth(block) => {
2222+
assert_eq!(block.num_values, 2048);
2223+
let decoded = block.data.borrow_to_typed_slice::<i32>();
2224+
let decoded = decoded.as_ref();
2225+
assert_eq!(decoded.len(), 2048);
2226+
assert!(decoded[..2000].iter().all(|&v| v == 7));
2227+
assert!(decoded[2000..].iter().all(|&v| v == 8));
2228+
}
2229+
_ => panic!("Expected FixedWidth block"),
2230+
}
2231+
}
2232+
21382233
#[test]
21392234
fn test_empty_data_handling() {
21402235
let encoder = RleEncoder::new();

0 commit comments

Comments
 (0)