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.

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

forbid insert vertex when vertex_key flag is off #4727

Merged
merged 5 commits into from
Oct 17, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 2 additions & 0 deletions src/graph/service/GraphFlags.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,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 @@ -65,4 +65,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) |
4 changes: 0 additions & 4 deletions tests/tck/features/ttl/TTL.feature
Original file line number Diff line number Diff line change
Expand Up @@ -394,21 +394,18 @@ Feature: TTLTest
"""
Then the result should be, in any order, with relax comparison:
| node |
| ("1") |
Copy link
Contributor

Choose a reason for hiding this comment

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

Why got empty set when vertex inserted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has ttl column, and ttl expired

When executing query:
"""
FETCH PROP ON person "1" YIELD person.id as id
"""
Then the result should be, in any order:
| id |
| EMPTY |
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 @@ -492,5 +489,4 @@ Feature: TTLTest
"""
Then the result should be, in any order:
| age |
| EMPTY |
And drop the used space