Skip to content

Commit

Permalink
Rewrite attribute expresion to labeltagprop for unwind. Fix other rel…
Browse files Browse the repository at this point in the history 
…ated cases such as ListComprehensionExpression, reduce, etc.
  • Loading branch information
@xtcyclist
xtcyclist committed Dec 15, 2022
1 parent c3866b3 commit 4b3841c
Show file tree
Hide file tree
Showing 11 changed files with 181 additions and 55 deletions.
34 changes: 29 additions & 5 deletions src/common/expression/AttributeExpression.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,41 @@ const Value &AttributeExpression::eval(ExpressionContext &ctx) {
case Value::Type::MAP:
return lvalue.getMap().at(rvalue.getStr());
case Value::Type::VERTEX: {
/*
* WARNING(Xuntao): this entire case shall be removed, in theory.
* 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 a 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<AttributeExpression *>(left())->right()->kind() ==
Expression::Kind::kConstant) {
auto &tagName = dynamic_cast<AttributeExpression *>(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());
Expand Down
48 changes: 34 additions & 14 deletions src/common/expression/test/AttributeExpressionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
*
* This source code is licensed under Apache 2.0 License.
*/
#include <unordered_map>

#include "common/expression/test/TestBase.h"
#include "graph/util/ExpressionUtils.h"

namespace nebula {

Expand Down Expand Up @@ -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"},
Expand All @@ -91,35 +96,50 @@ 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<std::string, graph::AliasType> 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());
}
{
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());
}
Expand Down
13 changes: 11 additions & 2 deletions src/common/expression/test/ExpressionContextMock.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down
32 changes: 23 additions & 9 deletions src/common/expression/test/ListComprehensionExpressionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}})});
Expand All @@ -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<std::string, graph::AliasType> 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);
Expand Down Expand Up @@ -88,17 +94,25 @@ TEST_F(ListComprehensionExpressionTest, ListComprehensionExprToString) {
{
ArgumentList *argList = ArgumentList::make(&pool);
argList->addArgument(LabelExpression::make(&pool, "p"));
std::unordered_map<std::string, graph::AliasType> aliasTypeMap = {
{"n", graph::AliasType::kNode}};
auto expr = ListComprehensionExpression::make(
&pool,
"n",
FunctionCallExpression::make(&pool, "nodes", argList),
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);
Expand Down
34 changes: 23 additions & 11 deletions src/common/expression/test/PredicateExpressionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}})});
Expand All @@ -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<std::string, graph::AliasType> 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);
Expand Down Expand Up @@ -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<std::string, graph::AliasType> 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);
Expand Down
1 change: 1 addition & 0 deletions src/graph/util/ParserUtil.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "common/base/ObjectPool.h"
#include "common/base/Status.h"
#include "common/base/StatusOr.h"
#include "graph/util/ExpressionUtils.h"

namespace nebula {
namespace graph {
Expand Down
16 changes: 13 additions & 3 deletions src/graph/validator/MatchValidator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,8 +473,17 @@ Status MatchValidator::validateReturn(MatchReturn *ret,
ExpressionUtils::hasAny(column->expr(), {Expression::Kind::kAggregate})) {
retClauseCtx.yield->hasAgg_ = true;
}
column->setExpr(ExpressionUtils::rewriteAttr2LabelTagProp(
column->expr(), retClauseCtx.yield->aliasesAvailable));
if (column->expr()->kind() == Expression::Kind::kListComprehension) {
auto expr = dynamic_cast<ListComprehensionExpression *>(column->expr());
if (expr->hasMapping()) {
expr->setMapping(ExpressionUtils::rewriteAttr2LabelTagProp(
expr->mapping(), retClauseCtx.yield->aliasesAvailable));
column->setExpr(expr);
}
} else {
column->setExpr(ExpressionUtils::rewriteAttr2LabelTagProp(
column->expr(), retClauseCtx.yield->aliasesAvailable));
}
exprs.push_back(column->expr());
columns->addColumn(column->clone().release());
}
Expand Down Expand Up @@ -636,7 +645,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());
Expand Down
5 changes: 3 additions & 2 deletions tests/tck/features/expression/ListComprehension.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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 |
Expand Down
8 changes: 5 additions & 3 deletions tests/tck/features/expression/Predicate.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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 |
Expand Down
Loading

0 comments on commit 4b3841c

Please sign in to comment.