Query::Plan - RemoveLabels added and tested

Summary:
Query::Plan::RemoveProperty op added and tested

Query::AST:: Remove added to cypher main visitor. Tested. Grammar corrected on missing whitespace in removeItem production

Reviewers: buda, mislav.bradac, teon.banek

Reviewed By: mislav.bradac

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D202

src/query/frontend/ast/ast.hpp

@@ -631,6 +631,42 @@ class SetLabels : public Clause {
       : Clause(uid), identifier_(identifier), labels_(labels) {}
 };
 
+class RemoveProperty : public Clause {
+  friend class AstTreeStorage;
+
+ public:
+  void Accept(TreeVisitorBase &visitor) override {
+    visitor.Visit(*this);
+    property_lookup_->Accept(visitor);
+    visitor.PostVisit(*this);
+  }
+  PropertyLookup *property_lookup_ = nullptr;
+
+ protected:
+  RemoveProperty(int uid) : Clause(uid) {}
+  RemoveProperty(int uid, PropertyLookup *property_lookup)
+      : Clause(uid), property_lookup_(property_lookup) {}
+};
+
+class RemoveLabels : public Clause {
+  friend class AstTreeStorage;
+
+ public:
+  void Accept(TreeVisitorBase &visitor) override {
+    visitor.Visit(*this);
+    identifier_->Accept(visitor);
+    visitor.PostVisit(*this);
+  }
+  Identifier *identifier_ = nullptr;
+  std::vector<GraphDbTypes::Label> labels_;
+
+ protected:
+  RemoveLabels(int uid) : Clause(uid) {}
+  RemoveLabels(int uid, Identifier *identifier,
+               const std::vector<GraphDbTypes::Label> &labels)
+      : Clause(uid), identifier_(identifier), labels_(labels) {}
+};
+
 // It would be better to call this AstTree, but we already have a class Tree,
 // which could be renamed to Node or AstTreeNode, but we also have a class
 // called NodeAtom...

src/query/frontend/ast/ast_visitor.hpp

@@ -38,6 +38,8 @@ class Where;
 class SetProperty;
 class SetProperties;
 class SetLabels;
+class RemoveProperty;
+class RemoveLabels;
 
 using TreeVisitorBase = ::utils::Visitor<
     Query, NamedExpression, OrOperator, XorOperator, AndOperator, NotOperator,
@@ -46,5 +48,5 @@ using TreeVisitorBase = ::utils::Visitor<
     LessOperator, GreaterOperator, LessEqualOperator, GreaterEqualOperator,
     UnaryPlusOperator, UnaryMinusOperator, Identifier, Literal, PropertyLookup,
     Create, Match, Return, Pattern, NodeAtom, EdgeAtom, Delete, Where,
-    SetProperty, SetProperties, SetLabels>;
+    SetProperty, SetProperties, SetLabels, RemoveProperty, RemoveLabels>;
 }

src/query/frontend/ast/cypher_main_visitor.cpp

@@ -65,6 +65,10 @@ antlrcpp::Any CypherMainVisitor::visitClause(CypherParser::ClauseContext *ctx) {
     // Different return type!!!
     return ctx->set()->accept(this).as<std::vector<Clause *>>();
   }
+  if (ctx->remove()) {
+    // Different return type!!!
+    return ctx->remove()->accept(this).as<std::vector<Clause *>>();
+  }
   // TODO: implement other clauses.
   throw NotYetImplemented();
   return 0;
@@ -744,6 +748,32 @@ antlrcpp::Any CypherMainVisitor::visitSetItem(
   return static_cast<Clause *>(set_labels);
 }
 
+antlrcpp::Any CypherMainVisitor::visitRemove(CypherParser::RemoveContext *ctx) {
+  std::vector<Clause *> remove_items;
+  for (auto *remove_item : ctx->removeItem()) {
+    remove_items.push_back(remove_item->accept(this));
+  }
+  return remove_items;
+}
+
+antlrcpp::Any CypherMainVisitor::visitRemoveItem(
+    CypherParser::RemoveItemContext *ctx) {
+  // RemoveProperty
+  if (ctx->propertyExpression()) {
+    auto *remove_property = storage_.Create<RemoveProperty>();
+    remove_property->property_lookup_ = ctx->propertyExpression()->accept(this);
+    return static_cast<Clause *>(remove_property);
+  }
+
+  // RemoveLabels
+  auto *remove_labels = storage_.Create<RemoveLabels>();
+  remove_labels->identifier_ = storage_.Create<Identifier>(
+      ctx->variable()->accept(this).as<std::string>());
+  remove_labels->labels_ =
+      ctx->nodeLabels()->accept(this).as<std::vector<GraphDbTypes::Label>>();
+  return static_cast<Clause *>(remove_labels);
+}
+
 antlrcpp::Any CypherMainVisitor::visitPropertyExpression(
     CypherParser::PropertyExpressionContext *ctx) {
   Expression *expression = ctx->atom()->accept(this);

src/query/frontend/ast/cypher_main_visitor.hpp

@@ -421,6 +421,16 @@ class CypherMainVisitor : public antlropencypher::CypherBaseVisitor {
    */
   antlrcpp::Any visitSetItem(CypherParser::SetItemContext *ctx) override;
 
+  /**
+   * return vector<Clause*>
+   */
+  antlrcpp::Any visitRemove(CypherParser::RemoveContext *ctx) override;
+
+  /**
+   * @return Clause*
+   */
+  antlrcpp::Any visitRemoveItem(CypherParser::RemoveItemContext *ctx) override;
+
   /**
    * @return PropertyLookup*
    */

src/query/frontend/logical/operator.hpp

@@ -34,11 +34,13 @@ class Delete;
 class SetProperty;
 class SetProperties;
 class SetLabels;
+class RemoveProperty;
+class RemoveLabels;
 
 using LogicalOperatorVisitor =
     ::utils::Visitor<CreateNode, CreateExpand, ScanAll, Expand, NodeFilter,
                      EdgeFilter, Filter, Produce, Delete, SetProperty,
-                     SetProperties, SetLabels>;
+                     SetProperties, SetLabels, RemoveProperty, RemoveLabels>;
 
 class LogicalOperator : public ::utils::Visitable<LogicalOperatorVisitor> {
  public:
@@ -1041,5 +1043,114 @@ class SetLabels : public LogicalOperator {
   std::vector<GraphDbTypes::Label> labels_;
 };
 
+/**
+ * Logical op for removing a property from an
+ * edge or a vertex.
+ */
+class RemoveProperty : public LogicalOperator {
+ public:
+  RemoveProperty(const std::shared_ptr<LogicalOperator> input,
+                 PropertyLookup *lhs)
+      : input_(input), lhs_(lhs) {}
+
+  void Accept(LogicalOperatorVisitor &visitor) override {
+    visitor.Visit(*this);
+    input_->Accept(visitor);
+    visitor.PostVisit(*this);
+  }
+
+ private:
+  class RemovePropertyCursor : public Cursor {
+   public:
+    RemovePropertyCursor(RemoveProperty &self, GraphDbAccessor &db)
+        : self_(self), input_cursor_(self.input_->MakeCursor(db)) {}
+
+    bool Pull(Frame &frame, SymbolTable &symbol_table) override {
+      if (!input_cursor_->Pull(frame, symbol_table)) return false;
+
+      ExpressionEvaluator evaluator(frame, symbol_table);
+      self_.lhs_->expression_->Accept(evaluator);
+      TypedValue lhs = evaluator.PopBack();
+
+      switch (lhs.type()) {
+        case TypedValue::Type::Vertex:
+          lhs.Value<VertexAccessor>().PropsErase(self_.lhs_->property_);
+          break;
+        case TypedValue::Type::Edge:
+          lhs.Value<EdgeAccessor>().PropsErase(self_.lhs_->property_);
+          break;
+        default:
+          // TODO consider throwing a TypedValueException here
+          // deal with this when we'll be overhauling error-feedback
+          throw QueryRuntimeException(
+              "Properties can only be removed on Vertices and Edges");
+      }
+      return true;
+    }
+
+   private:
+    RemoveProperty &self_;
+    std::unique_ptr<Cursor> input_cursor_;
+  };
+
+ public:
+  std::unique_ptr<Cursor> MakeCursor(GraphDbAccessor &db) override {
+    return std::make_unique<RemovePropertyCursor>(*this, db);
+  }
+
+ private:
+  std::shared_ptr<LogicalOperator> input_;
+  PropertyLookup *lhs_;
+};
+/**
+ * Logical operator for removing an arbitrary number of
+ * labels on a Vertex. If a label does not exist on a Vertex,
+ * nothing happens.
+ */
+class RemoveLabels : public LogicalOperator {
+ public:
+  RemoveLabels(const std::shared_ptr<LogicalOperator> input,
+               const Symbol input_symbol,
+               const std::vector<GraphDbTypes::Label> &labels)
+      : input_(input), input_symbol_(input_symbol), labels_(labels) {}
+
+  void Accept(LogicalOperatorVisitor &visitor) override {
+    visitor.Visit(*this);
+    input_->Accept(visitor);
+    visitor.PostVisit(*this);
+  }
+
+ private:
+  class RemoveLabelsCursor : public Cursor {
+   public:
+    RemoveLabelsCursor(RemoveLabels &self, GraphDbAccessor &db)
+        : self_(self), input_cursor_(self.input_->MakeCursor(db)) {}
+
+    bool Pull(Frame &frame, SymbolTable &symbol_table) override {
+      if (!input_cursor_->Pull(frame, symbol_table)) return false;
+
+      TypedValue vertex_value = frame[self_.input_symbol_];
+      VertexAccessor vertex = vertex_value.Value<VertexAccessor>();
+      for (auto label : self_.labels_) vertex.remove_label(label);
+
+      return true;
+    }
+
+   private:
+    RemoveLabels &self_;
+    std::unique_ptr<Cursor> input_cursor_;
+  };
+
+ public:
+  std::unique_ptr<Cursor> MakeCursor(GraphDbAccessor &db) override {
+    return std::make_unique<RemoveLabelsCursor>(*this, db);
+  }
+
+ private:
+  std::shared_ptr<LogicalOperator> input_;
+  const Symbol input_symbol_;
+  std::vector<GraphDbTypes::Label> labels_;
+};
+
 }  // namespace plan
 }  // namespace query

src/query/frontend/opencypher/grammar/Cypher.g4

@@ -65,7 +65,7 @@ cypherDelete : ( DETACH SP )? DELETE SP? expression ( SP? ',' SP? expression )*
 
 remove : REMOVE SP removeItem ( SP? ',' SP? removeItem )* ;
 
-removeItem : ( variable nodeLabels )
+removeItem : ( variable SP? nodeLabels )
            | propertyExpression
            ;
 

tests/unit/cypher_main_visitor.cpp

@@ -669,4 +669,31 @@ TEST(Visitor, Set) {
                                      ast_generator.db_accessor_->label("i")));
   }
 }
+
+TEST(Visitor, Remove) {
+  AstGenerator ast_generator("REMOVE a.x, g : h : i");
+  auto *query = ast_generator.query_;
+  ASSERT_EQ(query->clauses_.size(), 2U);
+
+  {
+    auto *remove_property = dynamic_cast<RemoveProperty *>(query->clauses_[0]);
+    ASSERT_TRUE(remove_property);
+    ASSERT_TRUE(remove_property->property_lookup_);
+    auto *identifier1 = dynamic_cast<Identifier *>(
+        remove_property->property_lookup_->expression_);
+    ASSERT_TRUE(identifier1);
+    ASSERT_EQ(identifier1->name_, "a");
+    ASSERT_EQ(remove_property->property_lookup_->property_,
+              ast_generator.db_accessor_->property("x"));
+  }
+  {
+    auto *remove_labels = dynamic_cast<RemoveLabels *>(query->clauses_[1]);
+    ASSERT_TRUE(remove_labels);
+    ASSERT_TRUE(remove_labels->identifier_);
+    ASSERT_EQ(remove_labels->identifier_->name_, "g");
+    ASSERT_THAT(remove_labels->labels_,
+                UnorderedElementsAre(ast_generator.db_accessor_->label("h"),
+                                     ast_generator.db_accessor_->label("i")));
+  }
+}
 }

tests/unit/interpreter.cpp

@@ -1004,3 +1004,86 @@ TEST(Interpreter, SetLabels) {
     EXPECT_TRUE(vertex.has_label(label3));
   }
 }
+
+TEST(Interpreter, RemoveProperty) {
+  Dbms dbms;
+  auto dba = dbms.active();
+
+  // graph with 4 vertices in connected pairs
+  // the origin vertex in each par and both edges
+  // have a property set
+  auto prop1 = dba->property("prop1");
+  auto v1 = dba->insert_vertex();
+  auto v2 = dba->insert_vertex();
+  auto v3 = dba->insert_vertex();
+  auto v4 = dba->insert_vertex();
+  auto edge_type = dba->edge_type("edge_type");
+  dba->insert_edge(v1, v3, edge_type).PropsSet(prop1, 42);
+  dba->insert_edge(v2, v4, edge_type);
+  v2.PropsSet(prop1, 42);
+  v3.PropsSet(prop1, 42);
+  v4.PropsSet(prop1, 42);
+  auto prop2 = dba->property("prop2");
+  v1.PropsSet(prop2, 0);
+  v2.PropsSet(prop2, 0);
+  dba->advance_command();
+
+  AstTreeStorage storage;
+  SymbolTable symbol_table;
+
+  // scan (n)-[r]->(m)
+  auto n = MakeScanAll(storage, symbol_table, "n");
+  auto r_m = MakeExpand(storage, symbol_table, n.op_, n.sym_, "r",
+                        EdgeAtom::Direction::RIGHT, false, "m", false);
+
+  auto n_p = PROPERTY_LOOKUP("n", prop1);
+  symbol_table[*n_p->expression_] = n.sym_;
+  auto set_n_p = std::make_shared<plan::RemoveProperty>(r_m.op_, n_p);
+
+  auto r_p = PROPERTY_LOOKUP("r", prop1);
+  symbol_table[*r_p->expression_] = r_m.edge_sym_;
+  auto set_r_p = std::make_shared<plan::RemoveProperty>(set_n_p, r_p);
+  EXPECT_EQ(2, PullAll(set_r_p, *dba, symbol_table));
+  dba->advance_command();
+
+  EXPECT_EQ(CountIterable(dba->edges()), 2);
+  for (EdgeAccessor edge : dba->edges()) {
+    EXPECT_EQ(edge.PropsAt(prop1).type(), PropertyValue::Type::Null);
+    VertexAccessor from = edge.from();
+    VertexAccessor to = edge.to();
+    EXPECT_EQ(from.PropsAt(prop1).type(), PropertyValue::Type::Null);
+    EXPECT_EQ(from.PropsAt(prop2).type(), PropertyValue::Type::Int);
+    EXPECT_EQ(to.PropsAt(prop1).type(), PropertyValue::Type::Int);
+  }
+}
+
+TEST(Interpreter, RemoveLabels) {
+  Dbms dbms;
+  auto dba = dbms.active();
+
+  auto label1 = dba->label("label1");
+  auto label2 = dba->label("label2");
+  auto label3 = dba->label("label3");
+  auto v1 = dba->insert_vertex();
+  v1.add_label(label1);
+  v1.add_label(label2);
+  v1.add_label(label3);
+  auto v2 = dba->insert_vertex();
+  v2.add_label(label1);
+  v2.add_label(label3);
+  dba->advance_command();
+
+  AstTreeStorage storage;
+  SymbolTable symbol_table;
+
+  auto n = MakeScanAll(storage, symbol_table, "n");
+  auto label_remove = std::make_shared<plan::RemoveLabels>(
+      n.op_, n.sym_, std::vector<GraphDbTypes::Label>{label1, label2});
+  EXPECT_EQ(2, PullAll(label_remove, *dba, symbol_table));
+
+  for (VertexAccessor vertex : dba->vertices()) {
+    EXPECT_EQ(1, vertex.labels().size());
+    EXPECT_FALSE(vertex.has_label(label1));
+    EXPECT_FALSE(vertex.has_label(label2));
+  }
+}