Fix BZMPOP return values, also add more argument checking. Add test.

microsoft · Mar 8, 2025 · 114c9fa · 114c9fa
1 parent a01bb51
commit 114c9fa
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 4 deletions.
diff --git a/libs/server/Resp/Objects/SortedSetCommands.cs b/libs/server/Resp/Objects/SortedSetCommands.cs
@@ -1552,7 +1552,7 @@ private unsafe bool SortedSetBlockingPop(RespCommand command)
                 return AbortWithWrongNumberOfArguments(command.ToString());
             }
 
-            if (!parseState.TryGetDouble(parseState.Count - 1, out var timeout))
+            if (!parseState.TryGetDouble(parseState.Count - 1, out var timeout) || (timeout < 0))
             {
                 return AbortWithErrorMessage(CmdStrings.RESP_ERR_TIMEOUT_NOT_VALID_FLOAT);
             }
@@ -1605,13 +1605,13 @@ private unsafe bool SortedSetBlockingMPop()
             var currTokenId = 0;
 
             // Read timeout
-            if (!parseState.TryGetDouble(currTokenId++, out var timeout))
+            if (!parseState.TryGetDouble(currTokenId++, out var timeout) || (timeout < 0))
             {
                 return AbortWithErrorMessage(CmdStrings.RESP_ERR_TIMEOUT_NOT_VALID_FLOAT);
             }
 
             // Read count of keys
-            if (!parseState.TryGetInt(currTokenId++, out var numKeys))
+            if (!parseState.TryGetInt(currTokenId++, out var numKeys) || (numKeys <= 0))
             {
                 var err = string.Format(CmdStrings.GenericParamShouldBeGreaterThanZero, "numkeys");
                 return AbortWithErrorMessage(Encoding.ASCII.GetBytes(err));
@@ -1685,7 +1685,7 @@ private unsafe bool SortedSetBlockingMPop()
             while (!RespWriteUtils.TryWriteArrayLength(result.Items.Length, ref dcurr, dend))
                 SendAndReset();
 
-            for (var i = 0; i < result.Items.Length; i += 2)
+            for (var i = 0; i < result.Items.Length; ++i)
             {
                 while (!RespWriteUtils.TryWriteArrayLength(2, ref dcurr, dend))
                     SendAndReset();

diff --git a/test/Garnet.test/RespBlockingCollectionTests.cs b/test/Garnet.test/RespBlockingCollectionTests.cs
@@ -7,6 +7,7 @@
 using Garnet.server;
 using NUnit.Framework;
 using NUnit.Framework.Legacy;
+using StackExchange.Redis;
 
 namespace Garnet.test
 {
@@ -413,6 +414,30 @@ public void BasicBzmpopTest(string mode, string expectedValue, double expectedSc
             ClassicAssert.AreEqual(expectedResponse, actualValue);
         }
 
+        [Test]
+        public void BzmpopReturnTest()
+        {
+            using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig());
+            var db = redis.GetDatabase(0);
+
+            var res = db.SortedSetAdd("a", [new SortedSetEntry("one", 1)]);
+            ClassicAssert.AreEqual(1, res);
+            res = db.SortedSetAdd("a", [new SortedSetEntry("two", 2)]);
+            ClassicAssert.AreEqual(1, res);
+            res = db.SortedSetAdd("b", [new SortedSetEntry("three", 3)]);
+            ClassicAssert.AreEqual(1, res);
+            var pop = db.Execute("BZMPOP", "10", "2", "a", "b", "MIN", "COUNT", "2");
+            ClassicAssert.AreEqual(2, pop.Length);
+            ClassicAssert.AreEqual("a", pop[0].ToString());
+            ClassicAssert.AreEqual(2, pop[1].Length);
+            ClassicAssert.AreEqual(2, pop[1][0].Length);
+            ClassicAssert.AreEqual("one", pop[1][0][0].ToString());
+            ClassicAssert.AreEqual(1, (int)(RedisValue)pop[1][0][1]);
+            ClassicAssert.AreEqual(2, pop[1][1].Length);
+            ClassicAssert.AreEqual("two", pop[1][1][0].ToString());
+            ClassicAssert.AreEqual(2, (int)(RedisValue)pop[1][1][1]);
+        }
+
         [Test]
         [TestCase(1, "key1", "value1", 1.5, Description = "First key has minimum value")]
         [TestCase(2, "key2", "value2", 2.5, Description = "Second key has minimum value")]