Skip to content
This repository has been archived by the owner on Dec 1, 2022. It is now read-only.

Commit

Permalink
v2.5.0 cherry-pick some bug fix from master. (#597)
Browse files Browse the repository at this point in the history 
* Modify the return value of partId function (#591)

* fix and simplify SubscriptRangeExpression's eval (#593)

* Fix toInteger() (#595)

* Fix toInteger/toFloat parsing string

* Check toInteger() overflow when  parsing string

Co-authored-by: laura-ding <[email protected]>
Co-authored-by: jie.wang <[email protected]>
Co-authored-by: Yichen Wang <[email protected]>
  • Loading branch information
@CPWstatic @laura-ding
@jievince @Aiee
4 people authored Aug 2, 2021
1 parent 7d9ad67 commit 5e9d15e
Show file tree
Hide file tree
Showing 7 changed files with 172 additions and 108 deletions.
2 changes: 1 addition & 1 deletion src/common/clients/meta/MetaClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1030,7 +1030,7 @@ void MetaClient::loadRemoteListeners() {

/// ================================== public methods =================================

StatusOr<PartitionID> MetaClient::partId(int32_t numParts, const VertexID id) const {
PartitionID MetaClient::partId(int32_t numParts, const VertexID id) const {
// If the length of the id is 8, we will treat it as int64_t to be compatible
// with the version 1.0
uint64_t vid = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/common/clients/meta/MetaClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ class MetaClient {

StatusOr<int32_t> partsNum(GraphSpaceID spaceId) const;

StatusOr<PartitionID> partId(int32_t numParts, VertexID id) const;
PartitionID partId(int32_t numParts, VertexID id) const;

StatusOr<std::shared_ptr<const NebulaSchemaProvider>>
getTagSchemaFromCache(GraphSpaceID spaceId, TagID tagID, SchemaVer ver = -1);
Expand Down
7 changes: 2 additions & 5 deletions src/common/clients/storage/InternalStorageClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,10 @@ folly::SemiFuture<ErrOrVal> InternalStorageClient::getValue(size_t vIdLen,
folly::StringPiece key,
folly::EventBase* evb) {
auto srcVid = key.subpiece(sizeof(PartitionID), vIdLen);
auto stPartId = metaClient_->partId(spaceId, srcVid.str());
if (!stPartId.ok()) {
return nebula::cpp2::ErrorCode::E_SPACE_NOT_FOUND;
}
auto partId = metaClient_->partId(spaceId, srcVid.str());

auto c = folly::makePromiseContract<ErrOrVal>();
getValueImpl(spaceId, stPartId.value(), key.str(), std::move(c.first), evb);
getValueImpl(spaceId, partId, key.str(), std::move(c.first), evb);
return std::move(c.second);
}

Expand Down
19 changes: 15 additions & 4 deletions src/common/datatypes/Value.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1595,7 +1595,7 @@ Value Value::toFloat() const {
}
case Value::Type::STRING: {
const auto& str = getStr();
char *pEnd;
char* pEnd;
double val = strtod(str.c_str(), &pEnd);
if (*pEnd != '\0') {
return Value::kNullValue;
Expand Down Expand Up @@ -1629,12 +1629,23 @@ Value Value::toInt() const {
}
case Value::Type::STRING: {
const auto& str = getStr();
char *pEnd;
double val = strtod(str.c_str(), &pEnd);
errno = 0;
char* pEnd;
auto it = std::find(str.begin(), str.end(), '.');
int64_t val =
(it == str.end())
? int64_t(strtoll(str.c_str(), &pEnd, 10)) // string representing int
: int64_t(strtod(str.c_str(), &pEnd)); // string representing double
if (*pEnd != '\0') {
return Value::kNullValue;
}
return static_cast<int64_t>(val);
// Check overflow
if ((val == std::numeric_limits<int64_t>::max() ||
val == std::numeric_limits<int64_t>::min()) &&
errno == ERANGE) {
return kNullOverflow;
}
return val;
}
default: {
return Value::kNullBadType;
Expand Down
166 changes: 116 additions & 50 deletions src/common/datatypes/test/ValueTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -638,76 +638,142 @@ TEST(Value, TypeCast) {
EXPECT_EQ(Value::Type::NULLVALUE, vb8.type());
}
{
auto vf1 = vInt.toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf1.type());
EXPECT_EQ(vf1.getFloat(), 1.0);
auto vf = vInt.toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf.type());
EXPECT_EQ(vf.getFloat(), 1.0);

auto vf2 = vFloat.toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf2.type());
EXPECT_EQ(vf2.getFloat(), 3.14);
vf = vFloat.toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf.type());
EXPECT_EQ(vf.getFloat(), 3.14);

auto vf3 = vStr1.toFloat();
EXPECT_EQ(Value::Type::NULLVALUE, vf3.type());
vf = vStr1.toFloat();
EXPECT_EQ(Value::Type::NULLVALUE, vf.type());

auto vf4 = vStr2.toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf4.type());
EXPECT_EQ(vf4.getFloat(), 3.14);
vf = vStr2.toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf.type());
EXPECT_EQ(vf.getFloat(), 3.14);

auto vf5 = vBool1.toFloat();
EXPECT_EQ(Value::Type::NULLVALUE, vf5.type());
vf = vBool1.toFloat();
EXPECT_EQ(Value::Type::NULLVALUE, vf.type());

auto vf6 = vBool2.toFloat();
EXPECT_EQ(Value::Type::NULLVALUE, vf6.type());
vf = vBool2.toFloat();
EXPECT_EQ(Value::Type::NULLVALUE, vf.type());

auto vf7 = vDate.toFloat();
EXPECT_EQ(Value::Type::NULLVALUE, vf7.type());
vf = vDate.toFloat();
EXPECT_EQ(Value::Type::NULLVALUE, vf.type());

auto vf8 = vNull.toFloat();
EXPECT_EQ(Value::Type::NULLVALUE, vf8.type());
vf = vNull.toFloat();
EXPECT_EQ(Value::Type::NULLVALUE, vf.type());

auto vf9 = vIntMin.toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf9.type());
EXPECT_EQ(vf9.getFloat(), std::numeric_limits<int64_t>::min());
vf = vIntMin.toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf.type());
EXPECT_EQ(vf.getFloat(), std::numeric_limits<int64_t>::min());

auto vf10 = vIntMax.toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf10.type());
EXPECT_EQ(vf10.getFloat(), std::numeric_limits<int64_t>::max());
vf = vIntMax.toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf.type());
EXPECT_EQ(vf.getFloat(), static_cast<double>(std::numeric_limits<int64_t>::max()));

// String of int
vf = Value("-9223372036854775807").toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf.type());
EXPECT_EQ(vf.getFloat(), static_cast<double>(std::numeric_limits<int64_t>::min() + 1));

vf = Value("-9223372036854775808")
.toFloat(); // will be converted to -9223372036854776000.0
EXPECT_EQ(Value::Type::FLOAT, vf.type());
EXPECT_EQ(vf.getFloat(), static_cast<double>(std::numeric_limits<int64_t>::min()));

vf = Value("9223372036854775807").toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf.type());
EXPECT_EQ(vf.getFloat(), static_cast<double>(std::numeric_limits<int64_t>::max()));

// String of double
vf = Value("123.").toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf.type());
EXPECT_EQ(vf.getFloat(), 123.0);

vf = Value(std::to_string(std::numeric_limits<double_t>::max())).toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf.type());
EXPECT_EQ(vf.getFloat(), std::numeric_limits<double_t>::max());

vf = Value(std::to_string(std::numeric_limits<double_t>::max())).toFloat();
EXPECT_EQ(Value::Type::FLOAT, vf.type());
EXPECT_EQ(vf.getFloat(), std::numeric_limits<double_t>::max());

// Invlaid string
vf = Value("12abc").toFloat();
EXPECT_EQ(Value::kNullValue, vf);

vf = Value("1.2abc").toFloat();
EXPECT_EQ(Value::kNullValue, vf);
}
{
auto vi1 = vInt.toInt();
EXPECT_EQ(Value::Type::INT, vi1.type());
EXPECT_EQ(vi1.getInt(), 1);
auto vi = vInt.toInt();
EXPECT_EQ(Value::Type::INT, vi.type());
EXPECT_EQ(vi.getInt(), 1);

vi = vFloat.toInt();
EXPECT_EQ(Value::Type::INT, vi.type());
EXPECT_EQ(vi.getInt(), 3);

vi = vStr1.toInt();
EXPECT_EQ(Value::Type::NULLVALUE, vi.type());

vi = vStr2.toInt();

EXPECT_EQ(Value::Type::INT, vi.type());
EXPECT_EQ(vi.getInt(), 3);

vi = vBool1.toInt();
EXPECT_EQ(Value::Type::NULLVALUE, vi.type());

vi = vBool2.toInt();
EXPECT_EQ(Value::Type::NULLVALUE, vi.type());

vi = vDate.toInt();
EXPECT_EQ(Value::Type::NULLVALUE, vi.type());

vi = vNull.toInt();
EXPECT_EQ(Value::Type::NULLVALUE, vi.type());

vi = vFloatMin.toInt();
EXPECT_EQ(Value::Type::INT, vi.type());
EXPECT_EQ(vi.getInt(), std::numeric_limits<int64_t>::min());

auto vi2 = vFloat.toInt();
EXPECT_EQ(Value::Type::INT, vi2.type());
EXPECT_EQ(vi2.getInt(), 3);
vi = vFloatMax.toInt();
EXPECT_EQ(Value::Type::INT, vi.type());
EXPECT_EQ(vi.getInt(), std::numeric_limits<int64_t>::max());

auto vi3 = vStr1.toInt();
EXPECT_EQ(Value::Type::NULLVALUE, vi3.type());
// string of int
vi = Value("123.").toInt();
EXPECT_EQ(Value::Type::INT, vi.type());
EXPECT_EQ(vi.getInt(), 123);

auto vi4 = vStr2.toInt();
EXPECT_EQ(Value::Type::INT, vi4.type());
EXPECT_EQ(vi4.getInt(), 3);
vi = Value("-9223372036854775807").toInt();
EXPECT_EQ(Value::Type::INT, vi.type());
EXPECT_EQ(vi.getInt(), std::numeric_limits<int64_t>::min() + 1);

auto vi5 = vBool1.toInt();
EXPECT_EQ(Value::Type::NULLVALUE, vi5.type());
vi = Value("-9223372036854775808").toInt();
EXPECT_EQ(Value::Type::INT, vi.type());
EXPECT_EQ(vi.getInt(), std::numeric_limits<int64_t>::min());

auto vi6 = vBool2.toInt();
EXPECT_EQ(Value::Type::NULLVALUE, vi6.type());
vi = Value("9223372036854775807").toInt();
EXPECT_EQ(Value::Type::INT, vi.type());
EXPECT_EQ(vi.getInt(), std::numeric_limits<int64_t>::max());

auto vi7 = vDate.toInt();
EXPECT_EQ(Value::Type::NULLVALUE, vi7.type());
// String to int Overflow
vi = Value("9223372036854775808").toInt();
EXPECT_EQ(Value::kNullOverflow, vi);

auto vi8 = vNull.toInt();
EXPECT_EQ(Value::Type::NULLVALUE, vi8.type());
vi = Value("-9223372036854775809").toInt();
EXPECT_EQ(Value::kNullOverflow, vi);

auto vi9 = vFloatMin.toInt();
EXPECT_EQ(Value::Type::INT, vi9.type());
EXPECT_EQ(vi9.getInt(), std::numeric_limits<int64_t>::min());
// Invlaid string
vi = Value("12abc").toInt();
EXPECT_EQ(Value::kNullValue, vi);

auto vi10 = vFloatMax.toInt();
EXPECT_EQ(Value::Type::INT, vi10.type());
EXPECT_EQ(vi10.getInt(), std::numeric_limits<int64_t>::max());
vi = Value("1.2abc").toInt();
EXPECT_EQ(Value::kNullValue, vi);
}
}

Expand Down
75 changes: 28 additions & 47 deletions src/common/expression/SubscriptExpression.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,26 +120,17 @@ void SubscriptExpression::accept(ExprVisitor *visitor) {
// For the positive range bound it start from begin,
// for the negative range bound it start from end
const Value& SubscriptRangeExpression::eval(ExpressionContext &ctx) {
const auto listValue = DCHECK_NOTNULL(list_)->eval(ctx);
const auto& listValue = DCHECK_NOTNULL(list_)->eval(ctx);
if (!listValue.isList()) {
result_ = Value::kNullBadType;
return result_;
}
const auto list = listValue.getList();
const auto& list = listValue.getList();
size_t size = list.size();
int64_t lo, hi;
if (lo_ == nullptr) {
auto hiValue = DCHECK_NOTNULL(hi_)->eval(ctx);
if (hiValue.isNull()) {
result_ = Value::kNullValue;
return result_;
}
if (!hiValue.isInt()) {
result_ = Value::kNullBadType;
return result_;
}
hi = hiValue.getInt();
lo = (hi < 0 ? -list.size() : 0);
} else if (hi_ == nullptr) {
lo = 0;
} else {
auto loValue = DCHECK_NOTNULL(lo_)->eval(ctx);
if (loValue.isNull()) {
result_ = Value::kNullValue;
Expand All @@ -150,57 +141,47 @@ const Value& SubscriptRangeExpression::eval(ExpressionContext &ctx) {
return result_;
}
lo = loValue.getInt();
hi = (lo < 0 ? -1 : list.size());
if (lo < 0) {
lo += size;
}
}

if (hi_ == nullptr) {
hi = size;
} else {
auto loValue = lo_->eval(ctx);
auto hiValue = hi_->eval(ctx);
if (loValue.isNull() || hiValue.isNull()) {
auto hiValue = DCHECK_NOTNULL(hi_)->eval(ctx);
if (hiValue.isNull()) {
result_ = Value::kNullValue;
return result_;
}
if (!(loValue.isInt() && hiValue.isInt())) {
if (!hiValue.isInt()) {
result_ = Value::kNullBadType;
return result_;
}
lo = loValue.getInt();
hi = hiValue.getInt();
if (hi < 0) {
hi += size;
}
}

// fit the out-of-range
// Out-of-bound slices are simply truncated
if (lo < 0) {
if (lo < -static_cast<int64_t>(list.size())) {
lo = -list.size();
}
} else {
if (lo >= static_cast<int64_t>(list.size())) {
result_ = List();
return result_;
}
lo = 0;
}

if (hi < 0) {
if (hi < -static_cast<int64_t>(list.size())) {
result_ = List();
return result_;
}
} else {
if (hi >= static_cast<int64_t>(list.size())) {
hi = list.size();
}
if (lo >= hi) {
result_ = List();
return result_;
}

const auto begin = lo < 0 ? list.values.end() + lo : list.values.begin() + lo;
const auto end = hi < 0 ? list.values.end() + hi : list.values.begin() + hi;
using iterCategory = typename std::iterator_traits<decltype(begin)>::iterator_category;
static_assert(std::is_base_of_v<std::random_access_iterator_tag, iterCategory>);
if (std::distance(begin, end) < 0) {
// The unclosed range
if (static_cast<size_t>(lo) >= size) {
result_ = List();
return result_;
}
if (static_cast<size_t>(hi) >= size) {
hi = size;
}

List r;
r.values.insert(r.values.end(), begin, end);
r.values = {list.values.begin()+lo, list.values.begin()+hi};
result_ = std::move(r);
return result_;
}
Expand Down
9 changes: 9 additions & 0 deletions src/common/expression/test/SubscriptExpressionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,15 @@ TEST_F(SubscriptExpressionTest, ListSubscriptRange) {
EXPECT_TRUE(value.isList());
EXPECT_EQ(List(), value.getList());
}
// [0,1,2,3,4,5][-2..] => [4,5]
{
auto *lo = ConstantExpression::make(&pool, -2);
auto *hi = ConstantExpression::make(&pool, 3);
auto expr = SubscriptRangeExpression::make(&pool, list->clone(), lo, hi);
auto value = Expression::eval(expr, gExpCtxt);
EXPECT_TRUE(value.isList());
EXPECT_EQ(List(), value.getList());
}
}

TEST_F(SubscriptExpressionTest, MapSubscript) {
Expand Down

0 comments on commit 5e9d15e

Please sign in to comment.