From 964e67a2900f474ed06dd22edd090f33a2514e2e 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. --- src/common/expression/AttributeExpression.cpp | 9 +-- .../test/AttributeExpressionTest.cpp | 58 +++++++++++++------ .../expression/test/ExpressionContextMock.h | 13 ++++- .../test/ListComprehensionExpressionTest.cpp | 32 +++++++--- .../test/PredicateExpressionTest.cpp | 34 +++++++---- src/graph/validator/MatchValidator.cpp | 3 +- tests/tck/features/match/Unwind.feature | 33 +++++++++++ 7 files changed, 133 insertions(+), 49 deletions(-) diff --git a/src/common/expression/AttributeExpression.cpp b/src/common/expression/AttributeExpression.cpp index 4f8f7b9c147..fff89680073 100644 --- a/src/common/expression/AttributeExpression.cpp +++ b/src/common/expression/AttributeExpression.cpp @@ -27,13 +27,8 @@ const Value &AttributeExpression::eval(ExpressionContext &ctx) { 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; - } - } - return Value::kNullValue; + LOG(ERROR) << "Var.tar.prop cannot be evaluated as an attribute expression."; + 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..e93e74039ca 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,36 +85,53 @@ 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"}, + {"Venus", Value("Mars")}, + {"Mull", Value("Kintyre")}, }; vertex.tags[1].props = { - {"Bip", "Bop"}, - {"Tug", "War"}, - {"Venus", "RocksShow"}, + {"Bip", Value("Bop")}, + {"Tug", Value("War")}, + {"Venus", Value("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/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}] |