Jackie-Jiang opened a new pull request, #19638:
URL: https://github.com/apache/pinot/pull/19638

   ## Summary
   
   `VarByteChunkForwardIndexReaderV4.ReaderContext.getValue` serves a doc from 
the currently loaded chunk via `readSmallUncompressedValue` whenever `docId` 
falls in `[_docIdOffset, _nextDocIdOffset)`, without checking `_regularChunk`.
   
   A value larger than the target chunk size is written into its own huge 
chunk, which holds only the value bytes and no `[numDocs][offsets...]` header. 
After such a read, the context's doc range is `[docId, docId + 1)`, so reading 
the **same doc again with the same context** took the fast path and parsed a 
non-header buffer as a chunk header:
   - `PASS_THROUGH` (`UncompressedReaderContext`): `_chunk` is the raw value, 
so the value's bytes are read as offsets. This throws 
`IllegalArgumentException: newPosition > limit`, `NegativeArraySizeException`, 
or `BufferUnderflowException`.
   - Compressed (`CompressedReaderContext`, V6's `V6CompressedReaderContext`): 
the huge path clears `_decompressedBuffer` without filling it, so the re-read 
parses the previous regular chunk's leftover bytes with its stale 
`_numDocsInCurrentChunk`. For SV STRING/BYTES this **silently returns a wrong 
value**.
   
   Consecutive reads of the same doc with one context are legitimate, e.g. 
`getNumValuesMV` followed by `getStringMV`, or two accessors on the same row.
   
   The fix only takes the fast path for regular chunks. The extra check is one 
boolean field test on the regular-chunk path. A huge value is re-read 
(re-copied or re-decompressed) on every access. V5 and V6 inherit the fix, 
since all three contexts go through the base `getValue`.
   
   ## Notes for reviewers
   
   - `recordRangesForDocId` does not have this issue: for a huge chunk the 
cached `_ranges` is the metadata plus the whole chunk, which is exactly the 
value's byte range.
   - The older `VarByteChunkSVForwardIndexReader` / 
`VarByteChunkMVForwardIndexReader` (`ChunkReaderContext`) path is unaffected: 
chunks hold a fixed number of docs, the chunk buffer is sized for the longest 
entry, and there are no huge chunks.
   - The existing `VarByteChunkV4Test` params already write huge values 
(`longestEntry = 2048`, `chunkSize = 1024`), but their read loops never read 
the same doc twice in a row.
   
   ## Tests
   
   Added `testHugeValueReadTwiceSV` and `testHugeValueReadTwiceMV` to 
`VarByteChunkV4Test`, inherited by `VarByteChunkV5Test` and 
`VarByteChunkV6Test`. They cover every compression type. Each writes a huge 
value between two small ones and, with a single context, reads each doc twice 
(SV) or through `getNumValuesMV`, `getStringMV(docId, context)`, and 
`getStringMV(docId, buffer, context)` (MV).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to