fix(cpp): stop misreading pages after an empty value page - #976
Merged
Merged
Conversation
…ge (apache#971) Java writes an all-null value page as a single zero varint and omits the compressed size and statistics (ValueChunkWriter.writeEmptyPageToPageBuffer). The C++ PageHeader::deserialize_from always read a second varint and, when requested, a statistic, so after an empty page it consumed the following page's bytes as the empty page's header and misread every later page. Return early when uncompressed_size_ == 0, matching the Java format. The aligned readers already treat compressed_size_ == 0 as an all-null page, so the fix is localized to PageHeader deserialization. Add PageHeader unit tests covering an empty page followed by a non-empty page, for both deserialize_stat values, asserting it consumes exactly one varint and leaves the following page intact.
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.
Fixes #971.
Bug
Java encodes an all-null aligned value page as a single zero varint. It writes no compressed size and no statistics, and the Java
PageHeader.deserializeFromreturns as soon as it reads that0. The C++PageHeader::deserialize_frompreviously read another varint and, when requested, a statistic. After an empty page it therefore consumed bytes from the next page as the empty page's header, so every later page in that value chunk was parsed from the wrong offset.For a column whose empty page is followed by a non-empty page this produces silently wrong results. The reported aligned table-model file has alternating filled and all-null tablets, so the failure repeats throughout the value chunk. A trailing empty page does not expose the offset error because there is no later page to consume.
Fix
Return early from
PageHeader::deserialize_fromwhenuncompressed_size_ == 0, settingcompressed_size_ = 0and leavingstatistic_null. This matches the Java empty-page encoding. The aligned readers already treatcompressed_size_ == 0as an all-null value page; the row count still comes from the time page, so the production change is limited to page-header parsing.Tests
cpp/test/common/tsfile_common_test.ccDeserializeEmptyPageConsumesSingleVarintchecks that an empty header does not consume the following page header.DeserializeEmptyPageWithStatisticConsumesSingleVarintcovers the statistics-enabled path.The existing Java/C++ compatibility fixture matrix now also generates null-page cases across
UNCOMPRESSED,LZ4,ZSTD, andLZMA2for all supported table value types. Each mixed case contains an empty first page, consecutive empty pages, an empty page followed by a non-empty page, an in-page null bitmap, and trailing empty pages. The Java generator asserts that empty pages use the one-varint wire form; both readers validate complete scans and ranges crossing page boundaries, with a non-null anchor column verifying aligned row preservation. The workflow continues to validate both Java-written fixtures with C++ and C++-written fixtures with Java.Validation