Skip to content

Key the combinations name cache by reference - #1861

Merged
SimonCropp merged 1 commit into
mainfrom
fix-combinations-name-cache
Aug 17, 2026
Merged

Key the combinations name cache by reference#1861
SimonCropp merged 1 commit into
mainfrom
fix-combinations-name-cache

Conversation

@SimonCropp

Copy link
Copy Markdown
Member

CombinationResultsConverter caches the rendered name per key in a Dictionary<object, string>, which used the boxed value's own equality. Two keys that compare equal but render differently therefore shared one name:

  • DateTime.Equals ignores Kind, but the name carries it.
  • DateTimeOffset.Equals compares only the instant, but the name carries the offset.

A combination over 2000-01-01 as Utc, Local and Unspecified labeled all three rows 2000-01-01Utc — so the output had duplicate property names and no way to tell the rows apart. Same for 2000-01-01T00:00+00:00 against 2000-01-01T01:00+01:00.

Each input list is materialized once (a.Cast<object?>() then ToArray() in the CombinationRunner constructor) and rows index into that array, so a key recurs as the same boxed instance. Reference equality is what the cache actually wants, and it sidesteps the value equality traps entirely — that is now what it uses, via a small internal ReferenceComparer.

Worth noting Counter already treats these as distinct (DateTimeComparer compares Kind, and there is a DateTimeOffset equivalent), so this brings the converter in line with the rest of the codebase.

Test CombinationTests.DateKeysThatCompareEqual combines the three DateTimeKind values against two equal-instant offsets. On main all six rows come out labeled 2000-01-01Utc, 2000-01-01+0; with the fix each row is distinct.

Verify.Tests (1300) and StaticSettingsTests pass, and Verify builds for all nine target frameworks.

The cache was a Dictionary<object, string> using the boxed value's own equality,
so keys that compare equal but render differently shared one name:
DateTime.Equals ignores Kind and DateTimeOffset.Equals compares only the instant,
while both of those are part of the rendered name. A combination over
2000-01-01 Utc, Local and Unspecified labeled every row 2000-01-01Utc.

Each input list is materialized once, so a key recurs as the same boxed instance
and reference equality is what the cache actually wants.
@SimonCropp SimonCropp added this to the 32.0.0 milestone Aug 17, 2026
@SimonCropp
SimonCropp merged commit 878196f into main Aug 17, 2026
7 checks passed
@SimonCropp
SimonCropp deleted the fix-combinations-name-cache branch August 17, 2026 12:26
This was referenced Aug 26, 2026
This was referenced Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant