Repository navigation
fix(dict-writer): stop DictStreamState::encode from looping on values that overflow an empty dictionary - #10427
Closed
wahajahmed010 wants to merge 1 commit into
Conversation
… that overflow an empty dictionary When every value in a chunk was longer than `max_bytes - 16` (the most an empty dictionary can hold once the 16-byte `BinaryView` header is included), `BytesDictBuilder::encode_value` returned `None` for every row. The dict writer then encoded 0 rows, handed the same chunk back to itself via `remainder`, and looped forever while pushing empty codes/values chunks into `res`. Detect the case where `encode_chunk` (or `start_encoding`) produced 0 codes from a non-empty chunk and return `res` as-is. The encoder is dropped so the next chunk starts fresh. Closes vortex-data#10397 AI assistance disclosure: this patch was drafted with help from an LLM-based coding assistant (Claude). The fix design, code, and tests were reviewed and edited manually before submission. Co-authored-by: Wahaj Ahmed <wahaj.ahmed010@gmail.com> Signed-off-by: Wahaj Ahmed <wahaj.ahmed010@gmail.com>
Contributor
|
see #10400 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tracking Issue: #10397
When every value in a chunk was longer than
max_bytes - 16(the most an empty dictionary can hold once the 16-byteBinaryViewheader is included),BytesDictBuilder::encode_valuereturnedNonefor every row.DictStreamState::encodethen produced 0 codes fromencode_chunk,remainderreturned the entire unencoded chunk, and the loop re-fed the same chunk indefinitely. Each pass also pushed empty codes/values chunks intores, so memory grew without bound until the process was killed.Changes
Detect the case where
encode_chunk(orstart_encoding) returnedEncodingState::Continuewith a zero-lengthencodedarray while the chunk still had rows. In both arms (self.encoder.take() == NoneandSome(encoder)) the function now returnsOk(res)immediately and lets the encoder drop. The outer loop never re-feeds the chunk that cannot be encoded.While here, also skip the inner
whilebody whenremainingis already empty (the caller's contract guarantees that branch is unreachable today, but the guard makes the loop self-consistent if it is reached).Two regression tests in
mod tests:test_dict_stream_state_encodes_value_filling_an_empty_dictionary— a 48-byte string under a 64-byte dictionary limit, which should still encode successfully.test_dict_stream_state_does_not_loop_on_oversized_value— a 49-byte string under the same 64-byte dictionary limit. The writer must return within 2 s (the test runsDictStreamState::encodein a spawned thread with anmpscchannel +recv_timeout, so a missing fix hangs the thread instead of the suite).API Changes
None. The bug is observable on the dict layout writer; no public signatures change.
AI assistance disclosure: this patch was drafted with help from an LLM-based coding assistant. The fix design, code, and tests were reviewed and edited manually before submission.