Skip to content

Handle null values in joined summary columns#3197

Open
infisble wants to merge 3 commits into
dotnet:masterfrom
infisble:agent/fix-joined-summary-null-column
Open

Handle null values in joined summary columns#3197
infisble wants to merge 3 commits into
dotnet:masterfrom
infisble:agent/fix-joined-summary-null-column

Conversation

@infisble

@infisble infisble commented Jul 16, 2026

Copy link
Copy Markdown

Fixes #2656.

When summaries with different column providers are joined, a column may not have a value for benchmarks originating from another summary. SummaryTable stored that null value and later dereferenced it while calculating the column width, which caused the reported NullReferenceException.

This change renders missing column values as NA and adds a regression test that joins two summaries where a custom column only applies to one of them.

Tests:

  • dotnet test tests/BenchmarkDotNet.Tests/BenchmarkDotNet.Tests.csproj -c Release -f net10.0 --no-restore --filter FullyQualifiedName~JoinedSummaryHandlesMissingColumnValues (1 passed)
  • dotnet test tests/BenchmarkDotNet.Tests/BenchmarkDotNet.Tests.csproj -c Release -f net10.0 --no-restore --no-build --filter FullyQualifiedName~BenchmarkDotNet.Tests.Reports.SummaryTableTests (23 passed)
  • Full BenchmarkDotNet.Tests run: 1022 passed, 4 skipped, 2 failed. The two Linux CPU parser fixture failures reproduce unchanged on a clean upstream/master worktree.

@infisble
infisble marked this pull request as ready for review July 16, 2026 23:06
@timcassell

Copy link
Copy Markdown
Collaborator

I thought usually the non-existing values are displayed as - rather than empty. It would be good to unify the behavior.

@infisble

Copy link
Copy Markdown
Author

Good point. I updated the fallback to use - and adjusted the regression test in 6c3b992. The focused test passes, along with all 23 SummaryTableTests.

@timcassell

timcassell commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Actually looking at the code it seems the common display values for non-existant are NA or ?. - is used for 0 values in metric columns. I think NA fits better here.

@infisble

Copy link
Copy Markdown
Author

Thanks for double-checking. I switched the fallback to NA and updated the regression test in 626fef8. The focused test passes, along with all 23 SummaryTableTests.

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.

Bug: NRE possible on joining categories

2 participants