tidying (support ulong as canonical)

Author: mgravell

src/StackExchange.Redis/RedisValue.cs

@@ -1498,7 +1498,6 @@ public bool TryParse(out long val)
                     return Format.TryParseInt64(RawString(), out val);
                 case StorageType.MemoryManager or StorageType.ByteArray or StorageType.ShortBlob:
                     return Format.TryParseInt64(UnsafeRawSpan(out _), out val);
-
                 case StorageType.Sequence:
                     // longer than the largest possible Int64 text => cannot be an Int64; otherwise
                     // linearize onto the stack and reuse the span-based parse (matching the ByteArray path)

src/StackExchange.Redis/RespReaderExtensions.cs

@@ -243,10 +243,11 @@ internal bool AnyNull()
 
     private static readonly int MaxCanonicalLength = Math.Max(Format.MaxInt64TextLen, Format.MaxDoubleTextLen);
 
-    // Recognizes the canonical decimal text of an integer or finite double, returning a numeric-backed
-    // RedisValue only when re-formatting the parsed value reproduces the exact input bytes. This keeps the
-    // optimization invisible to callers: non-canonical spellings ("01234", "+5", "1.50", "1e3"), oversize
-    // values, and the special inf/nan tokens all return false and are kept as a byte[] payload by the caller.
+    // Recognizes the canonical decimal text of an integer (signed Int64 or, for non-negative values up to
+    // ulong.MaxValue, UInt64) or a finite double, returning a numeric-backed RedisValue only when re-formatting
+    // the parsed value reproduces the exact input bytes. This keeps the optimization invisible to callers:
+    // non-canonical spellings ("01234", "+5", "1.50", "1e3"), values beyond the ulong/double range, and the
+    // special inf/nan tokens all return false and are kept as a byte[] payload by the caller.
     private static bool TryReadCanonicalNumber(ReadOnlySpan<byte> span, out RedisValue value)
     {
         static bool Failure(out RedisValue value)
@@ -258,7 +259,10 @@ static bool Failure(out RedisValue value)
         // leading '+', "-0", trailing junk, etc.) - so no need to re-emit and compare bytes
         if (span.IsEmpty | span.Length > MaxCanonicalLength) return Failure(out value);
 
-        // restrict to *just* basic number tokens
+        // restrict to *just* basic number tokens, tracking the two facts that let us pick a single parse:
+        // whether a '-' appeared (so we know whether the integer is signed), and whether a '.'/'e'/'E'
+        // appeared (which rules out an integer entirely - only a double can be canonical)
+        bool seenNegative = false, seenDotOrExp = false;
         foreach (var b in span)
         {
             switch (b)
@@ -273,23 +277,44 @@ static bool Failure(out RedisValue value)
                 case (byte)'7':
                 case (byte)'8':
                 case (byte)'9':
-                case (byte)'.':
+                    break;
                 case (byte)'-':
+                    seenNegative = true;
+                    break;
+                case (byte)'.':
                 case (byte)'E':
                 case (byte)'e':
+                    seenDotOrExp = true;
                     break;
                 default:
                     return Failure(out value);
             }
         }
 
-        if (Format.TryParseInt64(span, out var i64) && Format.MeasureInt64(i64) == span.Length)
+        if (!seenDotOrExp)
         {
-            value = i64;
-            return true;
-        }
+            // pure integer text. For a non-negative value parse as *unsigned* so the full ulong range is
+            // covered; the RedisValue(ulong) ctor demotes to Int64 storage when the value fits, so smaller
+            // values still land as Int64 exactly as before. A negative value can only be Int64.
+            if (seenNegative)
+            {
+                if (Format.TryParseInt64(span, out var i64) && Format.MeasureInt64(i64) == span.Length)
+                {
+                    value = i64;
+                    return true;
+                }
+            }
+            else if (Format.TryParseUInt64(span, out var u64) && Format.MeasureUInt64(u64) == span.Length)
+            {
+                value = u64;
+                return true;
+            }
 
-        if (Format.TryParseDouble(span, out var dbl))
+            // note that all-digit text which isn't a canonical integer (oversize, leading zero, etc.) can't
+            // be a canonical double either - any such value re-formats with an exponent - so we simply fall
+            // through to the failure at the bottom rather than attempting a (futile) double parse
+        }
+        else if (Format.TryParseDouble(span, out var dbl))
         {
             if (dbl == 0.0) dbl = Math.Abs(dbl); // prevent problems with -0 formatting
             Span<byte> formatted = stackalloc byte[Format.MaxDoubleTextLen];

tests/StackExchange.Redis.Tests/Issues/Issue1103Tests.cs

@@ -9,12 +9,12 @@ public class Issue1103Tests(ITestOutputHelper output) : TestBase(output)
 {
     [Theory]
     [InlineData(142205255210238005UL, (int)StorageType.Int64, (int)StorageType.Int64)]
-    [InlineData(ulong.MaxValue, (int)StorageType.UInt64)]
+    [InlineData(ulong.MaxValue, (int)StorageType.UInt64, (int)StorageType.UInt64)] // 20-byte canonical uint => UInt64 on read
     [InlineData(ulong.MinValue, (int)StorageType.Int64, (int)StorageType.ShortBlob)]
-    [InlineData(0x8000000000000000UL, (int)StorageType.UInt64)]
-    [InlineData(0x8000000000000001UL, (int)StorageType.UInt64)]
+    [InlineData(0x8000000000000000UL, (int)StorageType.UInt64, (int)StorageType.UInt64)] // long.MaxValue+1: 19-byte canonical uint => UInt64 on read
+    [InlineData(0x8000000000000001UL, (int)StorageType.UInt64, (int)StorageType.UInt64)]
     [InlineData(0x7FFFFFFFFFFFFFFFUL, (int)StorageType.Int64, (int)StorageType.Int64)] // long.MaxValue: 19-byte canonical int => Int64 on read
-    public async Task LargeUInt64StoredCorrectly(ulong value, int storageType, int fromRedisType = (int)StorageType.ByteArray)
+    public async Task LargeUInt64StoredCorrectly(ulong value, int storageType, int fromRedisType)
     {
         await using var conn = Create();
 
@@ -35,7 +35,7 @@ public async Task LargeUInt64StoredCorrectly(ulong value, int storageType, int f
 
         var simplified = fromRedis.Simplify();
         Log($"{simplified.Type}: {simplified}");
-        Assert.Equal((StorageType)storageType, typed.Type);
+        Assert.Equal((StorageType)storageType, simplified.Type);
         Assert.Equal(value, (ulong)simplified);
         Assert.Equal(value.ToString(CultureInfo.InvariantCulture), fromRedis.ToString());
     }

tests/StackExchange.Redis.Tests/RedisValueShortBlobTests.cs

@@ -112,6 +112,25 @@ public void ShortBlob_StartsWith_MatchesByteArray()
         Assert.False(shortBlob.StartsWith("abcdef"u8.ToArray()));
     }
 
+    [Fact]
+    public void ShortBlob_CompareTo_Sequence_NonEqualOrdering()
+    {
+        // the equal-content tests only assert CompareTo == 0; this pins the *non-zero* cross-kind branches
+        // of BlobCompareTo (contiguous ShortBlob vs multi-segment Sequence), in both directions.
+        var abc = Short("abc"u8.ToArray());
+        var abd = Sequence("abd"u8.ToArray());
+
+        // differing content: "abc" < "abd"
+        Assert.True(abc.CompareTo(abd) < 0); // ShortBlob (x) vs Sequence (y) - the ySeq branch
+        Assert.True(abd.CompareTo(abc) > 0); // Sequence (x) vs ShortBlob (y) - the negated xSeq branch
+
+        // length mismatch: "ab" is a prefix of "abc", so the shorter value sorts first
+        var ab = Short("ab"u8.ToArray());
+        var abcSeq = Sequence("abc"u8.ToArray());
+        Assert.True(ab.CompareTo(abcSeq) < 0);
+        Assert.True(abcSeq.CompareTo(ab) > 0);
+    }
+
     [Fact]
     public void FromRaw_RoutesByLength()
     {

tests/StackExchange.Redis.Tests/RedisValueStorageKindUnitTests.cs

@@ -42,6 +42,9 @@ public class RedisValueStorageKindUnitTests(ITestOutputHelper output, InProcServ
     // > 8 bytes AND a canonical number => compact Int64/Double (also avoids the byte[] alloc)
     [InlineData("123456789", "Int64")] // 9 bytes
     [InlineData("9223372036854775807", "Int64")] // long.MaxValue (19 bytes)
+    // canonical, non-negative, and > long.MaxValue => UInt64 (covers the full ulong range on read)
+    [InlineData("9223372036854775808", "UInt64")] // long.MaxValue + 1 (19 bytes)
+    [InlineData("18446744073709551615", "UInt64")] // ulong.MaxValue (20 bytes)
     [InlineData("1048576.5", "Double")] // 9 bytes, exactly representable => canonical under G17
     [InlineData("-1048576.25", "Double")] // 11 bytes
     // > 8 bytes and not a canonical number => materialized as a byte[]