StackExchange / StackExchange/StackExchange.Redis

RedisValue equality treats "1,000", "(5)" and " 5 " as numbers, via NumberStyles.Any

Open
#3,233 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
6.2k
Forks
1.6k
Avg merge
1d 15h
Merged PRs (30d)
43

Description

Raised during review of #3230, where the numeric side of RedisValue equality came up. Pre-existing
behaviour, not caused by that PR.

What happens

operator == runs Simplify() on both sides, so text that parses as a number is compared as a number.
Format.cs:178 does that parse with NumberStyles.Any:

return double.TryParse(s, NumberStyles.Any, NumberFormatInfo.InvariantInfo, out value);

NumberStyles.Any allows thousands separators, parentheses for negatives, leading and trailing whitespace, a
currency symbol, and exponents. So:

comparison result storage
"1,000" == "1000" True String / String
"(5)" == "-5" True String / String
" 5 " == "5" True String / String
"1,0,0,0" == "1000" True String / String
"5." == "5" True String / String
"+5" == "5" True String / String
"1e2" == "100" True String / String

Both sides are String-typed throughout — these are ordinary string values, not numbers that happen to be
spelled differently.

Every integer path in Format.cs uses the much tighter NumberStyles.Integer; TryParseDouble is the only
one using Any, which makes it look more like an oversight than a decision.

Why it matters

These are distinct values on the server — distinct set members, distinct hash fields, distinct keys — that
compare equal in the client. Anything deduplicating RedisValues in a HashSet<RedisValue>, or keying a
Dictionary<RedisValue, T>, silently merges them. " 5 " and "5" are plainly different values by any
reading.

It is at least self-consistent: GetHashCode simplifies too, so equal values do share a hash and no hash
container is corrupted. The problem is that the equality is much wider than "numeric text".

Possible direction

NumberStyles.Float (leading/trailing whitespace, leading sign, decimal point, exponent) would drop the
separators, parentheses and currency symbol while keeping what Redis actually emits for numbers. Even that
leaves " 5 " == "5" true and "1e2" == "100" true, so the whitespace and exponent allowances are worth a
separate decision.

Any change here alters shipped comparison behaviour, so it wants its own discussion rather than being folded
into unrelated work — hence this issue rather than a PR.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with Format.cs:178 and the TryParseDouble path used by RedisValue.Simplify(), then compare it with the integer parsing paths that use NumberStyles.Integer. Review the listed equality cases and the proposed NumberStyles alternatives. Done means the accepted numeric-text semantics are agreed and the affected comparisons have regression coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, redis
Domain
backend, databases
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.