Fix SORT GET to ignore special pattern # in cluster slot check (#1182)

This special pattern '#' is used to get the element itself,
it does not actually participate in the slot check.

In this case, passing `GET #` will cause '#' to participate
in the slot check, causing the command to get an
`pattern may be in different slots` error.

Signed-off-by: Binbin <[email protected]>

src/db.c

@@ -2418,9 +2418,9 @@ int bzmpopGetKeys(struct serverCommand *cmd, robj **argv, int argc, getKeysResul
 
 /* Helper function to extract keys from the SORT RO command.
  *
- * SORT <sort-key>
+ * SORT_RO <sort-key>
  *
- * The second argument of SORT is always a key, however an arbitrary number of
+ * The second argument of SORT_RO is always a key, however an arbitrary number of
  * keys may be accessed while doing the sort (the BY and GET args), so the
  * key-spec declares incomplete keys which is why we have to provide a concrete
  * implementation to fetch the keys.

src/sort.c

@@ -43,6 +43,11 @@ serverSortOperation *createSortOperation(int type, robj *pattern) {
     return so;
 }
 
+/* Return 1 if pattern is the special pattern '#'. */
+static int isReturnSubstPattern(sds pattern) {
+    return pattern[0] == '#' && pattern[1] == '\0';
+}
+
 /* Return the value associated to the key with a name obtained using
  * the following rules:
  *
@@ -68,7 +73,7 @@ robj *lookupKeyByPattern(serverDb *db, robj *pattern, robj *subst) {
     /* If the pattern is "#" return the substitution object itself in order
      * to implement the "SORT ... GET #" feature. */
     spat = pattern->ptr;
-    if (spat[0] == '#' && spat[1] == '\0') {
+    if (isReturnSubstPattern(spat)) {
         incrRefCount(subst);
         return subst;
     }
@@ -258,6 +263,7 @@ void sortCommandGeneric(client *c, int readonly) {
              * unless we can make sure the keys formed by the pattern are in the same slot
              * as the key to sort. */
             if (server.cluster_enabled &&
+                !isReturnSubstPattern(c->argv[j + 1]->ptr) &&
                 patternHashSlot(c->argv[j + 1]->ptr, sdslen(c->argv[j + 1]->ptr)) != getKeySlot(c->argv[1]->ptr)) {
                 addReplyError(c, "GET option of SORT denied in Cluster mode when "
                                  "keys formed by the pattern may be in different slots.");

tests/unit/sort.tcl

@@ -384,24 +384,28 @@ start_cluster 1 0 {tags {"external:skip cluster sort"}} {
     test "sort by in cluster mode" {
         catch {r sort "{a}mylist" by by*} e
         assert_match {ERR BY option of SORT denied in Cluster mode when *} $e
-        r sort "{a}mylist" by "{a}by*"
-    } {3 1 2}
+        assert_equal {3 1 2} [r sort "{a}mylist" by "{a}by*"]
+        assert_equal {3 1 2} [r sort "{a}mylist" by "{a}by*" get #]
+    }
 
     test "sort get in cluster mode" {
         catch {r sort "{a}mylist" by "{a}by*" get get*} e
         assert_match {ERR GET option of SORT denied in Cluster mode when *} $e
-        r sort "{a}mylist" by "{a}by*" get "{a}get*"
-    } {30 200 100}
+        assert_equal {30 200 100} [r sort "{a}mylist" by "{a}by*" get "{a}get*"]
+        assert_equal {30 3 200 1 100 2} [r sort "{a}mylist" by "{a}by*" get "{a}get*" get #]
+    }
 
     test "sort_ro by in cluster mode" {
         catch {r sort_ro "{a}mylist" by by*} e
         assert_match {ERR BY option of SORT denied in Cluster mode when *} $e
-        r sort_ro "{a}mylist" by "{a}by*"
-    } {3 1 2}
+        assert_equal {3 1 2} [r sort_ro "{a}mylist" by "{a}by*"]
+        assert_equal {3 1 2} [r sort_ro "{a}mylist" by "{a}by*" get #]
+    }
 
     test "sort_ro get in cluster mode" {
         catch {r sort_ro "{a}mylist" by "{a}by*" get get*} e
         assert_match {ERR GET option of SORT denied in Cluster mode when *} $e
-        r sort_ro "{a}mylist" by "{a}by*" get "{a}get*"
-    } {30 200 100}
+        assert_equal {30 200 100} [r sort_ro "{a}mylist" by "{a}by*" get "{a}get*"]
+        assert_equal {30 3 200 1 100 2} [r sort_ro "{a}mylist" by "{a}by*" get "{a}get*" get #]
+    }
 }