Fix expressions that shall be rewritten to LabelTagProperty

src/common/expression/AttributeExpression.cpp

@@ -23,14 +23,48 @@ const Value &AttributeExpression::eval(ExpressionContext &ctx) {
     case Value::Type::MAP:
       return lvalue.getMap().at(rvalue.getStr());
     case Value::Type::VERTEX: {
+      /*
+       * WARNING(Xuntao): Vertices shall not be evaluated as AttributeExpressions,
+       * since there shall always be a tag specified in the format of:
+       * var.tag.property. Due to design flaws, however, we have to keep
+       * this case segment.
+       */
       if (rvalue.getStr() == kVid) {
         result_ = lvalue.getVertex().vid;
         return result_;
       }
-      for (auto &tag : lvalue.getVertex().tags) {
-        auto iter = tag.props.find(rvalue.getStr());
-        if (iter != tag.props.end()) {
-          return iter->second;
+      /*
+       * WARNING(Xuntao): the following code snippet is dedicated to address the legacy
+       * problem of treating LabelTagProperty expressions as Attribute expressions.
+       * This snippet is necessary to allow users to write var.tag.prop in
+       * ListComprehensionExpression without making structural changes to the implementation.
+       */
+      if (left()->kind() != Expression::Kind::kAttribute) {
+        if (right()->kind() == Expression::Kind::kConstant &&
+            rvalue.type() == Value::Type::STRING) {
+          auto rStr = rvalue.getStr();
+          for (auto &tag : lvalue.getVertex().tags) {
+            if (rStr.compare(tag.name) == 0) {
+              return lvalue;
+            }
+          }
+          LOG(ERROR) << "Tag not found for: " << rStr
+                     << "Please check whether the related expression "
+                     << "follows the format of vertex.tag.property.";
+        }
+      } else if (left()->kind() == Expression::Kind::kAttribute &&
+                 dynamic_cast<AttributeExpression *>(left())->right()->kind() ==
+                     Expression::Kind::kConstant) {
+        auto &tagName = dynamic_cast<AttributeExpression *>(left())->right()->eval(ctx).getStr();
+        for (auto &tag : lvalue.getVertex().tags) {
+          if (tagName.compare(tag.name) != 0) {
+            continue;
+          } else {
+            auto iter = tag.props.find(rvalue.getStr());
+            if (iter != tag.props.end()) {
+              return iter->second;
+            }
+          }
         }
       }
       return Value::kNullValue;

src/common/expression/test/AttributeExpressionTest.cpp

@@ -2,7 +2,10 @@
  *
  * This source code is licensed under Apache 2.0 License.
  */
+#include <unordered_map>
+
 #include "common/expression/test/TestBase.h"
+#include "graph/util/ExpressionUtils.h"
 
 namespace nebula {
 
@@ -82,6 +85,8 @@ TEST_F(AttributeExpressionTest, VertexAttribute) {
   Vertex vertex;
   vertex.vid = "vid";
   vertex.tags.resize(2);
+  vertex.tags[0].name = "tag0";
+  vertex.tags[1].name = "tag1";
   vertex.tags[0].props = {
       {"Venus", "Mars"},
       {"Mull", "Kintyre"},
@@ -91,35 +96,50 @@ TEST_F(AttributeExpressionTest, VertexAttribute) {
       {"Tug", "War"},
       {"Venus", "RocksShow"},
   };
-  {
-    auto *left = ConstantExpression::make(&pool, Value(vertex));
-    auto *right = LabelExpression::make(&pool, "Mull");
-    auto expr = AttributeExpression::make(&pool, left, right);
-    auto value = Expression::eval(expr, gExpCtxt);
+  std::unordered_map<std::string, graph::AliasType> aliasTypeMap = {{"v", graph::AliasType::kNode}};
+  ExpressionContextMock expContext;
+  expContext.setVar("v", Value(vertex));
+  {
+    auto *left = LabelExpression::make(&pool, "v");
+    auto *right = ConstantExpression::make(&pool, "tag0");
+    auto *labelAttribute = LabelAttributeExpression::make(&pool, left, right);
+    auto expr =
+        AttributeExpression::make(&pool, labelAttribute, ConstantExpression::make(&pool, "Mull"));
+    auto rewriteExpr =
+        graph::ExpressionUtils::rewriteAttr2LabelTagProp(expr->clone(), aliasTypeMap);
+    auto value = Expression::eval(rewriteExpr, expContext);
     ASSERT_TRUE(value.isStr());
     ASSERT_EQ("Kintyre", value.getStr());
   }
   {
-    auto *left = ConstantExpression::make(&pool, Value(vertex));
-    auto *right = LabelExpression::make(&pool, "Bip");
-    auto expr = AttributeExpression::make(&pool, left, right);
-    auto value = Expression::eval(expr, gExpCtxt);
+    auto *left = LabelExpression::make(&pool, "v");
+    auto *right = ConstantExpression::make(&pool, "tag1");
+    auto *labelAttribute = LabelAttributeExpression::make(&pool, left, right);
+    auto expr =
+        AttributeExpression::make(&pool, labelAttribute, ConstantExpression::make(&pool, "Bip"));
+    auto rewriteExpr =
+        graph::ExpressionUtils::rewriteAttr2LabelTagProp(expr->clone(), aliasTypeMap);
+    auto value = Expression::eval(rewriteExpr, expContext);
     ASSERT_TRUE(value.isStr());
     ASSERT_EQ("Bop", value.getStr());
   }
   {
-    auto *left = ConstantExpression::make(&pool, Value(vertex));
-    auto *right = LabelExpression::make(&pool, "Venus");
-    auto expr = AttributeExpression::make(&pool, left, right);
-    auto value = Expression::eval(expr, gExpCtxt);
+    auto *left = LabelExpression::make(&pool, "v");
+    auto *right = ConstantExpression::make(&pool, "tag0");
+    auto *labelAttribute = LabelAttributeExpression::make(&pool, left, right);
+    auto expr =
+        AttributeExpression::make(&pool, labelAttribute, ConstantExpression::make(&pool, "Venus"));
+    auto rewriteExpr =
+        graph::ExpressionUtils::rewriteAttr2LabelTagProp(expr->clone(), aliasTypeMap);
+    auto value = Expression::eval(rewriteExpr, expContext);
     ASSERT_TRUE(value.isStr());
     ASSERT_EQ("Mars", value.getStr());
   }
   {
     auto *left = ConstantExpression::make(&pool, Value(vertex));
     auto *right = LabelExpression::make(&pool, "_vid");
     auto expr = AttributeExpression::make(&pool, left, right);
-    auto value = Expression::eval(expr, gExpCtxt);
+    auto value = Expression::eval(expr, expContext);
     ASSERT_TRUE(value.isStr());
     ASSERT_EQ("vid", value.getStr());
   }

src/common/expression/test/ExpressionContextMock.h

@@ -24,7 +24,16 @@ class ExpressionContextMock final : public ExpressionContext {
   }
 
   void setInnerVar(const std::string& var, Value val) override {
-    exprValueMap_[var] = std::move(val);
+    if (var == "xxx") {
+      if (vals_.empty()) {
+        vals_.emplace_back(val);
+        indices_[var] = vals_.size() - 1;
+      } else {
+        vals_[indices_[var]] = val;
+      }
+    } else {
+      exprValueMap_[var] = std::move(val);
+    }
   }
 
   const Value& getInnerVar(const std::string& var) const override {
@@ -143,7 +152,7 @@ class ExpressionContextMock final : public ExpressionContext {
 
   void setVar(const std::string& var, Value val) override {
     // used by tests of list comprehesion, predicate or reduce
-    if (var == "n" || var == "p" || var == "totalNum") {
+    if (var == "n" || var == "p" || var == "totalNum" || var == "v") {
       vals_.emplace_back(val);
       indices_[var] = vals_.size() - 1;
     }

src/common/expression/test/ListComprehensionExpressionTest.cpp

@@ -37,7 +37,7 @@ TEST_F(ListComprehensionExpressionTest, ListComprehensionEvaluate) {
     ASSERT_EQ(expected, value.getList());
   }
   {
-    // [n IN nodes(p) | n.age + 5]
+    // [n IN nodes(p) | n.player.age + 5]
     auto v1 = Vertex("101", {Tag("player", {{"name", "joe"}, {"age", 18}})});
     auto v2 = Vertex("102", {Tag("player", {{"name", "amber"}, {"age", 19}})});
     auto v3 = Vertex("103", {Tag("player", {{"name", "shawdan"}, {"age", 20}})});
@@ -46,19 +46,25 @@ TEST_F(ListComprehensionExpressionTest, ListComprehensionEvaluate) {
     path.steps.emplace_back(Step(v2, 1, "like", 0, {}));
     path.steps.emplace_back(Step(v3, 1, "like", 0, {}));
     gExpCtxt.setVar("p", path);
-
+    std::unordered_map<std::string, graph::AliasType> aliasTypeMap = {
+        {"xxx", graph::AliasType::kNode}};
     ArgumentList *argList = ArgumentList::make(&pool);
     argList->addArgument(VariableExpression::make(&pool, "p"));
     auto expr = ListComprehensionExpression::make(
         &pool,
-        "n",
+        "xxx",
         FunctionCallExpression::make(&pool, "nodes", argList),
         nullptr,
         ArithmeticExpression::makeAdd(
             &pool,
-            AttributeExpression::make(&pool,
-                                      VariableExpression::makeInner(&pool, "n"),
-                                      ConstantExpression::make(&pool, "age")),
+            graph::ExpressionUtils::rewriteAttr2LabelTagProp(
+                AttributeExpression::make(
+                    &pool,
+                    LabelAttributeExpression::make(&pool,
+                                                   LabelExpression::make(&pool, "xxx"),
+                                                   ConstantExpression::make(&pool, "player")),
+                    ConstantExpression::make(&pool, "age")),
+                aliasTypeMap),
             ConstantExpression::make(&pool, 5)));
 
     auto value = Expression::eval(expr, gExpCtxt);
@@ -88,17 +94,25 @@ TEST_F(ListComprehensionExpressionTest, ListComprehensionExprToString) {
   {
     ArgumentList *argList = ArgumentList::make(&pool);
     argList->addArgument(LabelExpression::make(&pool, "p"));
+    std::unordered_map<std::string, graph::AliasType> aliasTypeMap = {
+        {"n", graph::AliasType::kNode}};
     auto expr = ListComprehensionExpression::make(
         &pool,
         "n",
         FunctionCallExpression::make(&pool, "nodes", argList),
         nullptr,
         ArithmeticExpression::makeAdd(
             &pool,
-            LabelAttributeExpression::make(
-                &pool, LabelExpression::make(&pool, "n"), ConstantExpression::make(&pool, "age")),
+            graph::ExpressionUtils::rewriteAttr2LabelTagProp(
+                AttributeExpression::make(
+                    &pool,
+                    LabelAttributeExpression::make(&pool,
+                                                   LabelExpression::make(&pool, "n"),
+                                                   ConstantExpression::make(&pool, "player")),
+                    ConstantExpression::make(&pool, "age")),
+                aliasTypeMap),
             ConstantExpression::make(&pool, 10)));
-    ASSERT_EQ("[n IN nodes(p) | (n.age+10)]", expr->toString());
+    ASSERT_EQ("[n IN nodes(p) | (n.player.age+10)]", expr->toString());
   }
   {
     auto listItems = ExpressionList::make(&pool);

src/common/expression/test/PredicateExpressionTest.cpp

@@ -31,7 +31,7 @@ TEST_F(PredicateExpressionTest, PredicateEvaluate) {
     ASSERT_EQ(false, value.getBool());
   }
   {
-    // any(n IN nodes(p) WHERE n.age >= 19)
+    // any(xxx IN nodes(p) WHERE xxx.player.age >= 19)
     auto v1 = Vertex("101", {Tag("player", {{"name", "joe"}, {"age", 18}})});
     auto v2 = Vertex("102", {Tag("player", {{"name", "amber"}, {"age", 19}})});
     auto v3 = Vertex("103", {Tag("player", {{"name", "shawdan"}, {"age", 20}})});
@@ -40,19 +40,25 @@ TEST_F(PredicateExpressionTest, PredicateEvaluate) {
     path.steps.emplace_back(Step(v2, 1, "like", 0, {}));
     path.steps.emplace_back(Step(v3, 1, "like", 0, {}));
     gExpCtxt.setVar("p", path);
-
+    std::unordered_map<std::string, graph::AliasType> aliasTypeMap = {
+        {"xxx", graph::AliasType::kNode}};
     ArgumentList *argList = ArgumentList::make(&pool);
     argList->addArgument(VariableExpression::make(&pool, "p"));
     auto expr = PredicateExpression::make(
         &pool,
         "any",
-        "n",
+        "xxx",
         FunctionCallExpression::make(&pool, "nodes", argList),
         RelationalExpression::makeGE(
             &pool,
-            AttributeExpression::make(&pool,
-                                      VariableExpression::makeInner(&pool, "n"),
-                                      ConstantExpression::make(&pool, "age")),
+            graph::ExpressionUtils::rewriteAttr2LabelTagProp(
+                AttributeExpression::make(
+                    &pool,
+                    LabelAttributeExpression::make(&pool,
+                                                   LabelExpression::make(&pool, "xxx"),
+                                                   ConstantExpression::make(&pool, "player")),
+                    ConstantExpression::make(&pool, "age")),
+                aliasTypeMap),
             ConstantExpression::make(&pool, 19)));
 
     auto value = Expression::eval(expr, gExpCtxt);
@@ -90,19 +96,25 @@ TEST_F(PredicateExpressionTest, PredicateEvaluate) {
     path.steps.emplace_back(Step(v2, 1, "like", 0, {}));
     path.steps.emplace_back(Step(v3, 1, "like", 0, {}));
     gExpCtxt.setVar("p", path);
-
+    std::unordered_map<std::string, graph::AliasType> aliasTypeMap = {
+        {"xxx", graph::AliasType::kNode}};
     ArgumentList *argList = ArgumentList::make(&pool);
     argList->addArgument(VariableExpression::make(&pool, "p"));
     auto expr = PredicateExpression::make(
         &pool,
         "none",
-        "n",
+        "xxx",
         FunctionCallExpression::make(&pool, "nodes", argList),
         RelationalExpression::makeGE(
             &pool,
-            AttributeExpression::make(&pool,
-                                      VariableExpression::makeInner(&pool, "n"),
-                                      ConstantExpression::make(&pool, "age")),
+            graph::ExpressionUtils::rewriteAttr2LabelTagProp(
+                AttributeExpression::make(
+                    &pool,
+                    LabelAttributeExpression::make(&pool,
+                                                   LabelExpression::make(&pool, "xxx"),
+                                                   ConstantExpression::make(&pool, "player")),
+                    ConstantExpression::make(&pool, "age")),
+                aliasTypeMap),
             ConstantExpression::make(&pool, 19)));
 
     auto value = Expression::eval(expr, gExpCtxt);

src/graph/validator/MatchValidator.cpp

@@ -636,7 +636,8 @@ Status MatchValidator::validateUnwind(const UnwindClause *unwindClause,
     return Status::SemanticError("Expression in UNWIND must be aliased (use AS)");
   }
   unwindCtx.alias = unwindClause->alias();
-  unwindCtx.unwindExpr = unwindClause->expr()->clone();
+  unwindCtx.unwindExpr = ExpressionUtils::rewriteAttr2LabelTagProp(unwindClause->expr()->clone(),
+                                                                   unwindCtx.aliasesAvailable);
   if (ExpressionUtils::hasAny(unwindCtx.unwindExpr, {Expression::Kind::kAggregate})) {
     return Status::SemanticError("Can't use aggregating expressions in unwind clause, `%s'",
                                  unwindCtx.unwindExpr->toString().c_str());

tests/tck/features/expression/ListComprehension.feature

@@ -53,7 +53,8 @@ Feature: ListComprehension
     When executing query:
       """
       MATCH p = (n:player{name:"LeBron James"})<-[:like]-(m)
-      RETURN [n IN nodes(p) WHERE n.name NOT STARTS WITH "LeBron" | n.age + 100] AS r
+      RETURN [n IN nodes(p) WHERE n.player.name
+      NOT STARTS WITH "LeBron" | n.player.age + 100] AS r
       """
     Then the result should be, in any order:
       | r     |
@@ -66,7 +67,7 @@ Feature: ListComprehension
     When executing query:
       """
       MATCH p = (n:player{name:"LeBron James"})-[:like]->(m)
-      RETURN [n IN nodes(p) | n.age + 100] AS r
+      RETURN [n IN nodes(p) | n.player.age + 100] AS r
       """
     Then the result should be, in any order:
       | r          |

tests/tck/features/expression/Predicate.feature

@@ -185,8 +185,10 @@ Feature: Predicate
     When executing query:
       """
       MATCH p = (n:player{name:"LeBron James"})<-[:like]-(m)
-      RETURN nodes(p)[0].name AS n1, nodes(p)[1].name AS n2,
-      all(n IN nodes(p) WHERE n.name NOT STARTS WITH "D") AS b
+      RETURN
+        nodes(p)[0].player.name AS n1,
+        nodes(p)[1].player.name AS n2,
+        all(n IN nodes(p) WHERE n.player.name NOT STARTS WITH "D") AS b
       """
     Then the result should be, in any order:
       | n1             | n2                | b     |
@@ -199,7 +201,7 @@ Feature: Predicate
     When executing query:
       """
       MATCH p = (n:player{name:"LeBron James"})-[:like]->(m)
-      RETURN single(n IN nodes(p) WHERE n.age > 40) AS b
+      RETURN single(n IN nodes(p) WHERE n.player.age > 40) AS b
       """
     Then the result should be, in any order:
       | b    |

tests/tck/features/expression/Reduce.feature

@@ -40,9 +40,9 @@ Feature: Reduce
       """
       MATCH p = (n:player{name:"LeBron James"})<-[:like]-(m)
       RETURN
-        nodes(p)[0].age AS age1,
-        nodes(p)[1].age AS age2,
-        reduce(totalAge = 100, n IN nodes(p) | totalAge + n.age) AS r
+        nodes(p)[0].player.age AS age1,
+        nodes(p)[1].player.age AS age2,
+        reduce(totalAge = 100, n IN nodes(p) | totalAge + n.player.age) AS r
       """
     Then the result should be, in any order:
       | age1 | age2 | r   |
@@ -55,9 +55,9 @@ Feature: Reduce
     When executing query:
       """
       MATCH p = (n:player{name:"LeBron James"})-[:like]->(m)
-      RETURN nodes(p)[0].age AS age1,
-             nodes(p)[1].age AS age2,
-             reduce(x = 10, n IN nodes(p) | n.age - x) AS x
+      RETURN nodes(p)[0].player.age AS age1,
+             nodes(p)[1].player.age AS age2,
+             reduce(x = 10, n IN nodes(p) | n.player.age - x) AS x
       """
     Then the result should be, in any order:
       | age1 | age2 | x  |