Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ParseFloat and replace stod with it #1227

Merged
merged 6 commits into from
Jan 17, 2023
Merged

Add ParseFloat and replace stod with it #1227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5d7c3d7
Add ParseFloat and replace stod with it
PragmaTwice Jan 16, 2023
370836d
add unit tests
PragmaTwice Jan 16, 2023
be65233
use strto*
PragmaTwice Jan 17, 2023
21a84c3
replace more stod
PragmaTwice Jan 17, 2023
62bfd96
fix test
PragmaTwice Jan 17, 2023
4ffcd4d
Merge branch 'unstable' into parse-float
PragmaTwice Jan 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 56 additions & 60 deletions src/commands/redis_cmd.cc
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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