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

refactor index key utils #12

Merged
merged 2 commits into from
Apr 17, 2020
Merged

refactor index key utils #12

merged 2 commits into from
Apr 17, 2020

Conversation

bright-starry-sky
Copy link
Contributor

refactor index key utils. using Value instead of VariantType.

}
return std::move(v);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return v directly.

@bright-starry-sky
Copy link
Contributor Author

Addressed dangleptr's comments.
Added some test cases.

Copy link
Contributor

@dangleptr dangleptr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done. LGTM

CHECK_GE(rawKey.size(), kVertexIndexLen);
auto offset = rawKey.size() - sizeof(VertexIntID);
return *reinterpret_cast<const VertexIntID*>(rawKey.data() + offset);
static VertexIDSlice getIndexVertexID(size_t vIdLen, const folly::StringPiece& rawKey) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If so seems we don't need VertexIntID ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If so seems we don't need VertexIntID ?

Yes, we don't need VertexIntID , the vIdLen should be 8 if this is nebula1.0 data.

.append(reinterpret_cast<const char*>(&indexId), sizeof(IndexID));
indexRaw(values, key);
key.append(srcId.data(), srcId.size())
.append(vIdLen - srcId.size(), '\0')
Copy link
Contributor

@darionyaphet darionyaphet Apr 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zero filling will arise at here ? I mean for example: when we define the vertex size is 6 and current vertex ID is ABCD the graph service will send ABCD\0\0 or ABCD ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zero filling will arise at here ? I mean for example: when we define the vertex size is 6 and current vertex ID is ABCD the graph service will send ABCD\0\0 or ABCD ?

The graph service should send ABCD, then storage service will convert it to fixed length vid ABCD\0\0

darionyaphet
darionyaphet previously approved these changes Apr 15, 2020
Copy link
Contributor

@darionyaphet darionyaphet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM

@dangleptr
Copy link
Contributor

Please rebase on the master

@bright-starry-sky
Copy link
Contributor Author

Please rebase on the master

rebased done.

case Value::Type::INT :
return encodeInt64(v.getInt());
case Value::Type::FLOAT :
return encodeDouble(v.getFloat());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

encodeDouble is too good,👍

@dangleptr dangleptr merged commit 02e4a0e into vesoft-inc:master Apr 17, 2020
@bright-starry-sky bright-starry-sky mentioned this pull request May 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants