Fixed unary minus integer bug and support to take function call as vid (

vesoft-inc#429)

* Fixed unary minus integer bug and support to take function call as vid

* Fixed crash when a non-existing column referenced in PIPE

* Addressed @laura-ding's comment

* minor fixes

* add test for unary minus

* Address comments

* Addressed @CPWstatic's comments

src/executor/GoExecutor.cpp

@@ -56,6 +56,7 @@ Status GoExecutor::prepare() {
  } while (false);
 
  if (!status.ok()) {
+ LOG(ERROR) << "Preparing failed: " << status;
  return status;
  }
 
@@ -105,9 +106,7 @@ Status GoExecutor::prepareFrom() {
  if (clause == nullptr) {
  LOG(FATAL) << "From clause shall never be null";
  }
- if (!clause->isRef()) {
- starts_ = clause->srcNodeList()->nodeIds();
- } else {
+ if (clause->isRef()) {
  auto *expr = clause->ref();
  if (expr->isInputExpression()) {
  auto *iexpr = static_cast<InputPropertyExpression*>(expr);
@@ -120,6 +119,24 @@ Status GoExecutor::prepareFrom() {
  // No way to happen except memory corruption
  LOG(FATAL) << "Unknown kind of expression";
  }
+ break;
+ }
+
+ auto vidList = clause->vidList();
+ for (auto *expr : vidList) {
+ status = expr->prepare();
+ if (!status.ok()) {
+ break;
+ }
+ auto value = expr->eval();
+ if (!Expression::isInt(value)) {
+ status = Status::Error("Vertex ID should be of type integer");
+ break;
+ }
+ starts_.push_back(Expression::asInt(value));
+ }
+ if (!status.ok()) {
+ break;
  }
  } while (false);
  return status;
@@ -208,22 +225,26 @@ Status GoExecutor::setupStarts() {
  if (!starts_.empty()) {
  return Status::OK();
  }
+ const auto *inputs = inputs_.get();
  // Take one column from a variable
  if (varname_ != nullptr) {
- auto *varinput = ectx()->variableHolder()->get(*varname_);
- if (varinput == nullptr) {
+ auto *varInputs = ectx()->variableHolder()->get(*varname_);
+ if (varInputs == nullptr) {
  return Status::Error("Variable `%s' not defined", varname_->c_str());
  }
- starts_ = varinput->getVIDs(*colname_);
- return Status::OK();
+ DCHECK(inputs == nullptr);
+ inputs = varInputs;
  }
  // No error happened, but we are having empty inputs
- if (inputs_ == nullptr) {
+ if (inputs == nullptr) {
  return Status::OK();
  }
- // Take one column from the input of the pipe
- DCHECK(colname_ != nullptr);
- starts_ = inputs_->getVIDs(*colname_);
+
+ auto result = inputs->getVIDs(*colname_);
+ if (!result.ok()) {
+ return std::move(result).status();
+ }
+ starts_ = std::move(result).value();
  return Status::OK();
 }
 

src/executor/InsertEdgeExecutor.cpp

@@ -67,9 +67,27 @@ StatusOr<std::vector<storage::cpp2::Edge>> InsertEdgeExecutor::prepareEdges() {
  auto index = 0;
  for (auto i = 0u; i < rows_.size(); i++) {
  auto *row = rows_[i];
- auto src = row->srcid();
- auto dst = row->dstid();
- auto rank = row->rank();
+ auto status = row->srcid()->prepare();
+ if (!status.ok()) {
+ return status;
+ }
+ auto v = row->srcid()->eval();
+ if (!Expression::isInt(v)) {
+ return Status::Error("Vertex ID should be of type integer");
+ }
+ auto src = Expression::asInt(v);
+ status = row->dstid()->prepare();
+ if (!status.ok()) {
+ return status;
+ }
+ v = row->dstid()->eval();
+ if (!Expression::isInt(v)) {
+ return Status::Error("Vertex ID should be of type integer");
+ }
+ auto dst = Expression::asInt(v);
+
+ int64_t rank = row->rank();
+
  auto expressions = row->values();
 
  // Now default value is unsupported
@@ -121,6 +139,7 @@ StatusOr<std::vector<storage::cpp2::Edge>> InsertEdgeExecutor::prepareEdges() {
  in.__isset.props = true;
  }
  }
+
  return edges;
 }
 

src/executor/InsertVertexExecutor.cpp

@@ -80,7 +80,15 @@ StatusOr<std::vector<storage::cpp2::Vertex>> InsertVertexExecutor::prepareVertic
  std::vector<storage::cpp2::Vertex> vertices(rows_.size());
  for (auto i = 0u; i < rows_.size(); i++) {
  auto *row = rows_[i];
- auto id = row->id();
+ auto status = row->id()->prepare();
+ if (!status.ok()) {
+ return status;
+ }
+ auto v = row->id()->eval();
+ if (!Expression::isInt(v)) {
+ return Status::Error("Vertex ID should be of type integer");
+ }
+ auto id = Expression::asInt(v);
  auto expressions = row->values();
 
  std::vector<VariantType> values;

src/executor/InterimResult.cpp

@@ -22,7 +22,7 @@ InterimResult::InterimResult(std::vector<VertexID> vids) {
 }
 
 
-std::vector<VertexID> InterimResult::getVIDs(const std::string &col) const {
+StatusOr<std::vector<VertexID>> InterimResult::getVIDs(const std::string &col) const {
  if (!vids_.empty()) {
  DCHECK(rsReader_ == nullptr);
  return vids_;
@@ -33,7 +33,9 @@ std::vector<VertexID> InterimResult::getVIDs(const std::string &col) const {
  while (iter) {
  VertexID vid;
  auto rc = iter->getVid(col, vid);
- CHECK(rc == ResultType::SUCCEEDED);
+ if (rc != ResultType::SUCCEEDED) {
+ return Status::Error("Column `%s' not found", col.c_str());
+ }
  result.emplace_back(vid);
  ++iter;
  }

src/executor/InterimResult.h

@@ -8,6 +8,7 @@
 #define GRAPH_INTERIMRESULT_H_
 
 #include "base/Base.h"
+#include "base/StatusOr.h"
 #include "dataman/RowSetReader.h"
 #include "dataman/RowSetWriter.h"
 #include "dataman/SchemaWriter.h"
@@ -36,7 +37,7 @@ class InterimResult final {
  return rsReader_->schema();
  }
 
- std::vector<VertexID> getVIDs(const std::string &col) const;
+ StatusOr<std::vector<VertexID>> getVIDs(const std::string &col) const;
 
  std::vector<cpp2::RowValue> getRows() const;
  // TODO(dutor) iterating interfaces on rows and columns

src/executor/test/YieldTest.cpp

@@ -40,7 +40,7 @@ TEST_F(YieldTest, Basic) {
  }
  {
  cpp2::ExecutionResponse resp;
- std::string query = "YIELD 1 + 1";
+ std::string query = "YIELD 1+1";
  auto code = client->execute(query, resp);
  ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);
  std::vector<std::tuple<int64_t>> expected{

src/parser/Clauses.cpp

@@ -21,6 +21,7 @@ std::string StepClause::toString() const {
  return buf;
 }
 
+
 std::string SourceNodeList::toString() const {
  std::string buf;
  buf.reserve(256);
@@ -34,14 +35,27 @@ std::string SourceNodeList::toString() const {
  return buf;
 }
 
+std::string VertexIDList::toString() const {
+ std::string buf;
+ buf.reserve(256);
+ for (auto &expr : vidList_) {
+ buf += expr->toString();
+ buf += ",";
+ }
+ if (!buf.empty()) {
+ buf.resize(buf.size() - 1);
+ }
+ return buf;
+}
+
 std::string FromClause::toString() const {
  std::string buf;
  buf.reserve(256);
  buf += "FROM ";
- if (!isRef_) {
- buf += srcNodeList_->toString();
- } else {
+ if (isRef()) {
  buf += ref_->toString();
+ } else {
+ buf += vidList_->toString();
  }
  return buf;
 }

src/parser/Clauses.h

@@ -51,40 +51,56 @@ class SourceNodeList final {
 };
 
 
-class FromClause final {
+
+class VertexIDList final {
 public:
- explicit FromClause(SourceNodeList *srcNodeList) {
- srcNodeList_.reset(srcNodeList);
- isRef_ = false;
+ void add(Expression *expr) {
+ vidList_.emplace_back(expr);
  }
 
- explicit FromClause(Expression *expr) {
- ref_.reset(expr);
- isRef_ = true;
+ std::vector<Expression*> vidList() const {
+ std::vector<Expression*> result;
+ result.reserve(vidList_.size());
+ for (auto &expr : vidList_) {
+ result.push_back(expr.get());
+ }
+ return result;
  }
 
- void setSourceNodeList(SourceNodeList *srcNodeList) {
- srcNodeList_.reset(srcNodeList);
+ std::string toString() const;
+
+private:
+ std::vector<std::unique_ptr<Expression>> vidList_;
+};
+
+
+class FromClause final {
+public:
+ explicit FromClause(VertexIDList *vidList) {
+ vidList_.reset(vidList);
  }
 
- SourceNodeList* srcNodeList() const {
- return srcNodeList_.get();
+ explicit FromClause(Expression *ref) {
+ ref_.reset(ref);
  }
 
- Expression* ref() const {
- return ref_.get();
+ auto vidList() const {
+ return vidList_->vidList();
  }
 
- bool isRef() const {
- return isRef_;
+ auto isRef() const {
+ return ref_ != nullptr;
+ }
+
+ auto ref() const {
+ return ref_.get();
  }
 
  std::string toString() const;
 
 private:
- std::unique_ptr<SourceNodeList> srcNodeList_;
+ std::unique_ptr<VertexIDList>  vidList_;
  std::unique_ptr<Expression> ref_;
- bool isRef_{false};
 };
 
 

src/parser/MutateSentences.cpp

@@ -69,7 +69,7 @@ std::string ValueList::toString() const {
 std::string VertexRowItem::toString() const {
  std::string buf;
  buf.reserve(256);
- buf += std::to_string(id_);
+ buf += id_->toString();
  buf += ":";
  buf += "(";
  buf += values_->toString();
@@ -107,9 +107,9 @@ std::string EdgeRowItem::toString() const {
  std::string buf;
  buf.reserve(256);
 
- buf += std::to_string(srcid_);
+ buf += srcid_->toString();
  buf += "->";
- buf += std::to_string(dstid_);
+ buf += dstid_->toString();
  if (rank_ != 0) {
  buf += "@";
  buf += std::to_string(rank_);
@@ -185,7 +185,7 @@ std::string UpdateVertexSentence::toString() const {
  buf += "OR INSERT ";
  }
  buf += "VERTEX ";
- buf += std::to_string(vid_);
+ buf += vid_->toString();
  buf += " SET ";
  buf += updateItems_->toString();
  if (whereClause_ != nullptr) {
@@ -209,9 +209,9 @@ std::string UpdateEdgeSentence::toString() const {
  buf += "OR INSERT ";
  }
  buf += "EDGE ";
- buf += std::to_string(srcid_);
+ buf += srcid_->toString();
  buf += "->";
- buf += std::to_string(dstid_);
+ buf += dstid_->toString();
  buf += " SET ";
  buf += updateItems_->toString();
  if (whereClause_ != nullptr) {
@@ -230,7 +230,7 @@ std::string DeleteVertexSentence::toString() const {
  std::string buf;
  buf.reserve(256);
  buf += "DELETE VERTEX ";
- buf += srcNodeList_->toString();
+ buf += vidList_->toString();
  if (whereClause_ != nullptr) {
  buf += " ";
  buf += whereClause_->toString();
@@ -241,10 +241,10 @@ std::string DeleteVertexSentence::toString() const {
 std::string EdgeList::toString() const {
  std::string buf;
  buf.reserve(256);
- for (auto edge : edges_) {
- buf += std::to_string(edge.first);
+ for (auto &edge : edges_) {
+ buf += edge.first->toString();
  buf += "->";
- buf += std::to_string(edge.second);
+ buf += edge.second->toString();
  buf += ",";
  }
  if (!buf.empty()) {