From 6e17afa80a115b21568cac16719eca6599d95476 Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Tue, 11 Aug 2020 04:55:06 -0400 Subject: [PATCH] Implement SMISMEMBER key member [member ...] (#7615) This is a rebased version of #3078 originally by shaharmor with the following patches by TysonAndre made after rebasing to work with the updated C API: 1. Add 2 more unit tests (wrong argument count error message, integer over 64 bits) 2. Use addReplyArrayLen instead of addReplyMultiBulkLen. 3. Undo changes to src/help.h - for the ZMSCORE PR, I heard those should instead be automatically generated from the redis-doc repo if it gets updated Motivations: - Example use case: Client code to efficiently check if each element of a set of 1000 items is a member of a set of 10 million items. (Similar to reasons for working on #7593) - HMGET and ZMSCORE already exist. This may lead to developers deciding to implement functionality that's best suited to a regular set with a data type of sorted set or hash map instead, for the multi-get support. Currently, multi commands or lua scripting to call sismember multiple times would almost definitely be less efficient than a native smismember for the following reasons: - Need to fetch the set from the string every time instead of reusing the C pointer. - Using pipelining or multi-commands would result in more bytes sent and received by the client for the repeated SISMEMBER KEY sections. - Need to specially encode the data and decode it from the client for lua-based solutions. - Proposed solutions using Lua or SADD/SDIFF could trigger writes to memory, which is undesirable on a redis replica server or when commands get replicated to replicas. Co-Authored-By: Shahar Mor Co-Authored-By: Tyson Andre --- src/server.c | 4 ++++ src/server.h | 1 + src/t_set.c | 19 +++++++++++++++++++ tests/support/cluster.tcl | 2 +- tests/unit/type/set.tcl | 34 ++++++++++++++++++++++++++++++++-- 5 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/server.c b/src/server.c index 15b962a2e..162dfa346 100644 --- a/src/server.c +++ b/src/server.c @@ -354,6 +354,10 @@ struct redisCommand redisCommandTable[] = { "read-only fast @set", 0,NULL,1,1,1,0,0,0}, + {"smismember",smismemberCommand,-3, + "read-only fast @set", + 0,NULL,1,1,1,0,0,0}, + {"scard",scardCommand,2, "read-only fast @set", 0,NULL,1,1,1,0,0,0}, diff --git a/src/server.h b/src/server.h index 5c5f939ce..7f1e7ea7b 100644 --- a/src/server.h +++ b/src/server.h @@ -2278,6 +2278,7 @@ void saddCommand(client *c); void sremCommand(client *c); void smoveCommand(client *c); void sismemberCommand(client *c); +void smismemberCommand(client *c); void scardCommand(client *c); void spopCommand(client *c); void srandmemberCommand(client *c); diff --git a/src/t_set.c b/src/t_set.c index 75678f47c..f474c2b56 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -382,6 +382,25 @@ void sismemberCommand(client *c) { addReply(c,shared.czero); } +void smismemberCommand(client *c) { + robj *set; + int j; + + /* Don't abort when the key cannot be found. Non-existing keys are empty + * sets, where SMISMEMBER should respond with a series of zeros. */ + set = lookupKeyRead(c->db,c->argv[1]); + if (set && checkType(c,set,OBJ_SET)) return; + + addReplyArrayLen(c,c->argc - 2); + + for (j = 2; j < c->argc; j++) { + if (set && setTypeIsMember(set,c->argv[j]->ptr)) + addReply(c,shared.cone); + else + addReply(c,shared.czero); + } +} + void scardCommand(client *c) { robj *o; diff --git a/tests/support/cluster.tcl b/tests/support/cluster.tcl index dced8d864..fb8c46e75 100644 --- a/tests/support/cluster.tcl +++ b/tests/support/cluster.tcl @@ -24,7 +24,7 @@ set ::redis_cluster::plain_commands { get set setnx setex psetex append strlen exists setbit getbit setrange getrange substr incr decr rpush lpush rpushx lpushx linsert rpop lpop brpop llen lindex lset lrange ltrim lrem - sadd srem sismember scard spop srandmember smembers sscan zadd + sadd srem sismember smismember scard spop srandmember smembers sscan zadd zincrby zrem zremrangebyscore zremrangebyrank zremrangebylex zrange zrangebyscore zrevrangebyscore zrangebylex zrevrangebylex zcount zlexcount zrevrange zcard zscore zmscore zrank zrevrank zscan hset hsetnx diff --git a/tests/unit/type/set.tcl b/tests/unit/type/set.tcl index 7b467f1c4..84c31c4e4 100644 --- a/tests/unit/type/set.tcl +++ b/tests/unit/type/set.tcl @@ -9,7 +9,7 @@ start_server { foreach entry $entries { r sadd $key $entry } } - test {SADD, SCARD, SISMEMBER, SMEMBERS basics - regular set} { + test {SADD, SCARD, SISMEMBER, SMISMEMBER, SMEMBERS basics - regular set} { create_set myset {foo} assert_encoding hashtable myset assert_equal 1 [r sadd myset bar] @@ -18,10 +18,15 @@ start_server { assert_equal 1 [r sismember myset foo] assert_equal 1 [r sismember myset bar] assert_equal 0 [r sismember myset bla] + assert_equal {1} [r smismember myset foo] + assert_equal {1 1} [r smismember myset foo bar] + assert_equal {1 0} [r smismember myset foo bla] + assert_equal {0 1} [r smismember myset bla foo] + assert_equal {0} [r smismember myset bla] assert_equal {bar foo} [lsort [r smembers myset]] } - test {SADD, SCARD, SISMEMBER, SMEMBERS basics - intset} { + test {SADD, SCARD, SISMEMBER, SMISMEMBER, SMEMBERS basics - intset} { create_set myset {17} assert_encoding intset myset assert_equal 1 [r sadd myset 16] @@ -30,9 +35,33 @@ start_server { assert_equal 1 [r sismember myset 16] assert_equal 1 [r sismember myset 17] assert_equal 0 [r sismember myset 18] + assert_equal {1} [r smismember myset 16] + assert_equal {1 1} [r smismember myset 16 17] + assert_equal {1 0} [r smismember myset 16 18] + assert_equal {0 1} [r smismember myset 18 16] + assert_equal {0} [r smismember myset 18] assert_equal {16 17} [lsort [r smembers myset]] } + test {SMISMEMBER against non set} { + r lpush mylist foo + assert_error WRONGTYPE* {r smismember mylist bar} + } + + test {SMISMEMBER non existing key} { + assert_equal {0} [r smismember myset1 foo] + assert_equal {0 0} [r smismember myset1 foo bar] + } + + test {SMISMEMBER requires one or more members} { + r del zmscoretest + r zadd zmscoretest 10 x + r zadd zmscoretest 20 y + + catch {r smismember zmscoretest} e + assert_match {*ERR*wrong*number*arg*} $e + } + test {SADD against non set} { r lpush mylist foo assert_error WRONGTYPE* {r sadd mylist bar} @@ -49,6 +78,7 @@ start_server { create_set myset {213244124402402314402033402} assert_encoding hashtable myset assert_equal 1 [r sismember myset 213244124402402314402033402] + assert_equal {1} [r smismember myset 213244124402402314402033402] } test "SADD overflows the maximum allowed integers in an intset" {