Skip to content

fix(cpp): correct aligned all-null pages and page advancement - #1001

Merged
ColinLeeo merged 4 commits into
apache:developfrom
ColinLeeo:colin/fix-aligned-null-page-read
Oct 10, 2026
Merged

ColinLeeo merged 4 commits into
apache:developfrom
ColinLeeo:colin/fix-aligned-null-page-read

Conversation

@ColinLeeo

@ColinLeeo ColinLeeo commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

What changed

C++ serialized aligned all-null value pages with a row count, bitmap, and optional statistics and codec payload instead of the compact empty-page marker used by Java. This could cause queries to miss subsequent non-null values. A normal GORILLA page can also leave a padding byte unread after all values are decoded, preventing the C++ reader from advancing to the next page.

  • Write each all-null value page as a single zero varint, without compressed size, statistics, bitmap, or codec payload.
  • Handle an empty deferred first page without appending statistics or data, and recognize single-page all-null value chunks during recovery.
  • Complete the aligned value page when its time rows have been consumed, clearing unread padding and decoder state before advancing.
  • Refresh CLI golden file sizes and sketch offsets for the compact empty page (the all-null fixture shrinks from 457 to 451 bytes).

The final diff contains only C++ changes. Java implementations and encoding defaults are unchanged.

Validation

  • C++ GTest suite with BUILD_TOOLS=ON: 1131 passed, 3 skipped (external fixture configuration unavailable), 10 disabled. The complete CTest run, including CLI and CMake tests, passed with zero failures.
  • 45 parameterized read cases cover the canonical format with DICTIONARY/GORILLA, three page sizes, and record/tablet/mixed writes. Removing the reader state cleanup reproducibly fails the INT32/GORILLA case with seven rows per page; an independent codec probe confirms one padding byte remains after decoding all seven values.
  • Added exact empty-page format and deferred-first-page regressions; confirmed they fail before the writer fix. Existing recovery tests pass.
  • Unmodified Java read 72 newly generated C++ files successfully: 144 query checks across PLAIN, DICTIONARY, RLE, TS_2DIFF, and GORILLA, three page sizes, and record/tablet/mixed writes.
  • Spotless checks for changed C++ test files and git diff --check passed.

Found while reviewing #968; the issue also reproduces without mixing record and tablet writes.

@ColinLeeo ColinLeeo changed the title fix: continue aligned queries past all-null pages fix(cpp): write canonical all-null pages and preserve aligned reads Oct 10, 2026
@ColinLeeo ColinLeeo changed the title fix(cpp): write canonical all-null pages and preserve aligned reads fix(cpp): correct aligned all-null pages and page advancement Oct 10, 2026
@ColinLeeo
ColinLeeo requested a balanced review from Copilot October 10, 2026 06:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ColinLeeo
ColinLeeo merged commit e0f7366 into apache:develop Oct 10, 2026
47 of 48 checks passed
ColinLeeo added a commit that referenced this pull request Oct 10, 2026
* fix: continue aligned queries past all-null pages

* fix(cpp): write canonical all-null pages and revert Java changes

* test(cpp): refresh CLI goldens for compact all-null pages

* test(cpp): focus null-page coverage on canonical format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants