fix with & optimizer rule error (#3736)

* fix error * fix collapseProjectRule Co-authored-by: cpw <[email protected]>
vesoft-inc · Jan 26, 2022 · db8672d · db8672d
1 parent 7682bf9
commit db8672d
Show file tree

Hide file tree

Showing 8 changed files with 108 additions and 16 deletions.
diff --git a/src/common/expression/PropertyExpression.h b/src/common/expression/PropertyExpression.h
@@ -155,6 +155,10 @@ class LabelTagPropertyExpression final : public PropertyExpression {
     return label_;
   }
 
+  void setLabel(Expression* label) {
+    label_ = label;
+  }
+
  private:
   LabelTagPropertyExpression(ObjectPool* pool,
                              Expression* label = nullptr,

diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp
@@ -126,18 +126,24 @@ bool ExpressionUtils::isEvaluableExpr(const Expression *expr, const QueryContext
 }
 
 // rewrite Attribute to LabelTagProp
-Expression *ExpressionUtils::rewriteAttr2LabelTagProp(const Expression *expr) {
+Expression *ExpressionUtils::rewriteAttr2LabelTagProp(
+    const Expression *expr, const std::unordered_map<std::string, AliasType> &aliasTypeMap) {
   ObjectPool *pool = expr->getObjPool();
 
-  auto matcher = [](const Expression *e) -> bool {
-    if (e->kind() == Expression::Kind::kAttribute) {
-      auto attrExpr = static_cast<const AttributeExpression *>(e);
-      if (attrExpr->left()->kind() == Expression::Kind::kLabelAttribute &&
-          attrExpr->right()->kind() == Expression::Kind::kConstant) {
-        return true;
-      }
+  auto matcher = [&aliasTypeMap](const Expression *e) -> bool {
+    if (e->kind() != Expression::Kind::kAttribute) {
+      return false;
     }
-    return false;
+    auto attrExpr = static_cast<const AttributeExpression *>(e);
+    if (attrExpr->left()->kind() != Expression::Kind::kLabelAttribute) {
+      return false;
+    }
+    auto label = static_cast<const LabelAttributeExpression *>(attrExpr->left())->left()->name();
+    auto iter = aliasTypeMap.find(label);
+    if (iter == aliasTypeMap.end() || iter->second != AliasType::kNode) {
+      return false;
+    }
+    return true;
   };
 
   auto rewriter = [pool](const Expression *e) -> Expression * {

diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h
@@ -15,14 +15,14 @@
 #include "common/expression/PropertyExpression.h"
 #include "common/expression/TypeCastingExpression.h"
 #include "common/expression/UnaryExpression.h"
+#include "graph/context/ast/CypherAstContext.h"
 #include "graph/visitor/EvaluableExprVisitor.h"
 #include "graph/visitor/FindVisitor.h"
 #include "graph/visitor/RewriteVisitor.h"
 
 namespace nebula {
 class ObjectPool;
 namespace graph {
-
 class ExpressionUtils {
  public:
   explicit ExpressionUtils(...) = delete;
@@ -55,7 +55,8 @@ class ExpressionUtils {
 
   static bool isEvaluableExpr(const Expression* expr, const QueryContext* qctx = nullptr);
 
-  static Expression* rewriteAttr2LabelTagProp(const Expression* expr);
+  static Expression* rewriteAttr2LabelTagProp(
+      const Expression* expr, const std::unordered_map<std::string, AliasType>& aliasTypeMap);
 
   static Expression* rewriteLabelAttr2TagProp(const Expression* expr);
 

diff --git a/src/graph/validator/MatchValidator.cpp b/src/graph/validator/MatchValidator.cpp
@@ -273,7 +273,8 @@ Status MatchValidator::validateFilter(const Expression *filter,
   auto transformRes = ExpressionUtils::filterTransform(filter);
   NG_RETURN_IF_ERROR(transformRes);
   // rewrite Attribute to LabelTagProperty
-  whereClauseCtx.filter = ExpressionUtils::rewriteAttr2LabelTagProp(transformRes.value());
+  whereClauseCtx.filter = ExpressionUtils::rewriteAttr2LabelTagProp(
+      transformRes.value(), whereClauseCtx.aliasesAvailable);
 
   auto typeStatus = deduceExprType(whereClauseCtx.filter);
   NG_RETURN_IF_ERROR(typeStatus);
@@ -383,7 +384,8 @@ Status MatchValidator::validateReturn(MatchReturn *ret,
           ExpressionUtils::hasAny(column->expr(), {Expression::Kind::kAggregate})) {
         retClauseCtx.yield->hasAgg_ = true;
       }
-      column->setExpr(ExpressionUtils::rewriteAttr2LabelTagProp(column->expr()));
+      column->setExpr(ExpressionUtils::rewriteAttr2LabelTagProp(
+          column->expr(), retClauseCtx.yield->aliasesAvailable));
       exprs.push_back(column->expr());
       columns->addColumn(column->clone().release());
     }
@@ -459,7 +461,8 @@ Status MatchValidator::validateWith(const WithClause *with,
   }
   if (with->returnItems()->columns()) {
     for (auto *column : with->returnItems()->columns()->columns()) {
-      column->setExpr(ExpressionUtils::rewriteAttr2LabelTagProp(column->expr()));
+      column->setExpr(ExpressionUtils::rewriteAttr2LabelTagProp(
+          column->expr(), withClauseCtx.yield->aliasesAvailable));
       columns->addColumn(column->clone().release());
     }
   }
@@ -819,7 +822,7 @@ Status MatchValidator::checkAlias(
       auto name = static_cast<const VariablePropertyExpression *>(labelExpr)->prop();
       auto res = getAliasType(aliasesAvailable, name);
       NG_RETURN_IF_ERROR(res);
-      if (res.value() != AliasType::kNode) {
+      if (res.value() == AliasType::kEdge || res.value() == AliasType::kPath) {
         return Status::SemanticError("The type of `%s' should be tag", name.c_str());
       }
       return Status::OK();

diff --git a/src/graph/visitor/RewriteVisitor.cpp b/src/graph/visitor/RewriteVisitor.cpp
@@ -237,6 +237,18 @@ void RewriteVisitor::visit(PathBuildExpression *expr) {
   }
 }
 
+void RewriteVisitor::visit(LabelTagPropertyExpression *expr) {
+  if (!care(expr->kind())) {
+    return;
+  }
+  auto label = expr->label();
+  if (matcher_(label)) {
+    expr->setLabel(rewriter_(label));
+  } else {
+    label->accept(this);
+  }
+}
+
 void RewriteVisitor::visit(AttributeExpression *expr) {
   if (!care(expr->kind())) {
     return;

diff --git a/src/graph/visitor/RewriteVisitor.h b/src/graph/visitor/RewriteVisitor.h
@@ -67,12 +67,12 @@ class RewriteVisitor final : public ExprVisitorImpl {
   void visit(RelationalExpression*) override;
   void visit(SubscriptExpression*) override;
   void visit(PathBuildExpression*) override;
+  void visit(LabelTagPropertyExpression*) override;
   void visit(SubscriptRangeExpression*) override;
   void visit(ConstantExpression*) override {}
   void visit(LabelExpression*) override {}
   void visit(UUIDExpression*) override {}
   void visit(LabelAttributeExpression*) override {}
-  void visit(LabelTagPropertyExpression*) override {}
   void visit(VariableExpression*) override {}
   void visit(VersionedVariableExpression*) override {}
   void visit(TagPropertyExpression*) override {}

diff --git a/tests/tck/features/match/With.feature b/tests/tck/features/match/With.feature
@@ -64,6 +64,38 @@ Feature: With clause
     Then the result should be, in any order:
       | count(a) |
       | 1        |
+    When executing query:
+      """
+      WITH {a:1, b:{c:3, d:{e:5}}} AS x
+      RETURN x.b.d.e
+      """
+    Then the result should be, in any order:
+      | x.b.d.e |
+      | 5       |
+    When executing query:
+      """
+      WITH {a:1, b:{c:3, d:{e:5}}} AS x
+      RETURN x.b.d
+      """
+    Then the result should be, in any order:
+      | x.b.d  |
+      | {e: 5} |
+    When executing query:
+      """
+      WITH {a:1, b:{c:3, d:{e:5}}} AS x
+      RETURN x.b
+      """
+    Then the result should be, in any order:
+      | x.b               |
+      | {c: 3, d: {e: 5}} |
+    When executing query:
+      """
+      WITH {a:1, b:{c:3, d:{e:5}}} AS x
+      RETURN x.c
+      """
+    Then the result should be, in any order:
+      | x.c          |
+      | UNKNOWN_PROP |
 
   Scenario: match with return
     When executing query:
@@ -185,6 +217,23 @@ Feature: With clause
     Then the result should be, in any order, with relax comparison:
       | avg  | max |
       | 90.0 | 90  |
+    When executing query:
+      """
+      MATCH (:player {name:"Tim Duncan"})-[e:like]->(dst)
+      WITH dst AS b
+      RETURN b.player.age AS age, b
+      """
+    Then the result should be, in any order, with relax comparison:
+      | age | b                                                         |
+      | 36  | ("Tony Parker" :player{age: 36, name: "Tony Parker"})     |
+      | 41  | ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) |
+    When executing query:
+      """
+      MATCH (:player {name:"Tim Duncan"})-[e:like]->(dst)
+      WITH dst AS b
+      RETURN b.age AS age, b
+      """
+    Then a SemanticError should be raised at runtime: To get the property of the vertex in `b.age', should use the format `var.tag.prop'
 
   @skip
   Scenario: with match return

diff --git a/tests/tck/features/optimizer/CollapseProjectRule.feature b/tests/tck/features/optimizer/CollapseProjectRule.feature
@@ -47,3 +47,20 @@ Feature: Collapse Project Rule
       | 6  | Project            | 5            |               |
       | 5  | TagIndexPrefixScan | 0            |               |
       | 0  | Start              |              |               |
+    When profiling query:
+      """
+      MATCH (:player {name:"Tim Duncan"})-[e:like]->(dst)
+      WITH dst AS b
+      RETURN b.player.age AS age
+      """
+    Then the result should be, in any order:
+      | age |
+      | 36  |
+      | 41  |
+    And the execution plan should be:
+      | id | name           | dependencies | operator info                                                                                                                                                       |
+      | 11 | Project        | 4            |                                                                                                                                                                     |
+      | 4  | AppendVertices | 3            |                                                                                                                                                                     |
+      | 3  | Traverse       | 8            |                                                                                                                                                                     |
+      | 8  | IndexScan      | 2            | {"indexCtx": {"columnHints":{"scanType":"PREFIX","column":"name","beginValue":"\"Tim Duncan\"","endValue":"__EMPTY__","includeBegin":"true","includeEnd":"false"}}} |
+      | 2  | Start          |              |                                                                                                                                                                     |