Fix crash when rewriting label symbols in match (#434)

* Fix crash when rewriting label symbols in match

* Update label attr expression

* Add test cases

* Fix test case

* Fix test

src/parser/parser.yy

@@ -663,10 +663,12 @@ subscript_expression
 attribute_expression
     : name_label DOT name_label {
         $$ = new LabelAttributeExpression(new LabelExpression($1),
-                                          new LabelExpression($3));
+                                          new ConstantExpression(*$3));
+        delete $3;
     }
     | compound_expression DOT name_label {
-        $$ = new AttributeExpression($1, new LabelExpression($3));
+        $$ = new AttributeExpression($1, new ConstantExpression(*$3));
+        delete $3;
     }
     ;
 
@@ -2349,8 +2351,9 @@ update_item
     }
     | name_label DOT name_label ASSIGN expression {
         auto expr = new LabelAttributeExpression(new LabelExpression($1),
-                                                 new LabelExpression($3));
+                                                 new ConstantExpression(*$3));
         $$ = new UpdateItem(expr, $5);
+        delete $3;
     }
     ;
 

src/parser/test/ExpressionParsingTest.cpp

@@ -262,8 +262,8 @@ TEST_F(ExpressionParsingTest, Associativity) {
 
     ast = make<AttributeExpression>(make<LabelAttributeExpression>(
                                         make<LabelExpression>("a"),
-                                        make<LabelExpression>("b")),
-                                    make<LabelExpression>("c"));
+                                        make<ConstantExpression>("b")),
+                                    make<ConstantExpression>("c"));
     add("a.b.c", ast);
 
     ast = make<LogicalExpression>(Kind::kLogicalAnd,
@@ -436,7 +436,7 @@ TEST_F(ExpressionParsingTest, Precedence) {
                         new std::string("var")),
                     make<ConstantExpression>(0)),
                 make<ConstantExpression>("1")),
-            make<LabelExpression>("m"));
+            make<ConstantExpression>("m"));
     add("$var[0]['1'].m", ast);
 
     ast = make<LogicalExpression>(Kind::kLogicalXor,
@@ -445,15 +445,15 @@ TEST_F(ExpressionParsingTest, Precedence) {
                                           make<AttributeExpression>(
                                               make<LabelAttributeExpression>(
                                                   make<LabelExpression>("a"),
-                                                  make<LabelExpression>("b")),
-                                              make<LabelExpression>("c")),
+                                                  make<ConstantExpression>("b")),
+                                              make<ConstantExpression>("c")),
                                       make<SubscriptExpression>(
                                           make<SubscriptExpression>(
                                               make<AttributeExpression>(
                                                   make<VariablePropertyExpression>(
                                                       new std::string("var"),
                                                       new std::string("p")),
-                                                  make<LabelExpression>("q")),
+                                                  make<ConstantExpression>("q")),
                                               make<LabelExpression>("r")),
                                           make<LabelExpression>("s")))),
                                   make<RelationalExpression>(Kind::kRelIn,

src/planner/planners/MatchSolver.cpp

@@ -80,11 +80,12 @@ Expression* MatchSolver::rewrite(const LabelExpression *label) {
 }
 
 Expression* MatchSolver::rewrite(const LabelAttributeExpression *la) {
+    const auto &value = la->right()->value();
     auto *expr = new AttributeExpression(
             new VariablePropertyExpression(
                 new std::string(),
                 new std::string(*la->left()->name())),
-            new LabelExpression(*la->right()->name()));
+            new ConstantExpression(value));
     return expr;
 }
 
@@ -207,8 +208,9 @@ Expression *MatchSolver::makeIndexFilter(const std::string &label,
             continue;
         }
 
+        const auto &value = la->right()->value();
         auto *tpExpr = new TagPropertyExpression(new std::string(label),
-                                                 new std::string(*la->right()->name()));
+                                                 new std::string(value.getStr()));
         auto *newConstant = constant->clone().release();
         if (left->kind() == Expression::Kind::kLabelAttribute) {
             auto *rel = new RelationalExpression(item->kind(), tpExpr, newConstant);

src/planner/planners/MatchVariableLengthPatternIndexScanPlanner.cpp

@@ -259,8 +259,7 @@ Status MatchVariableLengthPatternIndexScanPlanner::appendFetchVertexPlan(
         RewriteMatchLabelVisitor visitor([](const Expression *expr) {
             DCHECK_EQ(expr->kind(), Expression::Kind::kLabelAttribute);
             auto la = static_cast<const LabelAttributeExpression *>(expr);
-            auto attr = new LabelExpression(*la->right()->name());
-            return new AttributeExpression(new VertexExpression(), attr);
+            return new AttributeExpression(new VertexExpression(), la->right()->clone().release());
         });
         filter->accept(&visitor);
         root = Filter::make(matchCtx_->qctx, root, filter);
@@ -350,8 +349,7 @@ Status MatchVariableLengthPatternIndexScanPlanner::expandStep(const EdgeInfo &ed
         RewriteMatchLabelVisitor visitor([](const Expression *expr) {
             DCHECK_EQ(expr->kind(), Expression::Kind::kLabelAttribute);
             auto la = static_cast<const LabelAttributeExpression *>(expr);
-            auto attr = new LabelExpression(*la->right()->name());
-            return new AttributeExpression(new VertexExpression(), attr);
+            return new AttributeExpression(new VertexExpression(), la->right()->clone().release());
         });
         filter->accept(&visitor);
         auto filterNode = Filter::make(matchCtx_->qctx, root, filter);
@@ -363,8 +361,7 @@ Status MatchVariableLengthPatternIndexScanPlanner::expandStep(const EdgeInfo &ed
         RewriteMatchLabelVisitor visitor([](const Expression *expr) {
             DCHECK_EQ(expr->kind(), Expression::Kind::kLabelAttribute);
             auto la = static_cast<const LabelAttributeExpression *>(expr);
-            auto attr = new LabelExpression(*la->right()->name());
-            return new AttributeExpression(new EdgeExpression(), attr);
+            return new AttributeExpression(new EdgeExpression(), la->right()->clone().release());
         });
         auto filter = edge.filter->clone().release();
         filter->accept(&visitor);

src/util/ExpressionUtils.h

@@ -103,8 +103,9 @@ public:
               typename = std::enable_if_t<std::is_same<To, EdgePropertyExpression>::value ||
                                           std::is_same<To, TagPropertyExpression>::value>>
     static To* rewriteLabelAttribute(LabelAttributeExpression* expr) {
+        const auto& value = expr->right()->value();
         return new To(new std::string(std::move(*expr->left()->name())),
-                      new std::string(std::move(*expr->right()->name())));
+                      new std::string(value.getStr()));
     }
 
     // Clone and fold constant expression

src/util/test/ExpressionUtilsTest.cpp

@@ -368,7 +368,7 @@ TEST_F(ExpressionUtilsTest, pushOrs) {
         auto r = std::make_unique<RelationalExpression>(
             Expression::Kind::kRelEQ,
             new LabelAttributeExpression(new LabelExpression(folly::stringPrintf("tag%d", i)),
-                                         new LabelExpression(folly::stringPrintf("col%d", i))),
+                                         new ConstantExpression(folly::stringPrintf("col%d", i))),
             new ConstantExpression(Value(folly::stringPrintf("val%d", i))));
         rels.emplace_back(std::move(r));
     }

src/validator/IndexScanValidator.cpp

@@ -69,7 +69,8 @@ Status IndexScanValidator::prepareYield() {
         if (col->expr()->kind() == Expression::Kind::kLabelAttribute) {
             auto la = static_cast<LabelAttributeExpression *>(col->expr());
             schemaName = *la->left()->name();
-            colName = *la->right()->name();
+            const auto &value = la->right()->value();
+            colName = value.getStr();
         } else {
             return Status::SemanticError("Yield clauses are not supported : %s",
                                          col->expr()->toString().c_str());
@@ -298,7 +299,7 @@ Status IndexScanValidator::rewriteRelExpr(RelationalExpression* expr) {
                                      la->left()->name()->c_str());
     }
 
-    std::string prop = *la->right()->name();
+    std::string prop = la->right()->value().getStr();
     // rewrite ConstantExpression
     auto c = leftIsAE
              ? checkConstExpr(right, prop)

src/validator/MatchValidator.cpp

@@ -382,7 +382,7 @@ MatchValidator::makeSubFilter(const std::string &alias,
     root = new RelationalExpression(Expression::Kind::kRelEQ,
             new LabelAttributeExpression(
                 new LabelExpression(alias),
-                new LabelExpression(*items[0].first)),
+                new ConstantExpression(*items[0].first)),
             items[0].second->clone().release());
     for (auto i = 1u; i < items.size(); i++) {
         if (items[i].second->kind() != Expression::Kind::kConstant) {
@@ -393,7 +393,7 @@ MatchValidator::makeSubFilter(const std::string &alias,
         auto *right = new RelationalExpression(Expression::Kind::kRelEQ,
             new LabelAttributeExpression(
                 new LabelExpression(alias),
-                new LabelExpression(*items[i].first)),
+                new ConstantExpression(*items[i].first)),
             items[i].second->clone().release());
         root = new LogicalExpression(Expression::Kind::kLogicalAnd, left, right);
     }

src/validator/MutateValidator.cpp

@@ -545,7 +545,8 @@ Status UpdateValidator::getUpdateProps() {
             auto laExpr = static_cast<const LabelAttributeExpression*>(item->getFieldExpr());
             symNames.emplace(*laExpr->left()->name());
             symName = laExpr->left()->name();
-            fieldName = *laExpr->right()->name();
+            const auto &value = laExpr->right()->value();
+            fieldName = value.getStr();
         }
         auto valueExpr = item->value();
         if (valueExpr == nullptr) {

src/visitor/CollectAllExprsVisitor.cpp

@@ -115,10 +115,6 @@ void CollectAllExprsVisitor::visit(LabelAttributeExpression *expr) {
     collectExpr(expr);
 }
 
-void CollectAllExprsVisitor::visit(AttributeExpression *expr) {
-    collectExpr(expr);
-}
-
 void CollectAllExprsVisitor::visit(VertexExpression *expr) {
     collectExpr(expr);
 }

src/visitor/CollectAllExprsVisitor.h

@@ -52,7 +52,6 @@ private:
     void visit(VersionedVariableExpression* expr) override;
     void visit(LabelExpression* expr) override;
     void visit(LabelAttributeExpression* expr) override;
-    void visit(AttributeExpression* expr) override;
     void visit(VertexExpression* expr) override;
     void visit(EdgeExpression* expr) override;
     void visit(CaseExpression* expr) override;

src/visitor/DeduceTypeVisitor.cpp

@@ -312,7 +312,7 @@ void DeduceTypeVisitor::visit(AttributeExpression *expr) {
     if (type_ != Value::Type::STRING && !isSuperiorType(type_)) {
         std::stringstream ss;
         ss << "`" << expr->toString()
-           << "', expected an valid identifier: " << expr->left()->toString();
+           << "', expected an valid identifier: " << expr->right()->toString();
         status_ = Status::SemanticError(ss.str());
         return;
     }
@@ -350,12 +350,12 @@ void DeduceTypeVisitor::visit(LabelAttributeExpression *expr) {
         return;
     }
 
-    const_cast<LabelExpression*>(expr->right())->accept(this);
+    const_cast<ConstantExpression*>(expr->right())->accept(this);
     if (!ok()) return;
     if (type_ != Value::Type::STRING && !isSuperiorType(type_)) {
         std::stringstream ss;
         ss << "`" << expr->toString()
-           << "', expected an valid identifier: " << expr->left()->toString();
+           << "', expected an valid identifier: " << expr->right()->toString();
         status_ = Status::SemanticError(ss.str());
         return;
     }

src/visitor/ExprVisitorImpl.cpp

@@ -49,7 +49,7 @@ void ExprVisitorImpl::visit(LabelAttributeExpression *expr) {
     DCHECK(ok());
     const_cast<LabelExpression *>(expr->left())->accept(this);
     if (ok()) {
-        const_cast<LabelExpression *>(expr->right())->accept(this);
+        const_cast<ConstantExpression *>(expr->right())->accept(this);
     }
 }
 

src/visitor/RewriteLabelAttrVisitor.cpp

@@ -162,7 +162,8 @@ std::vector<std::unique_ptr<Expression>> RewriteLabelAttrVisitor::rewriteExprLis
 
 Expression* RewriteLabelAttrVisitor::createExpr(const LabelAttributeExpression* expr) {
     auto leftName = new std::string(*expr->left()->name());
-    auto rightName = new std::string(*expr->right()->name());
+    const auto &value = expr->right()->value();
+    auto rightName = new std::string(value.getStr());
     if (isTag_) {
         return new TagPropertyExpression(leftName, rightName);
     }

src/visitor/RewriteSymExprVisitor.cpp

@@ -43,8 +43,9 @@ void RewriteSymExprVisitor::visit(LabelExpression *expr) {
 
 void RewriteSymExprVisitor::visit(LabelAttributeExpression *expr) {
     if (isEdge_) {
-        expr_ = std::make_unique<EdgePropertyExpression>(new std::string(*expr->left()->name()),
-                                                         new std::string(*expr->right()->name()));
+        expr_ = std::make_unique<EdgePropertyExpression>(
+            new std::string(*expr->left()->name()),
+            new std::string(expr->right()->value().getStr()));
         hasWrongType_ = false;
     } else {
         hasWrongType_ = true;

tests/query/v2/match/test_variable_length_relationships.py

@@ -733,3 +733,29 @@ class TestVariableLengthRelationshipMatch(NebulaTestSuite):
             ],
         }
         self.check_rows_with_header(stmt, expected)
+
+        stmt='''
+        MATCH p=(v:player{name: 'Tim Duncan'})-[:like|:serve*1..3]->(v1)
+        WHERE e[0].likeness>90
+        RETURN p
+        '''
+        resp = self.execute_query(stmt)
+        self.check_resp_failed(resp)
+        self.check_error_msg(resp, "SemanticError: Alias used but not defined: `e'")
+
+        stmt='''
+        MATCH p=(v:player{name: 'Tim Duncan'})-[:like|:serve*1..3]->(v1)
+        RETURN e
+        '''
+        resp = self.execute_query(stmt)
+        self.check_resp_failed(resp)
+        self.check_error_msg(resp, "SemanticError: Alias used but not defined: `e'")
+
+        stmt='''
+        MATCH p=(v:player{name: 'Tim Duncan'})-[:like|:serve*1..3]->(v1)
+        WHERE e[0].likeness+e[1].likeness>90
+        RETURN p
+        '''
+        resp = self.execute_query(stmt)
+        self.check_resp_failed(resp)
+        self.check_error_msg(resp, "SemanticError: Alias used but not defined: `e'")