Skip to content

Commit

Permalink
forbid insert vertex when vertex_key flag is off (#4727)
Browse files Browse the repository at this point in the history
* forbid insert vertex when vertex_key flag is off

* fix issue ent#1420

* fix test case

* fix format

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>
  • Loading branch information
critical27 and Sophie-Xie authored Oct 17, 2022
1 parent ee38052 commit 6f551dc
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 12 deletions.
2 changes: 2 additions & 0 deletions src/graph/service/GraphFlags.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,5 @@ DEFINE_uint32(
gc_worker_size,
0,
"Background garbage clean workers, default number is 0 which means using hardware core size.");

DEFINE_bool(graph_use_vertex_key, false, "whether allow insert or query the vertex key");
3 changes: 3 additions & 0 deletions src/graph/service/GraphFlags.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,7 @@ DECLARE_int32(max_job_size);

DECLARE_bool(enable_async_gc);
DECLARE_uint32(gc_worker_size);

DECLARE_bool(graph_use_vertex_key);

#endif // GRAPH_GRAPHFLAGS_H_
3 changes: 3 additions & 0 deletions src/graph/validator/MutateValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ Status InsertVerticesValidator::check() {
}

auto tagItems = sentence->tagItems();
if (!FLAGS_graph_use_vertex_key && tagItems.empty()) {
return Status::SemanticError("Insert vertex is forbidden, please speicify the tag");
}

schemas_.reserve(tagItems.size());

Expand Down
38 changes: 35 additions & 3 deletions src/storage/exec/GetPropNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@ class GetTagPropNode : public QueryNode<VertexID> {
std::vector<TagNode*> tagNodes,
nebula::DataSet* resultDataSet,
Expression* filter,
std::size_t limit)
std::size_t limit,
TagContext* tagContext)
: context_(context),
tagNodes_(std::move(tagNodes)),
resultDataSet_(resultDataSet),
expCtx_(filter == nullptr
? nullptr
: new StorageExpressionContext(context->vIdLen(), context->isIntId())),
filter_(filter),
limit_(limit) {
limit_(limit),
tagContext_(tagContext) {
name_ = "GetTagPropNode";
}

Expand Down Expand Up @@ -69,7 +71,36 @@ class GetTagPropNode : public QueryNode<VertexID> {
} else if (!iter->valid()) {
return nebula::cpp2::ErrorCode::SUCCEEDED;
}
// if has any tag, will emplace a row with vId

bool hasValidTag = false;
for (; iter->valid(); iter->next()) {
// check if tag schema exists
auto key = iter->key();
auto tagId = NebulaKeyUtils::getTagId(context_->vIdLen(), key);
auto schemaIter = tagContext_->schemas_.find(tagId);
if (schemaIter == tagContext_->schemas_.end()) {
continue;
}
// check if ttl expired
auto schemas = &(schemaIter->second);
RowReaderWrapper reader;
reader.reset(*schemas, iter->val());
if (!reader) {
continue;
}
auto ttl = QueryUtils::getTagTTLInfo(tagContext_, tagId);
if (ttl.has_value() &&
CommonUtils::checkDataExpiredForTTL(
schemas->back().get(), reader.get(), ttl.value().first, ttl.value().second)) {
continue;
}
hasValidTag = true;
break;
}
if (!hasValidTag) {
return nebula::cpp2::ErrorCode::SUCCEEDED;
}
// if has any valid tag, will emplace a row with vId
}
}

Expand Down Expand Up @@ -131,6 +162,7 @@ class GetTagPropNode : public QueryNode<VertexID> {
std::unique_ptr<StorageExpressionContext> expCtx_{nullptr};
Expression* filter_{nullptr};
const std::size_t limit_{std::numeric_limits<std::size_t>::max()};
TagContext* tagContext_;
};

class GetEdgePropNode : public QueryNode<cpp2::EdgeKey> {
Expand Down
2 changes: 1 addition & 1 deletion src/storage/query/GetPropProcessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ StoragePlan<VertexID> GetPropProcessor::buildTagPlan(RuntimeContext* context,
plan.addNode(std::move(tag));
}
auto output = std::make_unique<GetTagPropNode>(
context, tags, result, filter_ == nullptr ? nullptr : filter_->clone(), limit_);
context, tags, result, filter_ == nullptr ? nullptr : filter_->clone(), limit_, &tagContext_);
for (auto* tag : tags) {
output->addDependency(tag);
}
Expand Down
1 change: 0 additions & 1 deletion tests/tck/features/delete/DeleteTag.IntVid.feature
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ Feature: Delete int vid of tag
| id |
Then drop the used space

@wtf
Scenario: delete int vid multiple vertex one tag
Given an empty graph
And load "nba_int_vid" csv data to a new space
Expand Down
4 changes: 4 additions & 0 deletions tests/tck/features/insert/insertVertexOnly.feature
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ Feature: insert vertex without tag
When executing query and retrying it on failure every 6 seconds for 3 times:
"""
INSERT VERTEX VALUES 1:(),2:(),3:();
"""
Then a SemanticError should be raised at runtime: Insert vertex is forbidden, please speicify the tag
When executing query:
"""
INSERT EDGE e() VALUES 1->2:(),2->3:();
"""
Then the execution should be successful
Expand Down
43 changes: 43 additions & 0 deletions tests/tck/features/match/Base.feature
Original file line number Diff line number Diff line change
Expand Up @@ -718,3 +718,46 @@ Feature: Basic match
Then the result should be, in any order:
| v |
| ("Boris Diaw" :player{age: 36, name: "Boris Diaw"}) |

Scenario: Match with id when all tag is dropped, ent-#1420
Given an empty graph
And create a space with following options:
| partition_num | 1 |
| replica_factor | 1 |
| vid_type | FIXED_STRING(20) |
And having executed:
"""
CREATE TAG IF NOT EXISTS player(name string, age int);
CREATE TAG IF NOT EXISTS team(name string);
CREATE TAG INDEX IF NOT EXISTS player_index_1 on player(name(10), age);
"""
And wait 5 seconds
When try to execute query:
"""
INSERT VERTEX player() VALUES "v2":();
INSERT VERTEX player(name, age) VALUES "v3":("v3", 18);
UPSERT VERTEX ON player "v4" SET name = "v4", age = 18;
"""
Then the execution should be successful
When executing query:
"""
MATCH (v) WHERE id(v) in ["v1", "v2", "v3", "v4"] return id(v) limit 10;
"""
Then the result should be, in any order, with relax comparison:
| id(v) |
| "v2" |
| "v3" |
| "v4" |
When try to execute query:
"""
DROP TAG INDEX player_index_1;
DROP TAG player;
"""
Then the execution should be successful
And wait 5 seconds
When executing query:
"""
MATCH (v) WHERE id(v) in ["v1", "v2", "v3", "v4"] return id(v) limit 10;
"""
Then the result should be, in any order, with relax comparison:
| id(v) |
10 changes: 3 additions & 7 deletions tests/tck/features/ttl/TTL.feature
Original file line number Diff line number Diff line change
Expand Up @@ -393,22 +393,19 @@ Feature: TTLTest
FETCH PROP ON person "1" YIELD vertex as node;
"""
Then the result should be, in any order, with relax comparison:
| node |
| ("1") |
| node |
When executing query:
"""
FETCH PROP ON person "1" YIELD person.id as id
"""
Then the result should be, in any order:
| id |
| EMPTY |
| id |
When executing query:
"""
FETCH PROP ON * "1" YIELD person.id, career.id
"""
Then the result should be, in any order:
| person.id | career.id |
| EMPTY | EMPTY |
When executing query:
"""
FETCH PROP ON person "2" YIELD person.id
Expand Down Expand Up @@ -491,6 +488,5 @@ Feature: TTLTest
FETCH PROP ON person "1" YIELD person.age as age;
"""
Then the result should be, in any order:
| age |
| EMPTY |
| age |
And drop the used space

0 comments on commit 6f551dc

Please sign in to comment.