Fix if-not-exists behavior of create index statements #4705
Conversation
czpmango
added
ready-for-testing
| resp_.code_ref() = nebula::cpp2::ErrorCode::E_EXISTED; | ||
| if (ifNotExists) { | ||
| resp_.code_ref() = nebula::cpp2::ErrorCode::SUCCEEDED; | ||
| cpp2::ID thriftID; |
There was a problem hiding this comment.
maybe use tools like resp_.id_ref() = to(edgeIndex, EntryType::INDEX); ?
There was a problem hiding this comment.
Just extract the id from the index item.
| cpp2::ID thriftID; | ||
| // Fill index id to avoid broken promise | ||
| thriftID.index_id_ref() = item.get_index_id(); | ||
| resp_.id_ref() = thriftID; |
There was a problem hiding this comment.
It feels a little weird this way
- The first
ifNotExistsin line 45 is to judge whether the index name exists or not.
2 This place is to judge whether there are other indexes that contain this fields
for example
create edge e(a int, b int);
create edge index e_index on e(a, b)
create edge index if not exists e_index_2 on e(a);
When execute create edge index if not exists e_index_2 on e(a); ，this returns a creation index success and returns the indexId of the first index.
There is actually only one index e_index
There was a problem hiding this comment.
It shows that the index creation is successful, but when show, the index name does not exist
There was a problem hiding this comment.
Indeed, this is the point just as you said.
IMO, index name is a redundant design.
A better syntax might be this:
create edge index on e(a);
create edge index if not exists on e(a);
rebuild edge index on e(a);
In this way, the implementation layer does not need to verify the index name(line 45).
Considering compatibility, this pr is not going to do major refactoring.
There was a problem hiding this comment.
We should try to avoid similar problems in GQL version.
There was a problem hiding this comment.
5.0 index design needs to consider this problem, thx @MuYiYong
Codecov ReportBase: 84.69% // Head: 84.71% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #4705 +/- ##
==========================================
+ Coverage 84.69% 84.71% +0.02%
==========================================
Files 1358 1358
Lines 135996 136035 +39
==========================================
+ Hits 115185 115247 +62
+ Misses 20811 20788 -23
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com>
* Use vertex key only when use_vertex_key is on (#4716) * Revert "insert vertex key when only vertex and flag is set (#4685)" This reverts commit fff82a6. * Revert "only write vertex key when flag is set or explictly insert vertex (#4680)" This reverts commit a5bed33. * Revert "fix bug (#4675)" This reverts commit cccc014. * Revert "Rollback vertex key (#4629)" This reverts commit 545137a. * wtf * tck format * fix job status when all finished (#4717) * fix job name from zone balance to data balance * address wenhui's comment * fix create index if not exists behavior (#4705) Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com> Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com> Co-authored-by: Doodle <13706157+critical27@users.noreply.github.com> Co-authored-by: Alex Xing <90179377+SuperYoko@users.noreply.github.com> Co-authored-by: kyle.cao <kyle.cao@vesoft.com>
|
checked on 3.3-rc |
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
#4665
Checklist:
Tests:
Affects:
Release notes:
Please confirm whether to be reflected in release notes and how to describe: