Make Where and NamedExpression macros easier to use

Summary:
`Where` can now be constructed in a `QUERY`, instead of requiring manual
addition to `Match`. For example:

    auto query = QUERY(MATCH(pattern), WHERE(expr), ...);

compared to:

    auto match = MATCH(pattern);
    match->where_ = WHERE(expr);
    auto query = QUERY(match, ...);

Similarly, `AS` can be used instead of `NEXPR` to create
`NamedExpressions` only with a name. This is meant to be used with
`RETURN` which will look at the previous `Expression` and store it
inside `NamedExpression`. For example:

    auto ret = RETURN(IDENT("n"), AS("n"),
                      PROPERTY_LOOKUP("n", prop), AS("prop_val"));

compared to:

    auto ret = RETURN(NEXPR("n", IDENT("n")),
                      NEXPR("prop_val", PROPERTY_LOOKUP("n", prop)));

Reviewers: florijan, mislav.bradac

Reviewed By: florijan

Subscribers: pullbot

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

src/query/frontend/ast/ast.hpp

@@ -375,7 +375,8 @@ class NamedExpression : public Tree {
 
  protected:
   NamedExpression(int uid) : Tree(uid) {}
-  NamedExpression(int uid, std::string name, Expression *expression)
+  NamedExpression(int uid, const std::string &name) : Tree(uid), name_(name) {}
+  NamedExpression(int uid, const std::string &name, Expression *expression)
       : Tree(uid), name_(name), expression_(expression) {}
 };
 

tests/unit/query_common.hpp

@@ -72,23 +72,54 @@ auto GetWithPatterns(AstTreeStorage &storage, std::vector<Pattern *> patterns) {
 ///
 /// Create a query with given clauses.
 ///
-auto GetQuery(AstTreeStorage &storage, std::vector<Clause *> clauses) {
+auto GetQuery(AstTreeStorage &storage, Clause *clause) {
-  auto query = storage.query();
+  storage.query()->clauses_.emplace_back(clause);
-  query->clauses_.insert(query->clauses_.begin(), clauses.begin(),
+  return storage.query();
-                         clauses.end());
+}
-  return query;
+template <class... T>
+auto GetQuery(AstTreeStorage &storage, Clause *clause, T *... clauses) {
+  storage.query()->clauses_.emplace_back(clause);
+  return GetQuery(storage, clauses...);
+}
+template <class... T>
+auto GetQuery(AstTreeStorage &storage, Match *match, Where *where,
+              T *... clauses) {
+  match->where_ = where;
+  storage.query()->clauses_.emplace_back(match);
+  return GetQuery(storage, clauses...);
 }
 
 ///
 /// Create the return clause with given named expressions.
 ///
-auto GetReturn(AstTreeStorage &storage,
+auto GetReturn(Return *ret, NamedExpression *named_expr) {
-               std::vector<NamedExpression *> named_exprs) {
+  ret->named_expressions_.emplace_back(named_expr);
-  auto ret = storage.Create<Return>();
-  ret->named_expressions_.insert(ret->named_expressions_.begin(),
-                                 named_exprs.begin(), named_exprs.end());
   return ret;
 }
+auto GetReturn(Return *ret, Expression *expr, NamedExpression *named_expr) {
+  // This overload supports `RETURN(expr, AS(name))` construct, since
+  // NamedExpression does not inherit Expression.
+  named_expr->expression_ = expr;
+  ret->named_expressions_.emplace_back(named_expr);
+  return ret;
+}
+template <class... T>
+auto GetReturn(Return *ret, Expression *expr, NamedExpression *named_expr,
+               T *... rest) {
+  named_expr->expression_ = expr;
+  ret->named_expressions_.emplace_back(named_expr);
+  return GetReturn(ret, rest...);
+}
+template <class... T>
+auto GetReturn(Return *ret, NamedExpression *named_expr, T *... rest) {
+  ret->named_expressions_.emplace_back(named_expr);
+  return GetReturn(ret, rest...);
+}
+template <class... T>
+auto GetReturn(AstTreeStorage &storage, T *... exprs) {
+  auto ret = storage.Create<Return>();
+  return GetReturn(ret, exprs...);
+}
 
 ///
 /// Create the delete clause with given named expressions.
@@ -158,10 +189,14 @@ auto GetSet(AstTreeStorage &storage, const std::string &name,
 #define PROPERTY_LOOKUP(...) \
   query::test_common::GetPropertyLookup(storage, __VA_ARGS__)
 #define NEXPR(name, expr) storage.Create<query::NamedExpression>((name), (expr))
-#define RETURN(...) query::test_common::GetReturn(storage, {__VA_ARGS__})
+// AS is alternative to NEXPR which does not initialize NamedExpression with
+// Expression. It should be used with RETURN. For example:
+// RETURN(IDENT("n"), AS("n")) vs. RETURN(NEXPR("n", IDENT("n"))).
+#define AS(name) storage.Create<query::NamedExpression>((name))
+#define RETURN(...) query::test_common::GetReturn(storage, __VA_ARGS__)
 #define DELETE(...) query::test_common::GetDelete(storage, {__VA_ARGS__})
 #define DETACH_DELETE(...) \
   query::test_common::GetDelete(storage, {__VA_ARGS__}, true)
 #define SET(...) query::test_common::GetSet(storage, __VA_ARGS__)
-#define QUERY(...) query::test_common::GetQuery(storage, {__VA_ARGS__})
+#define QUERY(...) query::test_common::GetQuery(storage, __VA_ARGS__)
 #define LESS(expr1, expr2) storage.Create<query::LessOperator>((expr1), (expr2))

tests/unit/query_planner.cpp

@@ -60,15 +60,14 @@ auto CheckPlan(query::Query &query, std::list<size_t> expected_types) {
 TEST(TestLogicalPlanner, MatchNodeReturn) {
   // Test MATCH (n) RETURN n AS n
   AstTreeStorage storage;
-  auto query = QUERY(MATCH(PATTERN(NODE("n"))), RETURN(NEXPR("n", IDENT("n"))));
+  auto query = QUERY(MATCH(PATTERN(NODE("n"))), RETURN(IDENT("n"), AS("n")));
   CheckPlan(*query, {typeid(ScanAll).hash_code(), typeid(Produce).hash_code()});
 }
 
 TEST(TestLogicalPlanner, CreateNodeReturn) {
   // Test CREATE (n) RETURN n AS n
   AstTreeStorage storage;
-  auto query =
+  auto query = QUERY(CREATE(PATTERN(NODE("n"))), RETURN(IDENT("n"), AS("n")));
-      QUERY(CREATE(PATTERN(NODE("n"))), RETURN(NEXPR("n", IDENT("n"))));
   CheckPlan(*query,
             {typeid(CreateNode).hash_code(), typeid(Produce).hash_code()});
 }
@@ -120,7 +119,7 @@ TEST(TestLogicalPlanner, MatchLabeledNodes) {
   AstTreeStorage storage;
   std::string label("label");
   auto query =
-      QUERY(MATCH(PATTERN(NODE("n", &label))), RETURN(NEXPR("n", IDENT("n"))));
+      QUERY(MATCH(PATTERN(NODE("n", &label))), RETURN(IDENT("n"), AS("n")));
   CheckPlan(*query,
             {typeid(ScanAll).hash_code(), typeid(NodeFilter).hash_code(),
              typeid(Produce).hash_code()});
@@ -132,7 +131,7 @@ TEST(TestLogicalPlanner, MatchPathReturn) {
   std::string relationship("relationship");
   auto query =
       QUERY(MATCH(PATTERN(NODE("n"), EDGE("r", &relationship), NODE("m"))),
-            RETURN(NEXPR("n", IDENT("n"))));
+            RETURN(IDENT("n"), AS("n")));
   CheckPlan(*query,
             {typeid(ScanAll).hash_code(), typeid(Expand).hash_code(),
              typeid(EdgeFilter).hash_code(), typeid(Produce).hash_code()});
@@ -142,9 +141,9 @@ TEST(TestLogicalPlanner, MatchWhereReturn) {
   // Test MATCH (n) WHERE n.property < 42 RETURN n AS n
   AstTreeStorage storage;
   std::string property("property");
-  auto match = MATCH(PATTERN(NODE("n")));
+  auto query = QUERY(MATCH(PATTERN(NODE("n"))),
-  match->where_ = WHERE(LESS(PROPERTY_LOOKUP("n", &property), LITERAL(42)));
+                     WHERE(LESS(PROPERTY_LOOKUP("n", &property), LITERAL(42))),
-  auto query = QUERY(match, RETURN(NEXPR("n", IDENT("n"))));
+                     RETURN(IDENT("n"), AS("n")));
   CheckPlan(*query, {typeid(ScanAll).hash_code(), typeid(Filter).hash_code(),
                      typeid(Produce).hash_code()});
 }

tests/unit/query_semantic.cpp

@@ -18,7 +18,7 @@ TEST(TestSymbolGenerator, MatchNodeReturn) {
   AstTreeStorage storage;
   // MATCH (node_atom_1) RETURN node_atom_1 AS node_atom_1
   auto query_ast = QUERY(MATCH(PATTERN(NODE("node_atom_1"))),
-                         RETURN(NEXPR("node_atom_1", IDENT("node_atom_1"))));
+                         RETURN(IDENT("node_atom_1"), AS("node_atom_1")));
   SymbolGenerator symbol_generator(symbol_table);
   query_ast->Accept(symbol_generator);
   EXPECT_EQ(symbol_table.max_position(), 2);
@@ -45,7 +45,7 @@ TEST(TestSymbolGenerator, MatchUnboundMultiReturn) {
   // MATCH (node_atom_1) RETURN node_atom_1 AS n, n AS n
   auto query_ast =
       QUERY(MATCH(PATTERN(NODE("node_atom_1"))),
-            RETURN(NEXPR("n", IDENT("node_atom_1")), NEXPR("n", IDENT("n"))));
+            RETURN(IDENT("node_atom_1"), AS("n"), IDENT("n"), AS("n")));
   SymbolGenerator symbol_generator(symbol_table);
   EXPECT_THROW(query_ast->Accept(symbol_generator), UnboundVariableError);
 }
@@ -55,7 +55,7 @@ TEST(TestSymbolGenerator, MatchNodeUnboundReturn) {
   AstTreeStorage storage;
   // AST with unbound variable in return: MATCH (n) RETURN x AS x
   auto query_ast =
-      QUERY(MATCH(PATTERN(NODE("n"))), RETURN(NEXPR("x", IDENT("x"))));
+      QUERY(MATCH(PATTERN(NODE("n"))), RETURN(IDENT("x"), AS("x")));
   SymbolGenerator symbol_generator(symbol_table);
   EXPECT_THROW(query_ast->Accept(symbol_generator), UnboundVariableError);
 }
@@ -68,7 +68,7 @@ TEST(TestSymbolGenerator, MatchSameEdge) {
   // This usually throws a redeclaration error, but we support it.
   auto query_ast = QUERY(
       MATCH(PATTERN(NODE("n"), EDGE("r"), NODE("n"), EDGE("r"), NODE("n"))),
-      RETURN(NEXPR("r", IDENT("r"))));
+      RETURN(IDENT("r"), AS("r")));
   SymbolGenerator symbol_generator(symbol_table);
   query_ast->Accept(symbol_generator);
   EXPECT_EQ(symbol_table.max_position(), 3);
@@ -119,7 +119,7 @@ TEST(TestSymbolGenerator, CreateNodeReturn) {
   AstTreeStorage storage;
   // Simple AST returning a created node: CREATE (n) RETURN n
   auto query_ast =
-      QUERY(CREATE(PATTERN(NODE("n"))), RETURN(NEXPR("n", IDENT("n"))));
+      QUERY(CREATE(PATTERN(NODE("n"))), RETURN(IDENT("n"), AS("n")));
   SymbolGenerator symbol_generator(symbol_table);
   query_ast->Accept(symbol_generator);
   EXPECT_EQ(symbol_table.max_position(), 2);
@@ -233,9 +233,9 @@ TEST(TestSymbolGenerator, MatchWhereUnbound) {
   // Test MATCH (n) WHERE missing < 42 RETURN n AS n
   AstTreeStorage storage;
   std::string property("property");
-  auto match = MATCH(PATTERN(NODE("n")));
+  auto query = QUERY(MATCH(PATTERN(NODE("n"))),
-  match->where_ = WHERE(LESS(IDENT("missing"), LITERAL(42)));
+                     WHERE(LESS(IDENT("missing"), LITERAL(42))),
-  auto query = QUERY(match, RETURN(NEXPR("n", IDENT("n"))));
+                     RETURN(IDENT("n"), AS("n")));
   SymbolTable symbol_table;
   SymbolGenerator symbol_generator(symbol_table);
   EXPECT_THROW(query->Accept(symbol_generator), UnboundVariableError);