Skip to content

Commit 61b8041

Browse files
vortex-claude[bot]github-actions[bot]robert3005claude
authored
fix(layout): stop dict writer looping on an oversized value (#10400)
Fixes #10397 A value larger than the dictionary byte budget did not fit an empty dictionary. The encoder then encoded 0 rows, and `DictStreamState::encode` retried the same chunk forever. The bytes and primitive dictionary builders now always admit the first entry, so each encode step makes progress. --------- Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Robert Kruszewski <github@robertk.io> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Robert Kruszewski <robert3005@users.noreply.github.com> Co-authored-by: Robert Kruszewski <github@robertk.io> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent b4cc1e2 commit 61b8041

3 files changed

Lines changed: 66 additions & 6 deletions

File tree

‎vortex-array/src/builders/dict/bytes.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,10 @@ impl<Code: UnsignedPType> BytesDictBuilder<Code> {
130130
) {
131131
HashTableEntry::Occupied(occupied) => Some(*occupied.get()),
132132
HashTableEntry::Vacant(vacant) => {
133-
if self.views.len() >= self.max_dict_len {
133+
// Admit the first entry even if it exceeds the budget, so the caller makes progress.
134+
let is_first_entry = self.views.is_empty();
135+
136+
if !is_first_entry && self.views.len() >= self.max_dict_len {
134137
return None;
135138
}
136139

@@ -146,7 +149,7 @@ impl<Code: UnsignedPType> BytesDictBuilder<Code> {
146149
size_of::<BinaryView>() + val.len()
147150
};
148151

149-
if self.dict_bytes() + additional_bytes > self.max_dict_bytes {
152+
if !is_first_entry && self.dict_bytes() + additional_bytes > self.max_dict_bytes {
150153
return None;
151154
}
152155

@@ -171,8 +174,9 @@ impl<Code: UnsignedPType> BytesDictBuilder<Code> {
171174
return Some(*code);
172175
}
173176

174-
if self.views.len() >= self.max_dict_len
175-
|| self.dict_bytes() + size_of::<BinaryView>() > self.max_dict_bytes
177+
if !self.views.is_empty()
178+
&& (self.views.len() >= self.max_dict_len
179+
|| self.dict_bytes() + size_of::<BinaryView>() > self.max_dict_bytes)
176180
{
177181
return None;
178182
}

‎vortex-array/src/builders/dict/primitive.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ where
101101
match self.lookup.entry(NativeValue(v)) {
102102
Entry::Occupied(o) => Some(*o.get()),
103103
Entry::Vacant(vac) => {
104-
if self.values.len() >= self.max_dict_len {
104+
// Admit the first entry even if it exceeds the budget, so the caller makes progress.
105+
if !self.values.is_empty() && self.values.len() >= self.max_dict_len {
105106
return None;
106107
}
107108
let next_code = Code::from_usize(self.values.len()).unwrap_or_else(|| {
@@ -121,7 +122,7 @@ where
121122
return Some(*code);
122123
}
123124

124-
if self.values.len() >= self.max_dict_len {
125+
if !self.values.is_empty() && self.values.len() >= self.max_dict_len {
125126
return None;
126127
}
127128

‎vortex-layout/src/layouts/dict/writer.rs‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,8 +597,12 @@ mod tests {
597597
use vortex_array::dtype::Nullability::NonNullable;
598598
use vortex_array::dtype::PType;
599599
use vortex_array::session::ArraySession;
600+
use vortex_error::VortexResult;
600601
use vortex_session::VortexSession;
601602

603+
use super::DictChunkLabeler;
604+
use super::DictStreamState;
605+
use super::DictionaryChunk;
602606
use super::DictionaryTransformer;
603607
use super::dict_encode_stream;
604608
use crate::sequence::SequenceId;
@@ -695,4 +699,55 @@ mod tests {
695699
"codes stream should use U16 dtype for dictionaries with >255 entries"
696700
);
697701
}
702+
703+
fn encoded_row_count(chunks: &[DictionaryChunk]) -> usize {
704+
chunks
705+
.iter()
706+
.map(|chunk| match chunk {
707+
DictionaryChunk::Codes { codes, .. } => codes.len(),
708+
DictionaryChunk::Values(_) => 0,
709+
})
710+
.sum()
711+
}
712+
713+
/// A 49-byte value plus its 16-byte `BinaryView` exceeds the 64-byte budget of an empty
714+
/// dictionary. Encoding must still make progress instead of looping forever.
715+
#[test]
716+
fn test_encode_value_larger_than_dictionary_budget_terminates() -> VortexResult<()> {
717+
let mut state = DictStreamState {
718+
encoder: None,
719+
constraints: DictConstraints {
720+
max_bytes: 64,
721+
max_len: 100,
722+
},
723+
};
724+
725+
let value = "x".repeat(49);
726+
let chunk = VarBinArray::from(vec![value.as_str()]).into_array();
727+
let mut labeler = DictChunkLabeler::new(SequenceId::root().advance());
728+
let encoded = state.encode(&mut labeler, chunk, &mut SESSION.create_execution_ctx())?;
729+
730+
assert_eq!(encoded_row_count(&encoded), 1);
731+
Ok(())
732+
}
733+
734+
#[test]
735+
fn test_encode_splits_oversized_value_into_its_own_dictionary() -> VortexResult<()> {
736+
let mut state = DictStreamState {
737+
encoder: None,
738+
constraints: DictConstraints {
739+
max_bytes: 64,
740+
max_len: 100,
741+
},
742+
};
743+
744+
let big = "x".repeat(49);
745+
let chunk = VarBinArray::from(vec!["a", big.as_str(), "b"]).into_array();
746+
let mut labeler = DictChunkLabeler::new(SequenceId::root().advance());
747+
let encoded = state.encode(&mut labeler, chunk, &mut SESSION.create_execution_ctx())?;
748+
let drained = state.drain_values(&mut labeler);
749+
750+
assert_eq!(encoded_row_count(&encoded) + encoded_row_count(&drained), 3);
751+
Ok(())
752+
}
698753
}

0 commit comments

Comments
 (0)