Fix SORT GET to ignore special pattern # in cluster mode

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

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 '#'. */
+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 #]
+ }
 }