diff --git a/src/facade/reply_builder.cc b/src/facade/reply_builder.cc index a3993f8..31cd810 100644 --- a/src/facade/reply_builder.cc +++ b/src/facade/reply_builder.cc @@ -27,6 +27,11 @@ constexpr char kCRLF[] = "\r\n"; constexpr char kErrPref[] = "-ERR "; constexpr char kSimplePref[] = "+"; +constexpr unsigned kConvFlags = + DoubleToStringConverter::UNIQUE_ZERO | DoubleToStringConverter::EMIT_POSITIVE_EXPONENT_SIGN; + +DoubleToStringConverter dfly_conv(kConvFlags, "inf", "nan", 'e', -6, 21, 6, 0); + } // namespace SinkReplyBuilder::SinkReplyBuilder(::io::Sink* sink) : sink_(sink) { @@ -148,6 +153,13 @@ void MCReplyBuilder::SendNotFound() { SendSimpleString("NOT_FOUND"); } + +char* RedisReplyBuilder::FormatDouble(double val, char* dest, unsigned dest_len) { + StringBuilder sb(dest, dest_len); + CHECK(dfly_conv.ToShortest(val, &sb)); + return sb.Finalize(); +} + RedisReplyBuilder::RedisReplyBuilder(::io::Sink* sink) : SinkReplyBuilder(sink) { } @@ -232,7 +244,7 @@ void RedisReplyBuilder::SendLong(long num) { void RedisReplyBuilder::SendDouble(double val) { char buf[64]; StringBuilder sb(buf, sizeof(buf)); - CHECK(DoubleToStringConverter::EcmaScriptConverter().ToShortest(val, &sb)); + CHECK(dfly_conv.ToShortest(val, &sb)); SendBulkString(sb.Finalize()); } diff --git a/src/facade/reply_builder.h b/src/facade/reply_builder.h index 5555f72..edd83cf 100644 --- a/src/facade/reply_builder.h +++ b/src/facade/reply_builder.h @@ -140,6 +140,8 @@ class RedisReplyBuilder : public SinkReplyBuilder { virtual void StartArray(unsigned len); + static char* FormatDouble(double val, char* dest, unsigned dest_len); + private: }; diff --git a/src/server/hset_family.cc b/src/server/hset_family.cc index e2f6d2a..c594ba3 100644 --- a/src/server/hset_family.cc +++ b/src/server/hset_family.cc @@ -11,8 +11,6 @@ extern "C" { #include "redis/util.h" } -#include - #include "base/logging.h" #include "facade/error.h" #include "server/command_registry.h" @@ -25,7 +23,6 @@ using namespace std; namespace dfly { using namespace facade; -using namespace double_conversion; namespace { @@ -813,9 +810,7 @@ OpStatus HSetFamily::OpIncrBy(const OpArgs& op_args, string_view key, string_vie } char buf[128]; - StringBuilder sb(buf, sizeof(buf)); - CHECK(DoubleToStringConverter::EcmaScriptConverter().ToShortest(value, &sb)); - char* str = sb.Finalize(); + char* str = RedisReplyBuilder::FormatDouble(value, buf, sizeof(buf)); string_view sval{str}; if (hset->encoding == OBJ_ENCODING_LISTPACK) { diff --git a/src/server/string_family.cc b/src/server/string_family.cc index af0a7e8..5aef1c7 100644 --- a/src/server/string_family.cc +++ b/src/server/string_family.cc @@ -9,7 +9,7 @@ extern "C" { } #include -#include + #include #include "base/logging.h" @@ -740,11 +740,9 @@ OpResult StringFamily::OpIncrFloat(const OpArgs& op_args, std::string_vi auto [it, inserted] = db_slice.AddOrFind(op_args.db_ind, key); char buf[128]; - StringBuilder sb(buf, sizeof(buf)); if (inserted) { - CHECK(DoubleToStringConverter::EcmaScriptConverter().ToShortest(val, &sb)); - char* str = sb.Finalize(); + char* str = RedisReplyBuilder::FormatDouble(val, buf, sizeof(buf)); it->second.SetString(str); return val; @@ -772,11 +770,8 @@ OpResult StringFamily::OpIncrFloat(const OpArgs& op_args, std::string_vi return OpStatus::INVALID_FLOAT; } - if (!DoubleToStringConverter::EcmaScriptConverter().ToShortest(base, &sb)) { - return OpStatus::INVALID_FLOAT; - } + char* str = RedisReplyBuilder::FormatDouble(base, buf, sizeof(buf)); - char* str = sb.Finalize(); db_slice.PreUpdate(op_args.db_ind, it); it->second.SetString(str); db_slice.PostUpdate(op_args.db_ind, it); diff --git a/src/server/zset_family.cc b/src/server/zset_family.cc index dcc3a30..9e3a34f 100644 --- a/src/server/zset_family.cc +++ b/src/server/zset_family.cc @@ -790,26 +790,10 @@ void ZSetFamily::ZRevRangeByScore(CmdArgList args, ConnectionContext* cntx) { RangeParams range_params; range_params.reverse = true; + args.remove_prefix(4); - for (size_t i = 4; i < args.size(); ++i) { - ToUpper(&args[i]); - - string_view cur_arg = ArgS(args, i); - if (cur_arg == "WITHSCORES") { - range_params.with_scores = true; - } else if (cur_arg == "LIMIT") { - if (i + 3 != args.size()) - return (*cntx)->SendError(kSyntaxErr); - - string_view os = ArgS(args, i + 1); - string_view cs = ArgS(args, i + 2); - - if (!SimpleAtoi(os, &range_params.offset) || !SimpleAtoi(cs, &range_params.limit)) - return (*cntx)->SendError(kSyntaxErr); - i += 3; - } else { - return (*cntx)->SendError(absl::StrCat("unsupported option ", cur_arg), kSyntaxErrType); - } + if (!ParseRangeByScoreParams(args, &range_params)) { + return (*cntx)->SendError(kSyntaxErr); } ZRangeByScoreInternal(key, min_s, max_s, range_params, cntx); @@ -865,16 +849,10 @@ void ZSetFamily::ZRangeByScore(CmdArgList args, ConnectionContext* cntx) { string_view max_s = ArgS(args, 3); RangeParams range_params; + args.remove_prefix(4); - for (size_t i = 4; i < args.size(); ++i) { - ToUpper(&args[i]); - - string_view cur_arg = ArgS(args, i); - if (cur_arg == "WITHSCORES") { - range_params.with_scores = true; - } else { - return cntx->reply_builder()->SendError(absl::StrCat("unsupported option ", cur_arg)); - } + if (!ParseRangeByScoreParams(args, &range_params)) { + return (*cntx)->SendError(kSyntaxErr); } ZRangeByScoreInternal(key, min_s, max_s, range_params, cntx); @@ -1122,6 +1100,31 @@ void ZSetFamily::ZRankGeneric(CmdArgList args, bool reverse, ConnectionContext* } } +bool ZSetFamily::ParseRangeByScoreParams(CmdArgList args, RangeParams* params) { + for (size_t i = 0; i < args.size(); ++i) { + ToUpper(&args[i]); + + string_view cur_arg = ArgS(args, i); + if (cur_arg == "WITHSCORES") { + params->with_scores = true; + } else if (cur_arg == "LIMIT") { + if (i + 3 != args.size()) + return false; + + string_view os = ArgS(args, i + 1); + string_view cs = ArgS(args, i + 2); + + if (!SimpleAtoi(os, ¶ms->offset) || !SimpleAtoi(cs, ¶ms->limit)) + return false; + i += 3; + } else { + return false; + } + } + + return true; +} + OpResult ZSetFamily::OpScan(const OpArgs& op_args, std::string_view key, uint64_t* cursor) { OpResult find_res = op_args.shard->db_slice().Find(op_args.db_ind, key, OBJ_ZSET); @@ -1143,11 +1146,10 @@ OpResult ZSetFamily::OpScan(const OpArgs& op_args, std::string_view k res.resize(arr.size() * 2); for (size_t i = 0; i < arr.size(); ++i) { - StringBuilder sb(buf, sizeof(buf)); - CHECK(DoubleToStringConverter::EcmaScriptConverter().ToShortest(arr[i].second, &sb)); + char* str = RedisReplyBuilder::FormatDouble(arr[i].second, buf, sizeof(buf)); res[2 * i] = std::move(arr[i].first); - res[2 * i + 1].assign(sb.Finalize()); + res[2 * i + 1].assign(str); } *cursor = 0; } else { @@ -1170,10 +1172,8 @@ OpResult ZSetFamily::OpScan(const OpArgs& op_args, std::string_view k double score = *(double*)dictGetVal(de); sargs->res->emplace_back(key, sdslen(key)); - - StringBuilder sb(sargs->sbuf, sizeof(buf)); - CHECK(DoubleToStringConverter::EcmaScriptConverter().ToShortest(score, &sb)); - sargs->res->emplace_back(sb.Finalize()); + char* str = RedisReplyBuilder::FormatDouble(score, sargs->sbuf, sizeof(buf)); + sargs->res->emplace_back(str); }; do { @@ -1480,7 +1480,7 @@ void ZSetFamily::Register(CommandRegistry* registry) { << CI{"ZREMRANGEBYSCORE", CO::WRITE, 4, 1, 1, 1}.HFUNC(ZRemRangeByScore) << CI{"ZREMRANGEBYLEX", CO::WRITE, 4, 1, 1, 1}.HFUNC(ZRemRangeByLex) << CI{"ZREVRANGE", CO::READONLY, 4, 1, 1, 1}.HFUNC(ZRevRange) - << CI{"ZREVRANGEBYSCORE", CO::READONLY, 4, 1, 1, 1}.HFUNC(ZRevRangeByScore) + << CI{"ZREVRANGEBYSCORE", CO::READONLY, -4, 1, 1, 1}.HFUNC(ZRevRangeByScore) << CI{"ZREVRANK", CO::READONLY | CO::FAST, 3, 1, 1, 1}.HFUNC(ZRevRank) << CI{"ZSCAN", CO::READONLY | CO::RANDOM, -3, 1, 1, 1}.HFUNC(ZScan); } diff --git a/src/server/zset_family.h b/src/server/zset_family.h index 188efe2..d807b1c 100644 --- a/src/server/zset_family.h +++ b/src/server/zset_family.h @@ -80,6 +80,8 @@ class ZSetFamily { ConnectionContext* cntx); static void ZRangeGeneric(CmdArgList args, bool reverse, ConnectionContext* cntx); static void ZRankGeneric(CmdArgList args, bool reverse, ConnectionContext* cntx); + static bool ParseRangeByScoreParams(CmdArgList args, RangeParams* params); + static OpResult OpScan(const OpArgs& op_args, std::string_view key, uint64_t* cursor); struct ZParams { diff --git a/src/server/zset_family_test.cc b/src/server/zset_family_test.cc index 5f70d10..233c4a6 100644 --- a/src/server/zset_family_test.cc +++ b/src/server/zset_family_test.cc @@ -68,9 +68,9 @@ TEST_F(ZSetFamilyTest, ZRem) { TEST_F(ZSetFamilyTest, ZRangeRank) { Run({"zadd", "x", "1.1", "a", "2.1", "b"}); EXPECT_THAT(Run({"zrangebyscore", "x", "0", "(1.1"}), ArrLen(0)); - EXPECT_THAT(Run({"zrangebyscore", "x", "-inf", "1.1"}), "a"); + EXPECT_THAT(Run({"zrangebyscore", "x", "-inf", "1.1", "limit", "0", "10"}), "a"); - auto resp = Run({"zrevrangebyscore", "x", "+inf", "-inf"}); + auto resp = Run({"zrevrangebyscore", "x", "+inf", "-inf", "limit", "0", "5"}); ASSERT_THAT(resp, ArgType(RespExpr::ARRAY)); ASSERT_THAT(resp.GetVec(), ElementsAre("b", "a")); @@ -137,7 +137,12 @@ TEST_F(ZSetFamilyTest, ByLex) { TEST_F(ZSetFamilyTest, ZRevRange) { Run({"zadd", "key", "-inf", "a", "1", "b", "2", "c"}); auto resp = Run({"zrevrangebyscore", "key", "2", "-inf"}); - EXPECT_THAT(resp, ArrLen(3)); + ASSERT_THAT(resp, ArrLen(3)); + EXPECT_THAT(resp.GetVec(), ElementsAre("c", "b", "a")); + + resp = Run({"zrevrangebyscore", "key", "2", "-inf", "withscores"}); + ASSERT_THAT(resp, ArrLen(6)); + EXPECT_THAT(resp.GetVec(), ElementsAre("c", "2", "b", "1", "a", "-inf")); } TEST_F(ZSetFamilyTest, ZScan) {