Skip to content

[ntuple] Fix page checksum handling during recompression - #24147

Open
Johnny-Kao wants to merge 2 commits into
root-project:masterfrom
Johnny-Kao:fix/rntuple-recompression-page-checksum
Open

Johnny-Kao wants to merge 2 commits into
root-project:masterfrom
Johnny-Kao:fix/rntuple-recompression-page-checksum

Conversation

@Johnny-Kao

@Johnny-Kao Johnny-Kao commented Nov 13, 2026 •

Copy link
Copy Markdown

Thank you for the previous PR. Appreciated.
Second time here.
Would need your review on the following.

Changes

Merging RNTuples with different page checksum and compression settings can abort or crash when a page needs to be recompressed.

The merger allocates space using the source page's checksum setting, but calculates the recompressed page's size using the destination setting. When these differ, the size and checksum flag no longer agree.

This keeps the checksum size consistent with the source page, as in the existing fast-merge path.

It's related to #24085, but separate: that PR handled same-page merging without checksums; this fixes the recompression path.

I've also added a regression test for mixed settings in both source orders.

Testing:

  • On ROOT 6.43.01 (5281bc2), 6/12 independent cases failed before the fix; all 12 passed afterward.
  • The public TFileMerger path aborted before the fix and passed afterward in both source orders (260 entries checked per merge).
  • The new upstream regression test passed C++17 syntax/API compilation; the complete ROOT test suite was not run.
  • The patch applies cleanly to the then-current master.

Research run: https://github.com/Johnny-Kao/root-rntuple-research/actions/runs/43455000906

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

@Johnny-Kao
Johnny-Kao marked this pull request as ready for review November 13, 2026 17:13

This branch has not been deployed

No deployments
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.

1 participant