Skip to content

Commit

Permalink
Correct/negative default value (vesoft-inc#1739)
Browse files Browse the repository at this point in the history
* Fix the bug default value can't accept negative literal value.

* Fix the bug tha default value can't accept negative literal.

* Let schema default value could accept expression.

* Fix one typo.

* Correct the value type definition.

* Using the type alias.

Co-authored-by: dutor <440396+dutor@users.noreply.github.com>
  • Loading branch information
Shylock-Hg and dutor authored Feb 25, 2020
1 parent 20422bb commit 4d5f567
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 58 deletions.
4 changes: 4 additions & 0 deletions src/graph/CreateEdgeExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,15 @@ namespace graph {
CreateEdgeExecutor::CreateEdgeExecutor(Sentence *sentence,
ExecutionContext *ectx)
: Executor(ectx, "create_edge") {
exprCtx_ = std::make_unique<ExpressionContext>();
sentence_ = static_cast<CreateEdgeSentence*>(sentence);
}


Status CreateEdgeExecutor::prepare() {
for (auto spec : sentence_->columnSpecs()) {
spec->setContext(exprCtx_.get());
}
return Status::OK();
}

Expand Down
1 change: 1 addition & 0 deletions src/graph/CreateEdgeExecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class CreateEdgeExecutor final : public Executor {

private:
CreateEdgeSentence *sentence_{nullptr};
std::unique_ptr<ExpressionContext> exprCtx_{nullptr};
nebula::cpp2::Schema schema_;
};

Expand Down
4 changes: 4 additions & 0 deletions src/graph/CreateTagExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@ CreateTagExecutor::CreateTagExecutor(Sentence *sentence,
ExecutionContext *ectx)
: Executor(ectx, "create_tag") {
sentence_ = static_cast<CreateTagSentence*>(sentence);
exprCtx_ = std::make_unique<ExpressionContext>();
}


Status CreateTagExecutor::prepare() {
for (auto spec : sentence_->columnSpecs()) {
spec->setContext(exprCtx_.get());
}
return Status::OK();
}

Expand Down
1 change: 1 addition & 0 deletions src/graph/CreateTagExecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class CreateTagExecutor final : public Executor {

private:
CreateTagSentence *sentence_{nullptr};
std::unique_ptr<ExpressionContext> exprCtx_{nullptr};
nebula::cpp2::Schema schema_;
};

Expand Down
15 changes: 10 additions & 5 deletions src/graph/SchemaHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,16 @@ Status SchemaHelper::createSchema(const std::vector<ColumnSpecification*>& specs
column.name = *spec->name();
column.type.type = columnTypeToSupportedType(spec->type());
nebula::cpp2::Value v;
Getters getter;
if (spec->hasDefault()) {
auto s = spec->prepare();
if (!s.ok()) {
return s;
}
switch (spec->type()) {
case nebula::ColumnType::BOOL:
{
auto ret = spec->getBoolValue();
auto ret = spec->getBoolValue(getter);
if (!ret.ok()) {
auto error = "Column `%s' set wrong default value,"
" schema type is `bool'";
Expand All @@ -60,7 +65,7 @@ Status SchemaHelper::createSchema(const std::vector<ColumnSpecification*>& specs
}
case nebula::ColumnType::INT:
{
auto ret = spec->getIntValue();
auto ret = spec->getIntValue(getter);
if (!ret.ok()) {
auto error = "Column `%s' set wrong default value,"
" schema type is `int'";
Expand All @@ -71,7 +76,7 @@ Status SchemaHelper::createSchema(const std::vector<ColumnSpecification*>& specs
}
case nebula::ColumnType::DOUBLE:
{
auto ret = spec->getDoubleValue();
auto ret = spec->getDoubleValue(getter);
if (!ret.ok()) {
auto error = "Column `%s' set wrong type default value,"
" schema type is `double'";
Expand All @@ -82,7 +87,7 @@ Status SchemaHelper::createSchema(const std::vector<ColumnSpecification*>& specs
}
case nebula::ColumnType::STRING:
{
auto ret = spec->getStringValue();
auto ret = spec->getStringValue(getter);
if (!ret.ok()) {
auto error = "Column `%s' set wrong type default value,"
" schema type is `string'";
Expand All @@ -93,7 +98,7 @@ Status SchemaHelper::createSchema(const std::vector<ColumnSpecification*>& specs
}
case nebula::ColumnType::TIMESTAMP:
{
auto ret = spec->getIntValue();
auto ret = spec->getIntValue(getter);
if (!ret.ok()) {
auto error = "Column `%s' set wrong type default value,"
" schema type is `timestamp'";
Expand Down
48 changes: 43 additions & 5 deletions src/graph/test/SchemaTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,44 @@ TEST_F(SchemaTest, metaCommunication) {
}
ASSERT_TRUE(verifyResult(resp, expected));
}
// Tag with negative default value
{
cpp2::ExecutionResponse resp;
const std::string cmd =
"CREATE TAG default_tag_neg(id int DEFAULT -10, height double DEFAULT -176.0)";
auto code = client->execute(cmd, resp);
EXPECT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
// Tag with expression default value
{
cpp2::ExecutionResponse resp;
const std::string cmd = "CREATE TAG default_tag_expr"
"(id int DEFAULT 3/2*4-5, " // arithmetic
"male bool DEFAULT 3 > 2, " // relation
"height double DEFAULT abs(-176.0), " // built-in function
"adult bool DEFAULT true && false)"; // logic
auto code = client->execute(cmd, resp);
EXPECT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
// Edge with negative default value
{
cpp2::ExecutionResponse resp;
const std::string cmd =
"CREATE EDGE default_edge_neg(id int DEFAULT -10, height double DEFAULT -176.0)";
auto code = client->execute(cmd, resp);
EXPECT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
// Edge with expression default value
{
cpp2::ExecutionResponse resp;
const std::string cmd = "CREATE EDGE default_edge_expr"
"(id int DEFAULT 3/2*4-5, " // arithmetic
"male bool DEFAULT 3 > 2, " // relation
"height double DEFAULT abs(-176.0), " // built-in function
"adult bool DEFAULT true && false)"; // logic
auto code = client->execute(cmd, resp);
EXPECT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
}
// Test different tag and edge in different space
{
cpp2::ExecutionResponse resp;
Expand Down Expand Up @@ -768,8 +806,8 @@ TEST_F(SchemaTest, metaCommunication) {
auto code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
std::vector<std::tuple<int32_t, std::string>> expected{
{1015, "animal"},
{1016, "person"},
{1019, "animal"},
{1020, "person"},
};
ASSERT_TRUE(verifyResult(resp, expected));
}
Expand All @@ -784,16 +822,16 @@ TEST_F(SchemaTest, metaCommunication) {
code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
std::vector<std::tuple<int32_t, std::string>> expected1{
{1018, "test_tag"},
{1022, "test_tag"},
};
ASSERT_TRUE(verifyResult(resp, expected1));

query = "USE test_multi; CREATE TAG test_tag1(); USE my_space; SHOW TAGS;";
code = client->execute(query, resp);
ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
std::vector<std::tuple<int32_t, std::string>> expected2{
{1015, "animal"},
{1016, "person"},
{1019, "animal"},
{1020, "person"},
};
ASSERT_TRUE(verifyResult(resp, expected2));

Expand Down
82 changes: 48 additions & 34 deletions src/parser/MaintainSentences.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace nebula {

class ColumnSpecification final {
public:
using Value = boost::variant<int64_t, bool, double, std::string>;
using Value = Expression;

ColumnSpecification(ColumnType type, std::string *name) {
type_ = type;
Expand All @@ -31,65 +31,79 @@ class ColumnSpecification final {
return name_.get();
}

void setIntValue(int64_t v) {
defaultValue_ = v;
hasDefault_ = true;
void setValue(Value* expr) {
defaultExpr_.reset(DCHECK_NOTNULL(expr));
}

StatusOr<int64_t> getIntValue() {
if (defaultValue_.which() != VAR_INT64) {
return Status::Error("Wrong type");
Status MUST_USE_RESULT prepare() {
if (hasDefault()) {
return defaultExpr_->prepare();
}
int64_t v = boost::get<int64_t>(defaultValue_);
return v;
}

void setBoolValue(bool v) {
defaultValue_ = v;
hasDefault_ = true;
return Status::Error();
}

StatusOr<bool> getBoolValue() {
if (defaultValue_.which() != VAR_BOOL) {
StatusOr<int64_t> getIntValue(Getters& getter) {
auto r = defaultExpr_->eval(getter);
if (!r.ok()) {
return std::move(r).status();
}
auto v = std::move(r).value();
if (!Value::isInt(v)) {
return Status::Error("Wrong type");
}
return boost::get<bool>(defaultValue_);
return Value::toInt(v);
}

void setDoubleValue(double v) {
defaultValue_ = v;
hasDefault_ = true;
StatusOr<bool> getBoolValue(Getters& getter) {
auto r = defaultExpr_->eval(getter);
if (!r.ok()) {
return std::move(r).status();
}
auto v = std::move(r).value();
if (!Value::isBool(v)) {
return Status::Error("Wrong type");
}
return Value::toBool(v);
}

StatusOr<double> getDoubleValue() {
if (defaultValue_.which() != VAR_DOUBLE) {
StatusOr<double> getDoubleValue(Getters& getter) {
auto r = defaultExpr_->eval(getter);
if (!r.ok()) {
return std::move(r).status();
}
auto v = std::move(r).value();
if (!Value::isDouble(v)) {
return Status::Error("Wrong type");
}
return boost::get<double>(defaultValue_);
return Value::toDouble(v);
}

void setStringValue(std::string *v) {
defaultValue_ = *v;
hasDefault_ = true;
delete v;
StatusOr<std::string> getStringValue(Getters& getter) {
auto r = defaultExpr_->eval(getter);
if (!r.ok()) {
return std::move(r).status();
}
auto v = std::move(r).value();
if (!Value::isString(v)) {
return Status::Error("Wrong type");
}
return Value::toString(v);
}

StatusOr<std::string> getStringValue() {
if (defaultValue_.which() != VAR_STR) {
return Status::Error("Wrong type");
void setContext(ExpressionContext* ctx) {
if (defaultExpr_ != nullptr) {
defaultExpr_->setContext(ctx);
}
return boost::get<std::string>(defaultValue_);
}

bool hasDefault() {
return hasDefault_;
return defaultExpr_ != nullptr;
}

private:
ColumnType type_;
std::unique_ptr<std::string> name_;
bool hasDefault_{false};
VariantType defaultValue_;
std::unique_ptr<Value> defaultExpr_{nullptr};
};


Expand Down
16 changes: 2 additions & 14 deletions src/parser/parser.yy
Original file line number Diff line number Diff line change
Expand Up @@ -1131,21 +1131,9 @@ column_spec_list

column_spec
: name_label type_spec { $$ = new ColumnSpecification($2, $1); }
| name_label type_spec KW_DEFAULT INTEGER {
| name_label type_spec KW_DEFAULT expression {
$$ = new ColumnSpecification($2, $1);
$$->setIntValue($4);
}
| name_label type_spec KW_DEFAULT BOOL {
$$ = new ColumnSpecification($2, $1);
$$->setBoolValue($4);
}
| name_label type_spec KW_DEFAULT DOUBLE {
$$ = new ColumnSpecification($2, $1);
$$->setDoubleValue($4);
}
| name_label type_spec KW_DEFAULT STRING {
$$ = new ColumnSpecification($2, $1);
$$->setStringValue($4);
$$->setValue($4);
}
;

Expand Down

0 comments on commit 4d5f567

Please sign in to comment.