Skip to content

[ntuple] Merger: fix bugs in GenerateZeroPagesForColumns #19198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

silverweed
Copy link
Contributor

  • nElements was used to grow each zero page instead of nElementsPerPage
  • only the last zero page was added to the page groups

This bug manifested as a crash when Union-merging big-enough RNTuples, so that GenerateZeroPagesForColumns actually generates more than 1 page.

This fix will be backported to 6.36.

cc @amete

Checklist:

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

@silverweed silverweed requested review from hahnjo and enirolf June 26, 2025 12:44
@silverweed silverweed self-assigned this Jun 26, 2025
@silverweed silverweed requested a review from jblomer as a code owner June 26, 2025 12:44
- nElements was used to grow each zero page instead of nElementsPerPage
- only the last zero page was added to the page groups
@silverweed silverweed force-pushed the ntuple_merger_page_nelems branch from 4a2c330 to eae9482 Compare June 26, 2025 12:45
Copy link

Test Results

    20 files      20 suites   3d 7h 19m 35s ⏱️
 3 050 tests  3 048 ✅   0 💤 2 ❌
59 400 runs  59 261 ✅ 137 💤 2 ❌

For more details on these failures, see this check.

Results for commit eae9482.

Copy link
Member

@hahnjo hahnjo left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

@jblomer jblomer left a comment

Choose a reason for hiding this comment

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

Thanks!

@silverweed silverweed merged commit e9b81c8 into root-project:master Jun 27, 2025
23 of 25 checks passed
@silverweed silverweed deleted the ntuple_merger_page_nelems branch June 27, 2025 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants