Skip to content

Commit

Permalink
Add ParseFloat and replace stod with it (#1227)
Browse files Browse the repository at this point in the history
  • Loading branch information
@PragmaTwice
PragmaTwice authored Jan 17, 2023
1 parent 003d8fb commit 84776a6
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 79 deletions.
116 changes: 56 additions & 60 deletions src/commands/redis_cmd.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,23 +64,23 @@

namespace Redis {

const char *kCursorPrefix = "_";

const char *errInvalidSyntax = "syntax error";
const char *errInvalidExpireTime = "invalid expire time";
const char *errWrongNumOfArguments = "wrong number of arguments";
const char *errValueNotInteger = "value is not an integer or out of range";
const char *errAdministorPermissionRequired = "administor permission required to perform the command";
const char *errValueMustBePositive = "value is out of range, must be positive";
const char *errNoSuchKey = "no such key";
const char *errUnbalancedStreamList =
constexpr const char *kCursorPrefix = "_";

constexpr const char *errInvalidSyntax = "syntax error";
constexpr const char *errInvalidExpireTime = "invalid expire time";
constexpr const char *errWrongNumOfArguments = "wrong number of arguments";
constexpr const char *errValueNotInteger = "value is not an integer or out of range";
constexpr const char *errAdministorPermissionRequired = "administor permission required to perform the command";
constexpr const char *errValueMustBePositive = "value is out of range, must be positive";
constexpr const char *errNoSuchKey = "no such key";
constexpr const char *errUnbalancedStreamList =
"Unbalanced XREAD list of streams: for each stream key an ID or '$' must be specified.";
const char *errTimeoutIsNegative = "timeout is negative";
const char *errLimitOptionNotAllowed = "syntax error, LIMIT cannot be used without the special ~ option";
const char *errZSetLTGTNX = "GT, LT, and/or NX options at the same time are not compatible";
const char *errScoreIsNotValidFloat = "score is not a valid float";
const char *errValueIsNotFloat = "value is not a valid float";
const char *errNoMatchingScript = "NOSCRIPT No matching script. Please use EVAL";
constexpr const char *errTimeoutIsNegative = "timeout is negative";
constexpr const char *errLimitOptionNotAllowed = "syntax error, LIMIT cannot be used without the special ~ option";
constexpr const char *errZSetLTGTNX = "GT, LT, and/or NX options at the same time are not compatible";
constexpr const char *errScoreIsNotValidFloat = "score is not a valid float";
constexpr const char *errValueIsNotFloat = "value is not a valid float";
constexpr const char *errNoMatchingScript = "NOSCRIPT No matching script. Please use EVAL";

enum class AuthResult {
OK,
Expand Down Expand Up @@ -812,11 +812,11 @@ class CommandIncrBy : public Commander {
class CommandIncrByFloat : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
try {
increment_ = std::stod(args[2]);
} catch (std::exception &e) {
return {Status::RedisParseErr, errValueNotInteger};
auto increment = ParseFloat(args[2]);
if (!increment) {
return {Status::RedisParseErr, errValueIsNotFloat};
}
increment_ = *increment;

return Commander::Parse(args);
}
Expand Down Expand Up @@ -1469,11 +1469,11 @@ class CommandHIncrBy : public Commander {
class CommandHIncrByFloat : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
try {
increment_ = std::stod(args[3]);
} catch (std::exception &e) {
return {Status::RedisParseErr, errValueNotInteger};
auto increment = ParseFloat(args[3]);
if (!increment) {
return {Status::RedisParseErr, errValueIsNotFloat};
}
increment_ = *increment;
return Commander::Parse(args);
}

Expand Down Expand Up @@ -2530,17 +2530,16 @@ class CommandZAdd : public Commander {
}
}

try {
for (size_t i = index; i < args.size(); i += 2) {
double score = std::stod(args[i]);
if (std::isnan(score)) {
return {Status::RedisParseErr, errScoreIsNotValidFloat};
}

member_scores_.emplace_back(MemberScore{args[i + 1], score});
for (size_t i = index; i < args.size(); i += 2) {
auto score = ParseFloat(args[i]);
if (!score) {
return {Status::RedisParseErr, errValueIsNotFloat};
}
} catch (const std::exception &e) {
return {Status::RedisParseErr, errValueIsNotFloat};
if (std::isnan(*score)) {
return {Status::RedisParseErr, errScoreIsNotValidFloat};
}

member_scores_.emplace_back(MemberScore{args[i + 1], *score});
}

return Commander::Parse(args);
Expand Down Expand Up @@ -2652,11 +2651,11 @@ class CommandZCard : public Commander {
class CommandZIncrBy : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
try {
incr_ = std::stod(args[2]);
} catch (const std::exception &e) {
return {Status::RedisParseErr, "value is not an double or out of range"};
auto increment = ParseFloat(args[2]);
if (!increment) {
return {Status::RedisParseErr, errValueIsNotFloat};
}
incr_ = *increment;
return Commander::Parse(args);
}

Expand Down Expand Up @@ -3145,14 +3144,12 @@ class CommandZUnionStore : public Commander {
} else if (Util::ToLower(args[i]) == "weights" && i + numkeys_ < args.size()) {
size_t k = 0;
while (k < numkeys_) {
try {
keys_weights_[k].weight = std::stod(args[i + k + 1]);
} catch (const std::exception &e) {
return {Status::RedisParseErr, "weight is not an double or out of range"};
}
if (std::isnan(keys_weights_[k].weight)) {
return {Status::RedisParseErr, "weight is not an double or out of range"};
auto weight = ParseFloat(args[i + k + 1]);
if (!weight || std::isnan(*weight)) {
return {Status::RedisParseErr, "weight is not a double or out of range"};
}
keys_weights_[k].weight = *weight;

k++;
}
i += numkeys_ + 1;
Expand Down Expand Up @@ -3217,12 +3214,13 @@ class CommandGeoBase : public Commander {

Status ParseLongLat(const std::string &longitude_para, const std::string &latitude_para, double *longitude,
double *latitude) {
try {
*longitude = std::stod(longitude_para);
*latitude = std::stod(latitude_para);
} catch (const std::exception &e) {
auto long_stat = ParseFloat(longitude_para);
auto lat_stat = ParseFloat(latitude_para);
if (!long_stat || !lat_stat) {
return {Status::RedisParseErr, errValueIsNotFloat};
}
*longitude = *long_stat;
*latitude = *lat_stat;

if (*longitude < GEO_LONG_MIN || *longitude > GEO_LONG_MAX || *latitude < GEO_LAT_MIN || *latitude > GEO_LAT_MAX) {
return {Status::RedisParseErr, "invalid longitude,latitude pair " + longitude_para + "," + latitude_para};
Expand Down Expand Up @@ -3390,11 +3388,11 @@ class CommandGeoRadius : public CommandGeoBase {
auto s = ParseLongLat(args[2], args[3], &longitude_, &latitude_);
if (!s.IsOK()) return s;

try {
radius_ = std::stod(args[4]);
} catch (const std::exception &e) {
auto radius = ParseFloat(args[4]);
if (!radius) {
return {Status::RedisParseErr, errValueIsNotFloat};
}
radius_ = *radius;

s = ParseDistanceUnit(args[5]);
if (!s.IsOK()) return s;
Expand Down Expand Up @@ -3521,11 +3519,11 @@ class CommandGeoRadiusByMember : public CommandGeoRadius {
CommandGeoRadiusByMember() : CommandGeoRadius() {}

Status Parse(const std::vector<std::string> &args) override {
try {
radius_ = std::stod(args[3]);
} catch (const std::exception &e) {
auto radius = ParseFloat(args[3]);
if (!radius) {
return {Status::RedisParseErr, errValueIsNotFloat};
}
radius_ = *radius;

auto s = ParseDistanceUnit(args[4]);
if (!s.IsOK()) return s;
Expand Down Expand Up @@ -4550,14 +4548,12 @@ class CommandDebug : public Commander {
Status Parse(const std::vector<std::string> &args) override {
subcommand_ = Util::ToLower(args[1]);
if ((subcommand_ == "sleep") && args.size() == 3) {
double second = 0.0;
try {
second = std::stod(args[2]);
} catch (const std::exception &e) {
auto second = ParseFloat(args[2]);
if (!second) {
return {Status::RedisParseErr, "invalid debug sleep time"};
}

microsecond_ = static_cast<uint64_t>(second * 1000 * 1000);
microsecond_ = static_cast<uint64_t>(*second * 1000 * 1000);
return Status::OK();
}
return {Status::RedisInvalidCmd, "Syntax error, DEBUG SLEEP <seconds>"};
Expand Down
53 changes: 52 additions & 1 deletion src/common/parse_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ StatusOr<ParseResultAndPos<T>> TryParseInt(const char *v, int base = 0) {
}

if (errno) {
return {Status::NotOK, std::strerror(errno)};
return Status::FromErrno();
}

if (!std::is_same<T, decltype(res)>::value &&
Expand Down Expand Up @@ -144,3 +144,54 @@ StatusOr<T> ParseInt(const std::string &v, NumericRange<T> range, int base = 0)

// available units: K, M, G, T, P
StatusOr<std::uint64_t> ParseSizeAndUnit(const std::string &v);

template <typename>
struct ParseFloatFunc;

template <>
struct ParseFloatFunc<float> {
constexpr static const auto value = strtof;
};

template <>
struct ParseFloatFunc<double> {
constexpr static const auto value = strtod;
};

template <>
struct ParseFloatFunc<long double> {
constexpr static const auto value = strtold;
};

// TryParseFloat parses a string to a floating-point number,
// it returns the first unmatched character position instead of an error status
template <typename T = double> // float or double
StatusOr<ParseResultAndPos<T>> TryParseFloat(const char *str) {
char *end = nullptr;

errno = 0;
T result = ParseFloatFunc<T>::value(str, &end);

if (str == end) {
return {Status::NotOK, "not started as a number"};
}

if (errno) {
return Status::FromErrno();
}

return {result, end};
}

// ParseFloat parses a string to a floating-point number
template <typename T = double> // float or double
StatusOr<T> ParseFloat(const std::string &str) {
const char *begin = str.c_str();
auto [result, pos] = GET_OR_RET(TryParseFloat<T>(begin));

if (pos != begin + str.size()) {
return {Status::NotOK, "encounter non-number characters"};
}

return result;
}
12 changes: 4 additions & 8 deletions src/types/redis_hash.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,18 +127,14 @@ rocksdb::Status Hash::IncrByFloat(const Slice &user_key, const Slice &field, dou
InternalKey(ns_key, field, metadata.version, storage_->IsSlotIdEncoded()).Encode(&sub_key);
if (s.ok()) {
std::string value_bytes;
std::size_t idx = 0;
s = db_->Get(rocksdb::ReadOptions(), sub_key, &value_bytes);
if (!s.ok() && !s.IsNotFound()) return s;
if (s.ok()) {
try {
old_value = std::stod(value_bytes, &idx);
} catch (std::exception &e) {
return rocksdb::Status::InvalidArgument(e.what());
}
if (isspace(value_bytes[0]) || idx != value_bytes.size()) {
return rocksdb::Status::InvalidArgument("value is not an float");
auto value_stat = ParseFloat(value_bytes);
if (!value_stat || isspace(value_bytes[0])) {
return rocksdb::Status::InvalidArgument("value is not a number");
}
old_value = *value_stat;
exists = true;
}
}
Expand Down
12 changes: 4 additions & 8 deletions src/types/redis_string.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,16 +334,12 @@ rocksdb::Status String::IncrByFloat(const std::string &user_key, double incremen
}
value = raw_value.substr(STRING_HDR_SIZE, raw_value.size() - STRING_HDR_SIZE);
double n = 0;
std::size_t idx = 0;
if (!value.empty()) {
try {
n = std::stod(value, &idx);
} catch (std::exception &e) {
return rocksdb::Status::InvalidArgument("value is not an float");
}
if (isspace(value[0]) || idx != value.size()) {
return rocksdb::Status::InvalidArgument("value is not an float");
auto n_stat = ParseFloat(value);
if (!n_stat || isspace(value[0])) {
return rocksdb::Status::InvalidArgument("value is not a number");
}
n = *n_stat;
}

n += increment;
Expand Down
21 changes: 21 additions & 0 deletions tests/cppunit/parse_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,24 @@ TEST(ParseUtil, ParseSizeAndUnit) {
ASSERT_FALSE(ParseSizeAndUnit("16384p"));
ASSERT_EQ(ParseSizeAndUnit("16388p").Msg(), "arithmetic overflow");
}

TEST(ParseUtil, ParseFloat) {
std::string v = "1.23";
ASSERT_EQ(*TryParseFloat(v.c_str()), ParseResultAndPos<double>(1.23, v.c_str() + v.size()));

v = "25345.346e65hello";
ASSERT_EQ(*TryParseFloat(v.c_str()), ParseResultAndPos<double>(25345.346e65, v.c_str() + v.size() - 5));

ASSERT_FALSE(TryParseFloat("eeeeeeee"));
ASSERT_FALSE(TryParseFloat(" "));
ASSERT_FALSE(TryParseFloat(""));
ASSERT_FALSE(TryParseFloat(" abcd"));

v = " 1e8 ";
ASSERT_EQ(*TryParseFloat(v.c_str()), ParseResultAndPos<double>(1e8, v.c_str() + v.size() - 3));

ASSERT_EQ(*ParseFloat("1.23"), 1.23);
ASSERT_EQ(*ParseFloat("1.23e2"), 1.23e2);
ASSERT_FALSE(ParseFloat("1.2 "));
ASSERT_FALSE(ParseFloat("1.2hello"));
}
4 changes: 2 additions & 2 deletions tests/gocase/unit/type/hash/hash_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,15 +479,15 @@ func TestHash(t *testing.T) {
t.Run("HINCRBYFLOAT fails against hash value with spaces (left)", func(t *testing.T) {
rdb.HSet(ctx, "samllhash", "str", " 11")
rdb.HSet(ctx, "bighash", "str", " 11")
pattern := "ERR.*not.*float.*"
pattern := "ERR.*not.*number.*"
util.ErrorRegexp(t, rdb.HIncrByFloat(ctx, "samllhash", "str", 1).Err(), pattern)
util.ErrorRegexp(t, rdb.HIncrByFloat(ctx, "bighash", "str", 1).Err(), pattern)
})

t.Run("HINCRBYFLOAT fails against hash value with spaces (right)", func(t *testing.T) {
rdb.HSet(ctx, "samllhash", "str", "11 ")
rdb.HSet(ctx, "bighash", "str", "11 ")
pattern := "ERR.*not.*float.*"
pattern := "ERR.*not.*number.*"
util.ErrorRegexp(t, rdb.HIncrByFloat(ctx, "samllhash", "str", 1).Err(), pattern)
util.ErrorRegexp(t, rdb.HIncrByFloat(ctx, "bighash", "str", 1).Err(), pattern)
})
Expand Down

0 comments on commit 84776a6

Please sign in to comment.