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

[mgravell]

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.

[mgravell]

Worse than first reported, and the round-trip case makes it a straightforward bug rather than a debatable
widening. Credit for the hypothesis goes to the reviewer on #3230; I ran it to confirm.

The same text does not equal itself, depending on storage

Simplify() has two parsers, and they disagree:

• String → Format.TryParseDouble(string), which is NumberStyles.Any
• Blob → the byte-based TryParseDouble(ReadOnlySpan<byte>), which is strict and must consume the
  whole input

So text the loose parser accepts becomes a number when stored as a string, and stays text when stored as
bytes — and a number never equals a non-number:

text	(RedisValue)text == (RedisValue)UTF8(text)	string simplifies to	blob simplifies to
"1,000"	False	Int64	ByteArray
"(5)"	False	Int64	ByteArray
" 5 "	False	Int64	ByteArray
"1,0,0,0"	False	Int64	ByteArray
"5."	True	Int64	Int64
"+5"	True	Int64	Int64
"1e2"	True	Int64	Int64
"42", "hello"	True	—	—

What that looks like to a caller

Against a real server, writing a value and reading it straight back:

RedisValue written = "1,000";
await db.StringSetAsync(key, written);
var read = await db.StringGetAsync(key);   // arrives as a blob

written == read   // False

'1,000': written == read -> False
'(5)':   written == read -> False
' 5 ':   written == read -> False
'42':    written == read -> True
'hello': written == read -> True

A value is not equal to itself after a round trip. That is a plainer bug than the original report of
"1,000" == "1000", and it needs no unusual container or comparer to hit — just StringSet then StringGet.

Still not a hash-contract violation: GetHashCode goes through the same Simplify(), so each storage form
is internally consistent and dictionaries are not corrupted. The two forms simply normalise differently.

Direction

The suggestion I'd take is to give strings the byte parser rather than trying to match flags by hand.
Numeric text is short, so the string path can length-check against Format.MaxDoubleTextLen, encode into a
stack buffer, and call the same TrySimplify(ReadOnlySpan<byte>) the blob and sequence paths already share —
note that Simplify() already does exactly this for StorageType.Sequence, with the comment "so that
identical bytes simplify identically regardless of how they happen to be stored". The string case is the one
that was left out. One definition of "numeric", matched by construction rather than by convention.

NumberStyles.Float would not be enough on its own: it still allows leading and trailing whitespace, so
" 5 " would keep parsing.

Worth deciding alongside

• It is a behaviour change. Anyone relying on "1,000" == "1000" sees different results, and hash codes
  for those values change — which only matters if they are persisted outside the process, something that was
  never safe anyway.
• Other callers of the loose parser. TryParseDouble(string) is not only used by Simplify(); the
  (double) conversion uses it too, and possibly IConvertible. Conversions may legitimately want to be
  forgiving even if equality does not, so decide whether to tighten Simplify() alone or the parser itself.
• A regression guard. A test that takes a set of text inputs and asserts that each, stored as a string and
  as a blob, is equal and shares a hash code. That would have caught this, and would stop the parsers drifting
  apart again.