From 06f1feba36babf9b2c3f15324a0ede4f706c6bbc Mon Sep 17 00:00:00 2001 From: Rahul Desirazu Date: Wed, 9 May 2018 13:24:42 -0700 Subject: [PATCH] (#175) ENG 3108: Implement ZSCORE Summary: ZSCORE key member for sorted sets. Github issue #175. Test Plan: c++ and jedis client tests. Reviewers: pritam.damania, amitanand, hector Reviewed By: hector Subscribers: kannan, ybase Differential Revision: https://phabricator.dev.yugabyte.com/D4785 --- .../java/com/yugabyte/jedis/TestYBJedis.java | 30 +++++++++++++++++++ src/yb/common/redis_protocol.proto | 1 + src/yb/docdb/doc_operation.cc | 27 +++++++++++++++++ .../yql/redis/redisserver/redis_commands.cc | 1 + src/yb/yql/redis/redisserver/redis_parser.cc | 13 ++++++++ .../yql/redis/redisserver/redisserver-test.cc | 30 +++++++++++++++++++ 6 files changed, 102 insertions(+) diff --git a/java/yb-jedis-tests/src/test/java/com/yugabyte/jedis/TestYBJedis.java b/java/yb-jedis-tests/src/test/java/com/yugabyte/jedis/TestYBJedis.java index 00db34112b66..c0013fdbd831 100644 --- a/java/yb-jedis-tests/src/test/java/com/yugabyte/jedis/TestYBJedis.java +++ b/java/yb-jedis-tests/src/test/java/com/yugabyte/jedis/TestYBJedis.java @@ -59,6 +59,36 @@ public void testBasicCommands() throws Exception { assertEquals("v1", jedis_client.get("k1")); } + @Test + public void testZScore() throws Exception { + Map pairs = new HashMap(); + pairs.put("v0", 0.0); + pairs.put("v_neg", -5.0); + pairs.put("v1", 1.0); + pairs.put("v2", 2.5); + assertEquals(4L, jedis_client.zadd("z_key", pairs).longValue()); + + assertEquals(0.0, jedis_client.zscore("z_key", "v0")); + assertEquals(-5.0, jedis_client.zscore("z_key", "v_neg")); + assertEquals(1.0, jedis_client.zscore("z_key", "v1")); + assertEquals(2.5, jedis_client.zscore("z_key", "v2")); + + assertNull(jedis_client.zscore("z_key", "v3")); + assertNull(jedis_client.zscore("z_no_exist", "v0")); + + TSValuePairs tsPairs = new TSValuePairs((1)); + assertEquals("OK", jedis_client.tsadd("ts_key", tsPairs.pairs)); + + try { + double d = jedis_client.zscore("ts_key", "v0"); + } catch (Exception e) { + assertTrue(e.getMessage().contains( + "WRONGTYPE Operation against a key holding the wrong kind of value")); + return; + } + assertTrue(false); + } + @Test public void testSetCommandWithOptions() throws Exception { // Test with lowercase "ex" option. diff --git a/src/yb/common/redis_protocol.proto b/src/yb/common/redis_protocol.proto index 402db410f74f..d415ed154daa 100644 --- a/src/yb/common/redis_protocol.proto +++ b/src/yb/common/redis_protocol.proto @@ -192,6 +192,7 @@ message RedisGetRequestPB { ZCARD = 15; TSGET = 14; TSCARD = 16; + ZSCORE = 17; UNKNOWN = 99; } diff --git a/src/yb/docdb/doc_operation.cc b/src/yb/docdb/doc_operation.cc index 3e3420f63c20..544d588c7fc9 100644 --- a/src/yb/docdb/doc_operation.cc +++ b/src/yb/docdb/doc_operation.cc @@ -1394,6 +1394,8 @@ Status RedisReadOperation::ExecuteGet() { expected_type = REDIS_TYPE_HASH; break; case RedisGetRequestPB_GetRequestType_SISMEMBER: expected_type = REDIS_TYPE_SET; break; + case RedisGetRequestPB_GetRequestType_ZSCORE: + expected_type = REDIS_TYPE_SORTEDSET; break; default: expected_type = REDIS_TYPE_NONE; } @@ -1414,6 +1416,31 @@ Status RedisReadOperation::ExecuteGet() { } return Status::OK(); } + case RedisGetRequestPB_GetRequestType_ZSCORE: { + auto type = GetValueType(); + RETURN_NOT_OK(type); + // If wrong type, we set the error code in the response. + if (!VerifyTypeAndSetCode(expected_type, *type, &response_, VerifySuccessIfMissing::kTrue)) { + return Status::OK(); + } + SubDocKey key_reverse = SubDocKey( + DocKey::FromRedisKey(request_.key_value().hash_code(), request_.key_value().key()), + PrimitiveValue(ValueType::kSSReverse), + PrimitiveValue(request_.key_value().subkey(0).string_subkey())); + SubDocument subdoc_reverse; + bool subdoc_reverse_found = false; + auto encoded_key_reverse = key_reverse.EncodeWithoutHt(); + GetSubDocumentData get_data = { encoded_key_reverse, &subdoc_reverse, &subdoc_reverse_found }; + RETURN_NOT_OK(GetSubDocument(db_, get_data, redis_query_id(), + boost::none /* txn_op_context */, read_time_)); + if (subdoc_reverse_found) { + double score = subdoc_reverse.GetDouble(); + response_.set_string_response(std::to_string(score)); + } else { + response_.set_code(RedisResponsePB_RedisStatusCode_NIL); + } + return Status::OK(); + } case RedisGetRequestPB_GetRequestType_HEXISTS: FALLTHROUGH_INTENDED; case RedisGetRequestPB_GetRequestType_SISMEMBER: { auto type = GetValueType(); diff --git a/src/yb/yql/redis/redisserver/redis_commands.cc b/src/yb/yql/redis/redisserver/redis_commands.cc index ed21b1a44ac2..38dc55b51d89 100644 --- a/src/yb/yql/redis/redisserver/redis_commands.cc +++ b/src/yb/yql/redis/redisserver/redis_commands.cc @@ -68,6 +68,7 @@ namespace redisserver { ((zrangebyscore, ZRangeByScore, -4, READ)) \ ((zrevrange, ZRevRange, -4, READ)) \ ((zrange, ZRange, -4, READ)) \ + ((zscore, ZScore, 3, READ)) \ ((tsrem, TsRem, -3, WRITE)) \ ((zrem, ZRem, -3, WRITE)) \ ((zadd, ZAdd, -4, WRITE)) \ diff --git a/src/yb/yql/redis/redisserver/redis_parser.cc b/src/yb/yql/redis/redisserver/redis_parser.cc index 89f1e2d5f428..dcecdda6526f 100644 --- a/src/yb/yql/redis/redisserver/redis_parser.cc +++ b/src/yb/yql/redis/redisserver/redis_parser.cc @@ -736,6 +736,19 @@ CHECKED_STATUS ParseTsGet(YBRedisReadOp* op, const RedisClientCommand& args) { return Status::OK(); } +CHECKED_STATUS ParseZScore(YBRedisReadOp* op, const RedisClientCommand& args) { + op->mutable_request()->set_allocated_get_request(new RedisGetRequestPB()); + op->mutable_request()->mutable_get_request()->set_request_type( + RedisGetRequestPB_GetRequestType_ZSCORE); + + const auto& key = args[1]; + op->mutable_request()->mutable_key_value()->set_key(key.cdata(), key.size()); + auto member = args[2].ToBuffer(); + op->mutable_request()->mutable_key_value()->add_subkey()->set_string_subkey(member); + + return Status::OK(); +} + CHECKED_STATUS ParseHStrLen(YBRedisReadOp* op, const RedisClientCommand& args) { return ParseHGetLikeCommands(op, args, RedisGetRequestPB_GetRequestType_HSTRLEN); } diff --git a/src/yb/yql/redis/redisserver/redisserver-test.cc b/src/yb/yql/redis/redisserver/redisserver-test.cc index ce58a9bbc647..681132ba9302 100644 --- a/src/yb/yql/redis/redisserver/redisserver-test.cc +++ b/src/yb/yql/redis/redisserver/redisserver-test.cc @@ -181,6 +181,16 @@ class TestRedisService : public RedisTableTestBase { ); } + void DoRedisTestDouble(int line, const std::vector& command, double expected) { + DoRedisTest(line, command, RedisReplyType::kString, + [line, expected](const RedisReply& reply) { + std::string::size_type sz; + double reply_score = std::stod(reply.as_string(), &sz); + ASSERT_EQ(reply_score, expected) << "Originator: " << __FILE__ << ":" << line; + } + ); + } + // Used to check pairs of doubles and strings, for range scans withscores. void DoRedisTestScoreValueArray(int line, const std::vector& command, @@ -1700,6 +1710,26 @@ TEST_F(TestRedisService, TestZRange) { VerifyCallbacks(); } +TEST_F(TestRedisService, TestZScore) { + // The default value is true, but we explicitly set this here for clarity. + FLAGS_emulate_redis_responses = true; + DoRedisTestInt(__LINE__, {"ZADD", "z_multi", "0", "v0", "0", "v0_copy", "1", "v1", + "2", "v2", "3", "v3", "4.5", "v4"}, 6); + SyncClient(); + + DoRedisTestDouble(__LINE__, {"ZSCORE", "z_multi", "v0"}, 0.0); + DoRedisTestDouble(__LINE__, {"ZSCORE", "z_multi", "v0_copy"}, 0.0); + DoRedisTestDouble(__LINE__, {"ZSCORE", "z_multi", "v1"}, 1.0); + DoRedisTestDouble(__LINE__, {"ZSCORE", "z_multi", "v2"}, 2.0); + DoRedisTestDouble(__LINE__, {"ZSCORE", "z_multi", "v3"}, 3.0); + DoRedisTestDouble(__LINE__, {"ZSCORE", "z_multi", "v4"}, 4.5); + + DoRedisTestNull(__LINE__, {"ZSCORE", "z_no_exist", "v4"}); + + SyncClient(); + VerifyCallbacks(); +} + TEST_F(TestRedisService, TestTimeSeriesTTL) { int64_t ttl_sec = 10; TestTSTtl("EXPIRE_IN", ttl_sec, ttl_sec, "test_expire_in");