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

Check the index have existed although the name not the same #1695

Merged
merged 2 commits into from
Jul 17, 2020
Merged

Check the index have existed although the name not the same #1695

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6742fb2
check index field conflict
darionyaphet Jan 20, 2020
6405dd4
Merge branch 'master' into index-conflict
Jul 17, 2020
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
63 changes: 51 additions & 12 deletions src/graph/test/IndexTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ TEST_F(IndexTest, TagIndex) {
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "CREATE TAG INDEX duplicate_person_index ON person(name)";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
// Tag not exist
{
cpp2::ExecutionResponse resp;
Expand Down Expand Up @@ -83,6 +89,24 @@ TEST_F(IndexTest, TagIndex) {
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "CREATE TAG INDEX duplicate_person_index ON person(name, email)";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "CREATE TAG INDEX duplicate_index ON person(name, name)";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "CREATE TAG INDEX disorder_person_index ON person(email, name)";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
{
cpp2::ExecutionResponse resp;
auto query = "INSERT VERTEX person(name, age, gender, email) VALUES "
Expand Down Expand Up @@ -133,12 +157,6 @@ TEST_F(IndexTest, TagIndex) {
ASSERT_NE("FAILED", columns[1].get_str());
}
}
{
cpp2::ExecutionResponse resp;
std::string query = "CREATE TAG INDEX duplicate_index ON person(name, name)";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
// Describe Tag Index
{
cpp2::ExecutionResponse resp;
Expand Down Expand Up @@ -179,6 +197,7 @@ TEST_F(IndexTest, TagIndex) {
std::vector<std::tuple<std::string>> expected{
{"single_person_index"},
{"multi_person_index"},
{"disorder_person_index"},
};
ASSERT_TRUE(verifyResult(resp, expected, true, {0}));
}
Expand Down Expand Up @@ -242,6 +261,12 @@ TEST_F(IndexTest, EdgeIndex) {
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "CREATE EDGE INDEX duplicate_friend_index ON friend(degree)";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
// Edge not exist
{
cpp2::ExecutionResponse resp;
Expand Down Expand Up @@ -270,6 +295,25 @@ TEST_F(IndexTest, EdgeIndex) {
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "CREATE EDGE INDEX duplicate_friend_index "
"ON friend(degree, start_time)";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "CREATE EDGE INDEX duplicate_index ON friend(degree, degree)";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string query = "CREATE EDGE INDEX disorder_friend_index ON friend(start_time, degree)";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
{
cpp2::ExecutionResponse resp;
auto query = "INSERT EDGE friend(degree, start_time) VALUES "
Expand Down Expand Up @@ -320,12 +364,6 @@ TEST_F(IndexTest, EdgeIndex) {
ASSERT_NE("FAILED", columns[1].get_str());
}
}
{
cpp2::ExecutionResponse resp;
std::string query = "CREATE EDGE INDEX duplicate_index ON friend(degree, degree)";
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
// Describe Edge Index
{
cpp2::ExecutionResponse resp;
Expand Down Expand Up @@ -366,6 +404,7 @@ TEST_F(IndexTest, EdgeIndex) {
std::vector<std::tuple<std::string>> expected{
{"single_friend_index"},
{"multi_friend_index"},
{"disorder_friend_index"},
};
ASSERT_TRUE(verifyResult(resp, expected, true, {0}));
}
Expand Down
5 changes: 4 additions & 1 deletion src/meta/processors/BaseProcessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,14 +237,17 @@ class BaseProcessor {
void doSyncMultiRemoveAndUpdate(std::vector<std::string> keys);

/**
* check if the edge or tag contains indexes when alter edge or tag.
* Check the edge or tag contains indexes when alter it.
**/
cpp2::ErrorCode indexCheck(const std::vector<nebula::cpp2::IndexItem>& items,
const std::vector<cpp2::AlterSchemaItem>& alterItems);

StatusOr<std::vector<nebula::cpp2::IndexItem>>
getIndexes(GraphSpaceID spaceId, int32_t tagOrEdge);

bool checkIndexExist(const std::vector<std::string>& fields,
const nebula::cpp2::IndexItem& item);

protected:
kvstore::KVStore* kvstore_ = nullptr;
RESP resp_;
Expand Down
16 changes: 16 additions & 0 deletions src/meta/processors/BaseProcessor.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,5 +469,21 @@ BaseProcessor<RESP>::indexCheck(const std::vector<nebula::cpp2::IndexItem>& item
return cpp2::ErrorCode::SUCCEEDED;
}

template<typename RESP>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think better put this function to processor/indexMan/ folder, because it's not the concept of BaseProcessor.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please check item.get_fields().szie() >= fields.size().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

before we check the index have checked the fields size.

bool BaseProcessor<RESP>::checkIndexExist(const std::vector<std::string>& fields,
const nebula::cpp2::IndexItem& item) {
for (size_t i = 0; i < fields.size(); i++) {
if (fields[i] != item.get_fields()[i].get_name()) {
break;
}

if (i == fields.size() - 1) {
LOG(ERROR) << "Index " << item.get_index_name() << " have existed";
return true;
}
}
return false;
}

darionyaphet marked this conversation as resolved.
Show resolved Hide resolved
} // namespace meta
} // namespace nebula
33 changes: 30 additions & 3 deletions src/meta/processors/indexMan/CreateEdgeIndexProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,41 @@ void CreateEdgeIndexProcessor::process(const cpp2::CreateEdgeIndexReq& req) {
}

auto edgeType = edgeTypeRet.value();
auto retSchema = getLatestEdgeSchema(space, edgeType);
if (!retSchema.ok()) {
auto prefix = MetaServiceUtils::indexPrefix(space);
std::unique_ptr<kvstore::KVIterator> checkIter;
auto checkRet = kvstore_->prefix(kDefaultSpaceId, kDefaultPartId, prefix, &checkIter);
if (checkRet != kvstore::ResultCode::SUCCEEDED) {
resp_.set_code(MetaCommon::to(checkRet));
onFinished();
return;
}

while (checkIter->valid()) {
auto val = checkIter->val();
auto item = MetaServiceUtils::parseIndex(val);
if (item.get_schema_id().getType() != nebula::cpp2::SchemaID::Type::edge_type ||
fieldNames.size() > item.get_fields().size() ||
edgeType != item.get_schema_id().get_edge_type()) {
checkIter->next();
continue;
}

if (checkIndexExist(fieldNames, item)) {
resp_.set_code(cpp2::ErrorCode::E_EXISTED);
onFinished();
return;
}
checkIter->next();
}

auto schemaRet = getLatestEdgeSchema(space, edgeType);
if (!schemaRet.ok()) {
handleErrorCode(cpp2::ErrorCode::E_NOT_FOUND);
onFinished();
return;
}

auto latestEdgeSchema = retSchema.value();
auto latestEdgeSchema = schemaRet.value();
if (tagOrEdgeHasTTL(latestEdgeSchema)) {
LOG(ERROR) << "Edge: " << edgeName << " has ttl, not create index";
handleErrorCode(cpp2::ErrorCode::E_INDEX_WITH_TTL);
Expand Down
33 changes: 30 additions & 3 deletions src/meta/processors/indexMan/CreateTagIndexProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,41 @@ void CreateTagIndexProcessor::process(const cpp2::CreateTagIndexReq& req) {
}

auto tagID = tagIDRet.value();
auto retSchema = getLatestTagSchema(space, tagID);
if (!retSchema.ok()) {
auto prefix = MetaServiceUtils::indexPrefix(space);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we unify the code.

std::unique_ptr<kvstore::KVIterator> checkIter;
auto checkRet = kvstore_->prefix(kDefaultSpaceId, kDefaultPartId, prefix, &checkIter);
if (checkRet != kvstore::ResultCode::SUCCEEDED) {
resp_.set_code(MetaCommon::to(checkRet));
onFinished();
return;
}

while (checkIter->valid()) {
auto val = checkIter->val();
auto item = MetaServiceUtils::parseIndex(val);
if (item.get_schema_id().getType() != nebula::cpp2::SchemaID::Type::tag_id ||
fieldNames.size() > item.get_fields().size() ||
tagID != item.get_schema_id().get_tag_id()) {
checkIter->next();
continue;
}

if (checkIndexExist(fieldNames, item)) {
resp_.set_code(cpp2::ErrorCode::E_EXISTED);
onFinished();
return;
}
checkIter->next();
}

auto schemaRet = getLatestTagSchema(space, tagID);
if (!schemaRet.ok()) {
handleErrorCode(cpp2::ErrorCode::E_NOT_FOUND);
onFinished();
return;
}

auto latestTagSchema = retSchema.value();
auto latestTagSchema = schemaRet.value();
if (tagOrEdgeHasTTL(latestTagSchema)) {
LOG(ERROR) << "Tag: " << tagName << " has ttl, not create index";
handleErrorCode(cpp2::ErrorCode::E_INDEX_WITH_TTL);
Expand Down
Loading