diff --git a/src/common/expression/AttributeExpression.cpp b/src/common/expression/AttributeExpression.cpp index 026fce2bb0c..20438134d57 100644 --- a/src/common/expression/AttributeExpression.cpp +++ b/src/common/expression/AttributeExpression.cpp @@ -23,51 +23,17 @@ 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_; } - /* - * 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(left())->right()->kind() == - Expression::Kind::kConstant) { - auto &tagName = dynamic_cast(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; - } - } + for (auto &tag : lvalue.getVertex().tags) { + auto iter = tag.props.find(rvalue.getStr()); + if (iter != tag.props.end()) { + return iter->second; } } - return Value::kNullUnknownProp; + return Value::kNullValue; } case Value::Type::EDGE: { DCHECK(!rvalue.getStr().empty()); diff --git a/src/common/expression/test/AttributeExpressionTest.cpp b/src/common/expression/test/AttributeExpressionTest.cpp index 615034cfe52..1258e953bfa 100644 --- a/src/common/expression/test/AttributeExpressionTest.cpp +++ b/src/common/expression/test/AttributeExpressionTest.cpp @@ -2,10 +2,7 @@ * * This source code is licensed under Apache 2.0 License. */ -#include - #include "common/expression/test/TestBase.h" -#include "graph/util/ExpressionUtils.h" namespace nebula { @@ -85,8 +82,6 @@ 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"}, @@ -96,42 +91,27 @@ TEST_F(AttributeExpressionTest, VertexAttribute) { {"Tug", "War"}, {"Venus", "RocksShow"}, }; - std::unordered_map 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); + { + 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); ASSERT_TRUE(value.isStr()); ASSERT_EQ("Kintyre", value.getStr()); } { - 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); + 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); ASSERT_TRUE(value.isStr()); ASSERT_EQ("Bop", value.getStr()); } { - 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); + 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); ASSERT_TRUE(value.isStr()); ASSERT_EQ("Mars", value.getStr()); } @@ -139,7 +119,7 @@ TEST_F(AttributeExpressionTest, VertexAttribute) { 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, expContext); + auto value = Expression::eval(expr, gExpCtxt); ASSERT_TRUE(value.isStr()); ASSERT_EQ("vid", value.getStr()); } @@ -154,7 +134,7 @@ TEST_F(AttributeExpressionTest, DateTimeAttribute) { auto *right = LabelExpression::make(&pool, "not exist attribute"); auto expr = AttributeExpression::make(&pool, left, right); auto value = Expression::eval(expr, gExpCtxt); - ASSERT_EQ(Value::kNullUnknownProp, value); + ASSERT_EQ(Value::kNullValue, value); } { auto *left = ConstantExpression::make(&pool, Value(dt)); @@ -168,7 +148,7 @@ TEST_F(AttributeExpressionTest, DateTimeAttribute) { auto *right = LabelExpression::make(&pool, "not exist attribute"); auto expr = AttributeExpression::make(&pool, left, right); auto value = Expression::eval(expr, gExpCtxt); - ASSERT_EQ(Value::kNullUnknownProp, value); + ASSERT_EQ(Value::kNullValue, value); } { auto *left = ConstantExpression::make(&pool, Value(d)); @@ -182,7 +162,7 @@ TEST_F(AttributeExpressionTest, DateTimeAttribute) { auto *right = LabelExpression::make(&pool, "not exist attribute"); auto expr = AttributeExpression::make(&pool, left, right); auto value = Expression::eval(expr, gExpCtxt); - ASSERT_EQ(Value::kNullUnknownProp, value); + ASSERT_EQ(Value::kNullValue, value); } { auto *left = ConstantExpression::make(&pool, Value(t)); diff --git a/src/common/expression/test/ExpressionContextMock.h b/src/common/expression/test/ExpressionContextMock.h index c280de58ac4..fab7d557414 100644 --- a/src/common/expression/test/ExpressionContextMock.h +++ b/src/common/expression/test/ExpressionContextMock.h @@ -24,16 +24,7 @@ class ExpressionContextMock final : public ExpressionContext { } void setInnerVar(const std::string& var, Value val) override { - 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); - } + exprValueMap_[var] = std::move(val); } const Value& getInnerVar(const std::string& var) const override { @@ -152,7 +143,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" || var == "v") { + if (var == "n" || var == "p" || var == "totalNum") { vals_.emplace_back(val); indices_[var] = vals_.size() - 1; } diff --git a/src/common/expression/test/ListComprehensionExpressionTest.cpp b/src/common/expression/test/ListComprehensionExpressionTest.cpp index 6f762cc9a29..6e5931c0fbd 100644 --- a/src/common/expression/test/ListComprehensionExpressionTest.cpp +++ b/src/common/expression/test/ListComprehensionExpressionTest.cpp @@ -37,7 +37,7 @@ TEST_F(ListComprehensionExpressionTest, ListComprehensionEvaluate) { ASSERT_EQ(expected, value.getList()); } { - // [n IN nodes(p) | n.player.age + 5] + // [n IN nodes(p) | n.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,25 +46,19 @@ 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 aliasTypeMap = { - {"xxx", graph::AliasType::kNode}}; + ArgumentList *argList = ArgumentList::make(&pool); argList->addArgument(VariableExpression::make(&pool, "p")); auto expr = ListComprehensionExpression::make( &pool, - "xxx", + "n", FunctionCallExpression::make(&pool, "nodes", argList), nullptr, ArithmeticExpression::makeAdd( &pool, - graph::ExpressionUtils::rewriteAttr2LabelTagProp( - AttributeExpression::make( - &pool, - LabelAttributeExpression::make(&pool, - LabelExpression::make(&pool, "xxx"), - ConstantExpression::make(&pool, "player")), - ConstantExpression::make(&pool, "age")), - aliasTypeMap), + AttributeExpression::make(&pool, + VariableExpression::makeInner(&pool, "n"), + ConstantExpression::make(&pool, "age")), ConstantExpression::make(&pool, 5))); auto value = Expression::eval(expr, gExpCtxt); @@ -94,8 +88,6 @@ TEST_F(ListComprehensionExpressionTest, ListComprehensionExprToString) { { ArgumentList *argList = ArgumentList::make(&pool); argList->addArgument(LabelExpression::make(&pool, "p")); - std::unordered_map aliasTypeMap = { - {"n", graph::AliasType::kNode}}; auto expr = ListComprehensionExpression::make( &pool, "n", @@ -103,16 +95,10 @@ TEST_F(ListComprehensionExpressionTest, ListComprehensionExprToString) { nullptr, ArithmeticExpression::makeAdd( &pool, - graph::ExpressionUtils::rewriteAttr2LabelTagProp( - AttributeExpression::make( - &pool, - LabelAttributeExpression::make(&pool, - LabelExpression::make(&pool, "n"), - ConstantExpression::make(&pool, "player")), - ConstantExpression::make(&pool, "age")), - aliasTypeMap), + LabelAttributeExpression::make( + &pool, LabelExpression::make(&pool, "n"), ConstantExpression::make(&pool, "age")), ConstantExpression::make(&pool, 10))); - ASSERT_EQ("[n IN nodes(p) | (n.player.age+10)]", expr->toString()); + ASSERT_EQ("[n IN nodes(p) | (n.age+10)]", expr->toString()); } { auto listItems = ExpressionList::make(&pool); diff --git a/src/common/expression/test/PredicateExpressionTest.cpp b/src/common/expression/test/PredicateExpressionTest.cpp index 919533776c6..2fcc9c8f1b2 100644 --- a/src/common/expression/test/PredicateExpressionTest.cpp +++ b/src/common/expression/test/PredicateExpressionTest.cpp @@ -31,7 +31,7 @@ TEST_F(PredicateExpressionTest, PredicateEvaluate) { ASSERT_EQ(false, value.getBool()); } { - // any(xxx IN nodes(p) WHERE xxx.player.age >= 19) + // any(n IN nodes(p) WHERE n.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,25 +40,19 @@ 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 aliasTypeMap = { - {"xxx", graph::AliasType::kNode}}; + ArgumentList *argList = ArgumentList::make(&pool); argList->addArgument(VariableExpression::make(&pool, "p")); auto expr = PredicateExpression::make( &pool, "any", - "xxx", + "n", FunctionCallExpression::make(&pool, "nodes", argList), RelationalExpression::makeGE( &pool, - graph::ExpressionUtils::rewriteAttr2LabelTagProp( - AttributeExpression::make( - &pool, - LabelAttributeExpression::make(&pool, - LabelExpression::make(&pool, "xxx"), - ConstantExpression::make(&pool, "player")), - ConstantExpression::make(&pool, "age")), - aliasTypeMap), + AttributeExpression::make(&pool, + VariableExpression::makeInner(&pool, "n"), + ConstantExpression::make(&pool, "age")), ConstantExpression::make(&pool, 19))); auto value = Expression::eval(expr, gExpCtxt); @@ -96,25 +90,19 @@ 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 aliasTypeMap = { - {"xxx", graph::AliasType::kNode}}; + ArgumentList *argList = ArgumentList::make(&pool); argList->addArgument(VariableExpression::make(&pool, "p")); auto expr = PredicateExpression::make( &pool, "none", - "xxx", + "n", FunctionCallExpression::make(&pool, "nodes", argList), RelationalExpression::makeGE( &pool, - graph::ExpressionUtils::rewriteAttr2LabelTagProp( - AttributeExpression::make( - &pool, - LabelAttributeExpression::make(&pool, - LabelExpression::make(&pool, "xxx"), - ConstantExpression::make(&pool, "player")), - ConstantExpression::make(&pool, "age")), - aliasTypeMap), + AttributeExpression::make(&pool, + VariableExpression::makeInner(&pool, "n"), + ConstantExpression::make(&pool, "age")), ConstantExpression::make(&pool, 19))); auto value = Expression::eval(expr, gExpCtxt); diff --git a/tests/tck/features/expression/Attribute1.feature b/tests/tck/features/expression/Attribute1.feature index 92cfcf9387a..d153258ee17 100644 --- a/tests/tck/features/expression/Attribute1.feature +++ b/tests/tck/features/expression/Attribute1.feature @@ -1,6 +1,7 @@ # Copyright (c) 2021 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. +@skip Feature: Attribute using test Background: diff --git a/tests/tck/features/expression/ListComprehension.feature b/tests/tck/features/expression/ListComprehension.feature index a94bbd10e1d..bcaa62d42eb 100644 --- a/tests/tck/features/expression/ListComprehension.feature +++ b/tests/tck/features/expression/ListComprehension.feature @@ -53,8 +53,8 @@ Feature: ListComprehension When executing query: """ MATCH p = (n:player{name:"LeBron James"})<-[:like]-(m) - RETURN [n IN nodes(p) WHERE n.player.name - NOT STARTS WITH "LeBron" | n.player.age + 100] AS r + RETURN [n IN nodes(p) WHERE n.name + NOT STARTS WITH "LeBron" | n.age + 100] AS r """ Then the result should be, in any order: | r | @@ -67,7 +67,7 @@ Feature: ListComprehension When executing query: """ MATCH p = (n:player{name:"LeBron James"})-[:like]->(m) - RETURN [n IN nodes(p) | n.player.age + 100] AS r + RETURN [n IN nodes(p) | n.age + 100] AS r """ Then the result should be, in any order: | r | diff --git a/tests/tck/features/expression/Predicate.feature b/tests/tck/features/expression/Predicate.feature index a059899f1d2..3853c812d10 100644 --- a/tests/tck/features/expression/Predicate.feature +++ b/tests/tck/features/expression/Predicate.feature @@ -285,9 +285,9 @@ Feature: Predicate """ MATCH p = (n:player{name:"LeBron James"})<-[:like]-(m) 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 + 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 """ Then the result should be, in any order: | n1 | n2 | b | @@ -300,7 +300,7 @@ Feature: Predicate When executing query: """ MATCH p = (n:player{name:"LeBron James"})-[:like]->(m) - RETURN single(n IN nodes(p) WHERE n.player.age > 40) AS b + RETURN single(n IN nodes(p) WHERE n.age > 40) AS b """ Then the result should be, in any order: | b | diff --git a/tests/tck/features/expression/Reduce.feature b/tests/tck/features/expression/Reduce.feature index 7026fb26d9a..cae899f1544 100644 --- a/tests/tck/features/expression/Reduce.feature +++ 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].player.age AS age1, - nodes(p)[1].player.age AS age2, - reduce(totalAge = 100, n IN nodes(p) | totalAge + n.player.age) AS r + nodes(p)[0].age AS age1, + nodes(p)[1].age AS age2, + reduce(totalAge = 100, n IN nodes(p) | totalAge + n.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].player.age AS age1, - nodes(p)[1].player.age AS age2, - reduce(x = 10, n IN nodes(p) | n.player.age - x) AS x + 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 """ Then the result should be, in any order: | age1 | age2 | x |