Skip to content

Commit

Permalink
Fix default value bug (#1452)
Browse files Browse the repository at this point in the history 
* Fixed default value related bug

* Check unknown column

* Fix ut failed

Co-authored-by: dutor <440396+dutor@users.noreply.github.com>
  • Loading branch information
@laura-ding @dutor
laura-ding and dutor committed Dec 26, 2019
1 parent ec25597 commit 739d57c
Show file tree
Hide file tree
Showing 9 changed files with 192 additions and 23 deletions.
8 changes: 8 additions & 0 deletions src/graph/Executor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,14 @@ StatusOr<VariantType> Executor::transformDefaultValue(nebula::cpp2::SupportedTyp
case nebula::cpp2::SupportedType::STRING:
return originalValue;
break;
case nebula::cpp2::SupportedType::TIMESTAMP:
try {
return folly::to<int64_t>(originalValue);
} catch (const std::exception& ex) {
LOG(ERROR) << "Conversion to int64_t failed: " << originalValue;
return Status::Error("Type Conversion Failed");
}
break;
default:
LOG(ERROR) << "Unknow type";
return Status::Error("Unknow type");
Expand Down
19 changes: 19 additions & 0 deletions src/graph/InsertEdgeExecutor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ Status InsertEdgeExecutor::check() {
return Status::Error("Wrong number of props");
}

// Check prop name is in schema
for (auto *it : props_) {
if (schema_->getFieldIndex(*it) < 0) {
LOG(ERROR) << "Unknown column `" << *it << "' in schema";
return Status::Error("Unknown column `%s' in schema", it->c_str());
}
}

auto *mc = ectx()->getMetaClient();

for (size_t i = 0; i < schema_->getNumFields(); i++) {
Expand Down Expand Up @@ -149,6 +157,8 @@ StatusOr<std::vector<storage::cpp2::Edge>> InsertEdgeExecutor::prepareEdges() {
}

RowWriter writer(schema_);
int64_t valuesSize = values.size();
int32_t handleValueNum = 0;
for (size_t schemaIndex = 0; schemaIndex < schema_->getNumFields(); schemaIndex++) {
auto fieldName = schema_->getFieldName(schemaIndex);
auto positionIter = propsPosition_.find(fieldName);
Expand All @@ -157,6 +167,10 @@ StatusOr<std::vector<storage::cpp2::Edge>> InsertEdgeExecutor::prepareEdges() {
auto schemaType = schema_->getFieldType(schemaIndex);
if (positionIter != propsPosition_.end()) {
auto position = propsPosition_[fieldName];
if (position >= valuesSize) {
LOG(ERROR) << fieldName << " need input value";
return Status::Error("`%s' need input value", fieldName);
}
value = values[position];
if (!checkValueType(schemaType, value)) {
LOG(ERROR) << "ValueType is wrong, schema type "
Expand Down Expand Up @@ -187,6 +201,11 @@ StatusOr<std::vector<storage::cpp2::Edge>> InsertEdgeExecutor::prepareEdges() {
if (!status.ok()) {
return status;
}
handleValueNum++;
}
// Input values more than schema props
if (handleValueNum < valuesSize) {
return Status::Error("Column count doesn't match value count");
}

{
Expand Down
21 changes: 20 additions & 1 deletion src/graph/InsertVertexExecutor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ Status InsertVertexExecutor::check() {
return Status::Error("Wrong number of props");
}

// Check prop name is in schema
for (auto *it : props) {
if (schema->getFieldIndex(*it) < 0) {
LOG(ERROR) << "Unknown column `" << *it << "' in schema";
return Status::Error("Unknown column `%s' in schema", it->c_str());
}
}

auto *mc = ectx()->getMetaClient();

std::unordered_map<std::string, int32_t> propsPosition;
Expand All @@ -81,7 +89,7 @@ Status InsertVertexExecutor::check() {
auto valueResult = mc->getTagDefaultValue(spaceId_, tagId, name).get();
if (!valueResult.ok()) {
LOG(ERROR) << "Not exist default value: " << name;
return Status::Error("Not exist default value");
return Status::Error("`%s' not exist default value", name.c_str());
} else {
VLOG(3) << "Default Value: " << name << ":" << valueResult.value();
defaultValues_.emplace(name, valueResult.value());
Expand Down Expand Up @@ -143,7 +151,9 @@ StatusOr<std::vector<storage::cpp2::Vertex>> InsertVertexExecutor::prepareVertic

std::vector<storage::cpp2::Tag> tags(tagIds_.size());

int32_t valuesSize = values.size();
int32_t valuePosition = 0;
int32_t handleValueNum = 0;
for (auto index = 0u; index < tagIds_.size(); index++) {
auto &tag = tags[index];
auto tagId = tagIds_[index];
Expand All @@ -161,6 +171,10 @@ StatusOr<std::vector<storage::cpp2::Vertex>> InsertVertexExecutor::prepareVertic
auto schemaType = schema->getFieldType(schemaIndex);
if (positionIter != propsPosition.end()) {
auto position = propsPosition[fieldName];
if (position + valuePosition >= valuesSize) {
LOG(ERROR) << fieldName << " need input value";
return Status::Error("`%s' need input value", fieldName);
}
value = values[position + valuePosition];

if (!checkValueType(schemaType, value)) {
Expand Down Expand Up @@ -192,12 +206,17 @@ StatusOr<std::vector<storage::cpp2::Vertex>> InsertVertexExecutor::prepareVertic
if (!status.ok()) {
return status;
}
handleValueNum++;
}

tag.set_tag_id(tagId);
tag.set_props(writer.encode());
valuePosition += propsPosition.size();
}
// Input values more than schema props
if (handleValueNum < valuesSize) {
return Status::Error("Column count doesn't match value count");
}

auto& vertex = vertices[i];
vertex.set_id(id);
Expand Down
52 changes: 48 additions & 4 deletions src/graph/SchemaHelper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,65 @@ Status SchemaHelper::createSchema(const std::vector<ColumnSpecification*>& specs
if (spec->hasDefault()) {
switch (spec->type()) {
case nebula::ColumnType::BOOL:
v.set_bool_value(spec->getBoolValue());
{
auto ret = spec->getBoolValue();
if (!ret.ok()) {
auto error = "Column `%s' set wrong default value,"
" schema type is `bool'";
return Status::Error(folly::stringPrintf(error, column.name.c_str()));
}
v.set_bool_value(ret.value());
column.set_default_value(std::move(v));
break;
}
case nebula::ColumnType::INT:
v.set_int_value(spec->getIntValue());
{
auto ret = spec->getIntValue();
if (!ret.ok()) {
auto error = "Column `%s' set wrong default value,"
" schema type is `int'";
return Status::Error(folly::stringPrintf(error, column.name.c_str()));
}
v.set_int_value(ret.value());
column.set_default_value(std::move(v));
break;
}
case nebula::ColumnType::DOUBLE:
v.set_double_value(spec->getDoubleValue());
{
auto ret = spec->getDoubleValue();
if (!ret.ok()) {
auto error = "Column `%s' set wrong type default value,"
" schema type is `double'";
return Status::Error(folly::stringPrintf(error, column.name.c_str()));
}
v.set_double_value(ret.value());
column.set_default_value(std::move(v));
break;
}
case nebula::ColumnType::STRING:
v.set_string_value(spec->getStringValue());
{
auto ret = spec->getStringValue();
if (!ret.ok()) {
auto error = "Column `%s' set wrong type default value,"
" schema type is `string'";
return Status::Error(folly::stringPrintf(error, column.name.c_str()));
}
v.set_string_value(std::move(ret).value());
column.set_default_value(std::move(v));
break;
}
case nebula::ColumnType::TIMESTAMP:
{
auto ret = spec->getIntValue();
if (!ret.ok()) {
auto error = "Column `%s' set wrong type default value,"
" schema type is `timestamp'";
return Status::Error(folly::stringPrintf(error, column.name.c_str()));
}
v.set_timestamp(ret.value());
column.set_default_value(std::move(v));
break;
}
default:
LOG(ERROR) << "Unsupport Type";
return Status::Error("Unsupport Type");
Expand Down
47 changes: 37 additions & 10 deletions src/graph/test/DataTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,12 @@ AssertionResult DataTest::prepareSchema() {
// create tag with default value
{
cpp2::ExecutionResponse resp;
std::string cmd = "CREATE TAG personWithDefault(name string default \"\", "
"age int default 18, "
"isMarried bool default false, "
"BMI double default 18.5, "
"department string default \"engineering\")";
std::string cmd = "CREATE TAG personWithDefault(name string DEFAULT \"\", "
"age int DEFAULT 18, "
"isMarried bool DEFAULT false, "
"BMI double DEFAULT 18.5, "
"department string DEFAULT \"engineering\","
"birthday timestamp DEFAULT 0)";
auto code = client_->execute(cmd, resp);
if (cpp2::ErrorCode::SUCCEEDED != code) {
return TestError() << "Do cmd:" << cmd
Expand All @@ -103,7 +104,7 @@ AssertionResult DataTest::prepareSchema() {
{
cpp2::ExecutionResponse resp;
std::string cmd = "CREATE TAG studentWithDefault"
"(grade string default \"one\", number int)";
"(grade string DEFAULT \"one\", number int)";
auto code = client_->execute(cmd, resp);
if (cpp2::ErrorCode::SUCCEEDED != code) {
return TestError() << "Do cmd:" << cmd
Expand All @@ -122,14 +123,14 @@ AssertionResult DataTest::prepareSchema() {
// create edge with default value
{
cpp2::ExecutionResponse resp;
std::string cmd = "CREATE EDGE schoolmateWithDefault(likeness int default 80)";
std::string cmd = "CREATE EDGE schoolmateWithDefault(likeness int DEFAULT 80)";
auto code = client_->execute(cmd, resp);
if (cpp2::ErrorCode::SUCCEEDED != code) {
return TestError() << "Do cmd:" << cmd
<< " failed, error code "<< static_cast<int32_t>(code);
}
}
// Test same propName diff tyep in diff tags
// Test same propName diff type in diff tags
{
cpp2::ExecutionResponse resp;
std::string cmd = "CREATE TAG employee(name int)";
Expand Down Expand Up @@ -713,11 +714,21 @@ TEST_F(DataTest, InsertWithDefaultValueTest) {
auto code = client_->execute(cmd, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
// Insert lack of the column value
{
cpp2::ExecutionResponse resp;
std::string cmd = "INSERT VERTEX personWithDefault"
"(name, age, isMarried, BMI, department, redundant)"
"VALUES hash(\"Tom\"):(\"Tom\", 18, false, 18.5, \"dev\", 0)";
"(age, isMarried, BMI)"
"VALUES hash(\"Tom\"):(18, false)";
auto code = client_->execute(cmd, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
// Insert column doesn't match value count
{
cpp2::ExecutionResponse resp;
std::string cmd = "INSERT VERTEX studentWithDefault"
"(grade, number)"
"VALUES hash(\"Tom\"):(\"one\", 111, \"\")";
auto code = client_->execute(cmd, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
Expand Down Expand Up @@ -761,6 +772,22 @@ TEST_F(DataTest, InsertWithDefaultValueTest) {
auto code = client_->execute(cmd, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
// Insert lack of the column value
{
cpp2::ExecutionResponse resp;
std::string cmd = "INSERT EDGE schoolmateWithDefault(likeness) VALUES "
"hash(\"Tom\")->hash(\"Lucy\"):()";
auto code = client_->execute(cmd, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
// Column count doesn't match value count
{
cpp2::ExecutionResponse resp;
std::string cmd = "INSERT EDGE schoolmateWithDefault(likeness) VALUES "
"hash(\"Tom\")->hash(\"Lucy\"):(60, \"\")";
auto code = client_->execute(cmd, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string cmd = "INSERT EDGE schoolmateWithDefault() VALUES "
Expand Down
6 changes: 3 additions & 3 deletions src/graph/test/GroupByLimitTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ TEST_F(GroupByLimitTest, GroupByOrderByLimitTest) {
"YIELD $-.name AS name, "
"SUM(1.5) AS sum, "
"COUNT(*) AS count "
"| ORDER BY $-.sum";
"| ORDER BY $-.sum, $-.name";
auto query = folly::stringPrintf(fmt, player1.vid(), player2.vid());
auto code = client_->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
Expand All @@ -482,8 +482,8 @@ TEST_F(GroupByLimitTest, GroupByOrderByLimitTest) {
ASSERT_TRUE(verifyColNames(resp, expectedColNames));

std::vector<std::tuple<std::string, double, uint64_t>> expected = {
{"Dwyane Wade", 1.5, 1},
{"Carmelo Anthony", 1.5, 1},
{"Dwyane Wade", 1.5, 1},
{"Chris Paul", 3.0, 2},
{"LeBron James", 3.0, 2},
};
Expand All @@ -500,7 +500,7 @@ TEST_F(GroupByLimitTest, GroupByOrderByLimitTest) {
"YIELD $-.name AS name, "
"SUM(1.5) AS sum, "
"COUNT(*) AS count "
"| ORDER BY $-.sum | LIMIT 2";
"| ORDER BY $-.sum, $-.name DESC | LIMIT 2";
auto query = folly::stringPrintf(fmt, player1.vid(), player2.vid());
auto code = client_->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
Expand Down
30 changes: 30 additions & 0 deletions src/graph/test/SchemaTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,36 @@ TEST_F(SchemaTest, TestComment) {
}
}

TEST_F(SchemaTest, TestDefaultValue) {
auto client = gEnv->getClient();
ASSERT_NE(nullptr, client);
// Test command is comment
{
cpp2::ExecutionResponse resp;
std::string cmd = "CREATE TAG default_tag(name string DEFAULT 10)";
auto code = client->execute(cmd, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string cmd = "CREATE TAG default_tag(name string, age int DEFAULT \"10\")";
auto code = client->execute(cmd, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string cmd = "CREATE TAG default_tag(name string DEFAULT \"\", "
"age int DEFAULT \"10\")";
auto code = client->execute(cmd, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
{
cpp2::ExecutionResponse resp;
std::string cmd = "CREATE TAG default_tag(name string DEFAULT 10, age int DEFAULT 10)";
auto code = client->execute(cmd, resp);
ASSERT_EQ(cpp2::ErrorCode::E_EXECUTION_ERROR, code);
}
}
TEST_F(SchemaTest, metaCommunication) {
auto client = gEnv->getClient();
ASSERT_NE(nullptr, client);
Expand Down
10 changes: 10 additions & 0 deletions src/meta/processors/schemaMan/CreateTagProcessor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,16 @@ void CreateTagProcessor::process(const cpp2::CreateTagReq& req) {
}
defaultValue = folly::to<std::string>(value->get_string_value());
break;
case nebula::cpp2::SupportedType::TIMESTAMP:
if (value->getType() != nebula::cpp2::Value::Type::timestamp) {
LOG(ERROR) << "Create Tag Failed: " << name
<< " type mismatch";
resp_.set_code(cpp2::ErrorCode::E_CONFLICT);
onFinished();
return;
}
defaultValue = folly::to<std::string>(value->get_timestamp());
break;
default:
LOG(ERROR) << "Unsupported type";
return;
Expand Down
Loading

0 comments on commit 739d57c

Please sign in to comment.