You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kvrocks.apache.org by tw...@apache.org on 2023/01/17 07:06:30 UTC
[incubator-kvrocks] branch unstable updated: Add ParseFloat and replace stod with it (#1227)
This is an automated email from the ASF dual-hosted git repository.
twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/incubator-kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 84776a63 Add ParseFloat and replace stod with it (#1227)
84776a63 is described below
commit 84776a63b18f108f670ad6a7f3cc27c31290f002
Author: Twice <tw...@gmail.com>
AuthorDate: Tue Jan 17 15:06:24 2023 +0800
Add ParseFloat and replace stod with it (#1227)
---
src/commands/redis_cmd.cc | 116 +++++++++++++++----------------
src/common/parse_util.h | 53 +++++++++++++-
src/types/redis_hash.cc | 12 ++--
src/types/redis_string.cc | 12 ++--
tests/cppunit/parse_util.cc | 21 ++++++
tests/gocase/unit/type/hash/hash_test.go | 4 +-
6 files changed, 139 insertions(+), 79 deletions(-)
diff --git a/src/commands/redis_cmd.cc b/src/commands/redis_cmd.cc
index dd1ee74b..28ecaa45 100644
--- a/src/commands/redis_cmd.cc
+++ b/src/commands/redis_cmd.cc
@@ -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,
@@ -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);
}
@@ -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);
}
@@ -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);
@@ -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);
}
@@ -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;
@@ -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};
@@ -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;
@@ -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;
@@ -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>"};
diff --git a/src/common/parse_util.h b/src/common/parse_util.h
index 86d81a7f..63c03c00 100644
--- a/src/common/parse_util.h
+++ b/src/common/parse_util.h
@@ -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 &&
@@ -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;
+}
diff --git a/src/types/redis_hash.cc b/src/types/redis_hash.cc
index b2bf589b..f2ea1c27 100644
--- a/src/types/redis_hash.cc
+++ b/src/types/redis_hash.cc
@@ -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;
}
}
diff --git a/src/types/redis_string.cc b/src/types/redis_string.cc
index faa96fbb..068aed75 100644
--- a/src/types/redis_string.cc
+++ b/src/types/redis_string.cc
@@ -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;
diff --git a/tests/cppunit/parse_util.cc b/tests/cppunit/parse_util.cc
index ed2c2f8b..07f494fb 100644
--- a/tests/cppunit/parse_util.cc
+++ b/tests/cppunit/parse_util.cc
@@ -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"));
+}
diff --git a/tests/gocase/unit/type/hash/hash_test.go b/tests/gocase/unit/type/hash/hash_test.go
index f8a23bc1..37277782 100644
--- a/tests/gocase/unit/type/hash/hash_test.go
+++ b/tests/gocase/unit/type/hash/hash_test.go
@@ -479,7 +479,7 @@ 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)
})
@@ -487,7 +487,7 @@ func TestHash(t *testing.T) {
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)
})