[format] Cross-check the row-file block index against its footer - #10001
jackylee-ch wants to merge 1 commit into
Conversation
| 5. **Decompress**: ZSTD decompress into a buffer of size `blockUncompressedSizes[b]`. | ||
| 6. **Locate Row**: Compute `localIdx = rowNum - blockRowStarts[b]`. Read `offsets[localIdx]` from the offset array at the end of the decompressed block. | ||
| 7. **Deserialize**: Read the row starting at the computed offset using the row serialization format. | ||
| 3. **Check Consistency**: The three arrays must have the same length, that length must equal `blockCount`, and `blockCompressedSizes[]` must sum to `indexOffset`, because the blocks are written contiguously from position 0 and the index follows the last one. A reader that bounds its block loop by one of the two — the footer's `blockCount` or the index array length — must reject a file where they disagree rather than silently reading fewer blocks. |
There was a problem hiding this comment.
[P1] Apply the new consistency contract to the Python row reader too
This now defines rejection as a format-reader requirement, and the PR description specifically calls out that Python bounds iteration by the footer count, but pypaimon/read/reader/format_row_reader.py::_read_metadata still trusts block_count, index_offset, and index_length and never compares the three decoded array lengths or their compressed-size sum. For example, changing blockCount to 0 still makes Python return an empty result for a non-empty file rather than reject it. Please implement the same validation and regression cases in Python so Java and Python do not retain the cross-language behavior this change is meant to remove.
| } | ||
|
|
||
| for (int i = 1; i < blockCount(); i++) { | ||
| if (blockRowStarts[i] < blockRowStarts[i - 1]) { |
There was a problem hiding this comment.
[P1] Reject row-start gaps and duplicates before selection uses them
Checking only for a decrease still accepts [10] for a one-block file and [0, 0] for two blocks. RowFormatReader.computeBlocksToRead then treats those values as block ranges: selected rows 0-9 are omitted in the first case, and the first block has the empty range [0, 0) in the second, so selection-backed reads can silently drop valid rows even though this validation succeeds. Please require the first start to be 0, subsequent starts to increase strictly, and an empty index only when totalRowCount is 0; a selection regression would pin the behavior.
2 participants
Purpose
The footer and the block index describe the same blocks twice.
blockCountis written into every row file and never read: Java bounds its block loop by the index array length, the Python reader by the footer field. RewritingblockCountto 99 in a 1000-row file changes nothing in Java — it opens, and all 1000 rows come back.Cross-check them where both are in hand.
blockCompressedSizesmust sum toindexOffset, since blocks are written contiguously from position 0 and the index follows the last one; the three arrays must agree in length; that length must equalblockCount; row starts must not go backwards. The spec asserted the prefix-sum rule in prose, so its lookup algorithm gains the consistency step.Footer offsets are bounded against the file size too, which is what makes
new byte[indexLength]safe — anindexOffsetoutside the file was anArrayIndexOutOfBoundsExceptionfromarraycopy. Underignore-corrupt-filessuch a file is skipped either way, so the named error only helps readers that did not opt into lenience.RowFileFooter.readFrom(SeekableInputStream, long)had no callers and goes with it.Tests
RowFileIndexConsistencyTest.paimon-format: 710 run, 0 failures.Written with Claude Code; verification is mine.