From 18510c25e3e20b48f7b0f5416ee14a7533239fd7 Mon Sep 17 00:00:00 2001 From: xtcyclist <7731943+xtcyclist@users.noreply.github.com> Date: Wed, 14 Dec 2022 00:11:34 +0800 Subject: [PATCH] Rewrite attribute expresion to labeltagprop for unwind. Fix other related cases such as ListComprehensionExpression, reduce, etc. --- src/common/expression/AttributeExpression.cpp | 33 +++++++++++-- .../test/AttributeExpressionTest.cpp | 48 +++++++++++++------ .../expression/test/ExpressionContextMock.h | 13 ++++- .../test/ListComprehensionExpressionTest.cpp | 32 +++++++++---- .../test/PredicateExpressionTest.cpp | 34 ++++++++----- src/graph/validator/MatchValidator.cpp | 3 +- .../expression/ListComprehension.feature | 5 +- .../tck/features/expression/Predicate.feature | 8 ++-- tests/tck/features/expression/Reduce.feature | 12 ++--- tests/tck/features/match/Unwind.feature | 33 +++++++++++++ 10 files changed, 168 insertions(+), 53 deletions(-) diff --git a/src/common/expression/AttributeExpression.cpp b/src/common/expression/AttributeExpression.cpp index 4f8f7b9c147..db68851542a 100644 --- a/src/common/expression/AttributeExpression.cpp +++ b/src/common/expression/AttributeExpression.cpp @@ -23,17 +23,40 @@ 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) { + return lvalue; + } 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; + } + } } } - return Value::kNullValue; + return Value::kNullBadType; } 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 1258e953bfa..5a42eca6d71 100644 --- a/src/common/expression/test/AttributeExpressionTest.cpp +++ b/src/common/expression/test/AttributeExpressionTest.cpp @@ -2,7 +2,10 @@ * * This source code is licensed under Apache 2.0 License. */ +#include + #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,27 +96,42 @@ 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 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()); } @@ -119,7 +139,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, gExpCtxt); + auto value = Expression::eval(expr, expContext); ASSERT_TRUE(value.isStr()); ASSERT_EQ("vid", value.getStr()); } diff --git a/src/common/expression/test/ExpressionContextMock.h b/src/common/expression/test/ExpressionContextMock.h index fab7d557414..c280de58ac4 100644 --- a/src/common/expression/test/ExpressionContextMock.h +++ b/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; } diff --git a/src/common/expression/test/ListComprehensionExpressionTest.cpp b/src/common/expression/test/ListComprehensionExpressionTest.cpp index 6e5931c0fbd..6f762cc9a29 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.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 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,6 +94,8 @@ 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", @@ -95,10 +103,16 @@ TEST_F(ListComprehensionExpressionTest, ListComprehensionExprToString) { 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); diff --git a/src/common/expression/test/PredicateExpressionTest.cpp b/src/common/expression/test/PredicateExpressionTest.cpp index 2fcc9c8f1b2..919533776c6 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(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 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 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); diff --git a/src/graph/validator/MatchValidator.cpp b/src/graph/validator/MatchValidator.cpp index cdd21da44b8..dba59f2080c 100644 --- a/src/graph/validator/MatchValidator.cpp +++ b/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()); diff --git a/tests/tck/features/expression/ListComprehension.feature b/tests/tck/features/expression/ListComprehension.feature index 4f9da9257f1..a94bbd10e1d 100644 --- a/tests/tck/features/expression/ListComprehension.feature +++ b/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 | diff --git a/tests/tck/features/expression/Predicate.feature b/tests/tck/features/expression/Predicate.feature index ed1dbcb3880..ae862639a7a 100644 --- a/tests/tck/features/expression/Predicate.feature +++ b/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 | diff --git a/tests/tck/features/expression/Reduce.feature b/tests/tck/features/expression/Reduce.feature index cae899f1544..7026fb26d9a 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].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 | diff --git a/tests/tck/features/match/Unwind.feature b/tests/tck/features/match/Unwind.feature index 40e4893e853..c47456e9d38 100644 --- a/tests/tck/features/match/Unwind.feature +++ b/tests/tck/features/match/Unwind.feature @@ -154,3 +154,36 @@ Feature: Unwind clause RETURN num """ Then a SemanticError should be raised at runtime: Can't use aggregating expressions in unwind clause, `count(b)' + + Scenario: unwind vtp edge expression + When executing query: + """ + match (v:player) + where v.player.name in ["Tim Duncan"] + unwind v.player.age as age + return age; + """ + Then the result should be, in any order: + | age | + | 42 | + When executing query: + """ + match (v:player)-[e:like]-() + where v.player.name in ["Tim Duncan"] + unwind e.likeness as age + return age, e + """ + Then the result should be, in any order: + | age | e | + | 55 | [:like "Marco Belinelli"->"Tim Duncan" @0 {likeness: 55}] | + | 70 | [:like "Danny Green"->"Tim Duncan" @0 {likeness: 70}] | + | 80 | [:like "Tiago Splitter"->"Tim Duncan" @0 {likeness: 80}] | + | 90 | [:like "Manu Ginobili"->"Tim Duncan" @0 {likeness: 90}] | + | 95 | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | + | 80 | [:like "Boris Diaw"->"Tim Duncan" @0 {likeness: 80}] | + | 75 | [:like "LaMarcus Aldridge"->"Tim Duncan" @0 {likeness: 75}] | + | 80 | [:like "Aron Baynes"->"Tim Duncan" @0 {likeness: 80}] | + | 95 | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | + | 95 | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | + | 99 | [:like "Dejounte Murray"->"Tim Duncan" @0 {likeness: 99}] | + | 80 | [:like "Shaquille O'Neal"->"Tim Duncan" @0 {likeness: 80}] |