From 5865318328c12262602a30139807832d4bf9dd51 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Thu, 13 Jan 2022 10:11:31 +0800 Subject: [PATCH 1/2] fix error --- src/graph/util/ExpressionUtils.cpp | 24 ++++++++++------ src/graph/util/ExpressionUtils.h | 5 ++-- src/graph/validator/MatchValidator.cpp | 11 ++++--- tests/tck/features/expression/Depth.feature | 1 + tests/tck/features/match/With.feature | 32 +++++++++++++++++++++ 5 files changed, 58 insertions(+), 15 deletions(-) diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp index 415532cddd2..4f7a2bed726 100644 --- a/src/graph/util/ExpressionUtils.cpp +++ b/src/graph/util/ExpressionUtils.cpp @@ -126,18 +126,24 @@ bool ExpressionUtils::isEvaluableExpr(const Expression *expr, const QueryContext } // rewrite Attribute to LabelTagProp -Expression *ExpressionUtils::rewriteAttr2LabelTagProp(const Expression *expr) { +Expression *ExpressionUtils::rewriteAttr2LabelTagProp( + const Expression *expr, const std::unordered_map &aliasTypeMap) { ObjectPool *pool = expr->getObjPool(); - auto matcher = [](const Expression *e) -> bool { - if (e->kind() == Expression::Kind::kAttribute) { - auto attrExpr = static_cast(e); - if (attrExpr->left()->kind() == Expression::Kind::kLabelAttribute && - attrExpr->right()->kind() == Expression::Kind::kConstant) { - return true; - } + auto matcher = [&aliasTypeMap](const Expression *e) -> bool { + if (e->kind() != Expression::Kind::kAttribute) { + return false; } - return false; + auto attrExpr = static_cast(e); + if (attrExpr->left()->kind() != Expression::Kind::kLabelAttribute) { + return false; + } + auto label = static_cast(attrExpr->left())->left()->name(); + auto iter = aliasTypeMap.find(label); + if (iter == aliasTypeMap.end() || iter->second != AliasType::kNode) { + return false; + } + return true; }; auto rewriter = [pool](const Expression *e) -> Expression * { diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h index f42241f348b..6f435fdf386 100644 --- a/src/graph/util/ExpressionUtils.h +++ b/src/graph/util/ExpressionUtils.h @@ -15,6 +15,7 @@ #include "common/expression/PropertyExpression.h" #include "common/expression/TypeCastingExpression.h" #include "common/expression/UnaryExpression.h" +#include "graph/context/ast/CypherAstContext.h" #include "graph/visitor/EvaluableExprVisitor.h" #include "graph/visitor/FindVisitor.h" #include "graph/visitor/RewriteVisitor.h" @@ -22,7 +23,6 @@ namespace nebula { class ObjectPool; namespace graph { - class ExpressionUtils { public: explicit ExpressionUtils(...) = delete; @@ -55,7 +55,8 @@ class ExpressionUtils { static bool isEvaluableExpr(const Expression* expr, const QueryContext* qctx = nullptr); - static Expression* rewriteAttr2LabelTagProp(const Expression* expr); + static Expression* rewriteAttr2LabelTagProp( + const Expression* expr, const std::unordered_map& aliasTypeMap); static Expression* rewriteLabelAttr2TagProp(const Expression* expr); diff --git a/src/graph/validator/MatchValidator.cpp b/src/graph/validator/MatchValidator.cpp index 2f704d8e53d..b079c3e7709 100644 --- a/src/graph/validator/MatchValidator.cpp +++ b/src/graph/validator/MatchValidator.cpp @@ -273,7 +273,8 @@ Status MatchValidator::validateFilter(const Expression *filter, auto transformRes = ExpressionUtils::filterTransform(filter); NG_RETURN_IF_ERROR(transformRes); // rewrite Attribute to LabelTagProperty - whereClauseCtx.filter = ExpressionUtils::rewriteAttr2LabelTagProp(transformRes.value()); + whereClauseCtx.filter = ExpressionUtils::rewriteAttr2LabelTagProp( + transformRes.value(), whereClauseCtx.aliasesAvailable); auto typeStatus = deduceExprType(whereClauseCtx.filter); NG_RETURN_IF_ERROR(typeStatus); @@ -383,7 +384,8 @@ Status MatchValidator::validateReturn(MatchReturn *ret, ExpressionUtils::hasAny(column->expr(), {Expression::Kind::kAggregate})) { retClauseCtx.yield->hasAgg_ = true; } - column->setExpr(ExpressionUtils::rewriteAttr2LabelTagProp(column->expr())); + column->setExpr(ExpressionUtils::rewriteAttr2LabelTagProp( + column->expr(), retClauseCtx.yield->aliasesAvailable)); exprs.push_back(column->expr()); columns->addColumn(column->clone().release()); } @@ -459,7 +461,8 @@ Status MatchValidator::validateWith(const WithClause *with, } if (with->returnItems()->columns()) { for (auto *column : with->returnItems()->columns()->columns()) { - column->setExpr(ExpressionUtils::rewriteAttr2LabelTagProp(column->expr())); + column->setExpr(ExpressionUtils::rewriteAttr2LabelTagProp( + column->expr(), withClauseCtx.yield->aliasesAvailable)); columns->addColumn(column->clone().release()); } } @@ -819,7 +822,7 @@ Status MatchValidator::checkAlias( auto name = static_cast(labelExpr)->prop(); auto res = getAliasType(aliasesAvailable, name); NG_RETURN_IF_ERROR(res); - if (res.value() != AliasType::kNode) { + if (res.value() == AliasType::kEdge || res.value() == AliasType::kPath) { return Status::SemanticError("The type of `%s' should be tag", name.c_str()); } return Status::OK(); diff --git a/tests/tck/features/expression/Depth.feature b/tests/tck/features/expression/Depth.feature index 71bf5e3498e..045d8b57cd4 100644 --- a/tests/tck/features/expression/Depth.feature +++ b/tests/tck/features/expression/Depth.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: Check Expression Depth Background: diff --git a/tests/tck/features/match/With.feature b/tests/tck/features/match/With.feature index 887d6aa002e..eefe0b9a0c4 100644 --- a/tests/tck/features/match/With.feature +++ b/tests/tck/features/match/With.feature @@ -64,6 +64,38 @@ Feature: With clause Then the result should be, in any order: | count(a) | | 1 | + When executing query: + """ + WITH {a:1, b:{c:3, d:{e:5}}} AS x + RETURN x.b.d.e + """ + Then the result should be, in any order: + | x.b.d.e | + | 5 | + When executing query: + """ + WITH {a:1, b:{c:3, d:{e:5}}} AS x + RETURN x.b.d + """ + Then the result should be, in any order: + | x.b.d | + | {e: 5} | + When executing query: + """ + WITH {a:1, b:{c:3, d:{e:5}}} AS x + RETURN x.b + """ + Then the result should be, in any order: + | x.b | + | {c: 3, d: {e: 5}} | + When executing query: + """ + WITH {a:1, b:{c:3, d:{e:5}}} AS x + RETURN x.c + """ + Then the result should be, in any order: + | x.c | + | UNKNOWN_PROP | Scenario: match with return When executing query: From dd3a73d1ec329012f732551ba4d935e0870b9625 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Mon, 17 Jan 2022 16:43:02 +0800 Subject: [PATCH 2/2] fix collapseProjectRule --- src/common/expression/PropertyExpression.h | 4 ++++ src/graph/visitor/RewriteVisitor.cpp | 12 ++++++++++++ src/graph/visitor/RewriteVisitor.h | 2 +- tests/tck/features/expression/Depth.feature | 1 - tests/tck/features/match/With.feature | 17 +++++++++++++++++ .../optimizer/CollapseProjectRule.feature | 17 +++++++++++++++++ 6 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/common/expression/PropertyExpression.h b/src/common/expression/PropertyExpression.h index 26cf1cee943..a9baf8d47a7 100644 --- a/src/common/expression/PropertyExpression.h +++ b/src/common/expression/PropertyExpression.h @@ -155,6 +155,10 @@ class LabelTagPropertyExpression final : public PropertyExpression { return label_; } + void setLabel(Expression* label) { + label_ = label; + } + private: LabelTagPropertyExpression(ObjectPool* pool, Expression* label = nullptr, diff --git a/src/graph/visitor/RewriteVisitor.cpp b/src/graph/visitor/RewriteVisitor.cpp index 3da631f210b..da207c2dbee 100644 --- a/src/graph/visitor/RewriteVisitor.cpp +++ b/src/graph/visitor/RewriteVisitor.cpp @@ -237,6 +237,18 @@ void RewriteVisitor::visit(PathBuildExpression *expr) { } } +void RewriteVisitor::visit(LabelTagPropertyExpression *expr) { + if (!care(expr->kind())) { + return; + } + auto label = expr->label(); + if (matcher_(label)) { + expr->setLabel(rewriter_(label)); + } else { + label->accept(this); + } +} + void RewriteVisitor::visit(AttributeExpression *expr) { if (!care(expr->kind())) { return; diff --git a/src/graph/visitor/RewriteVisitor.h b/src/graph/visitor/RewriteVisitor.h index 365908ed288..85d30836495 100644 --- a/src/graph/visitor/RewriteVisitor.h +++ b/src/graph/visitor/RewriteVisitor.h @@ -67,12 +67,12 @@ class RewriteVisitor final : public ExprVisitorImpl { void visit(RelationalExpression*) override; void visit(SubscriptExpression*) override; void visit(PathBuildExpression*) override; + void visit(LabelTagPropertyExpression*) override; void visit(SubscriptRangeExpression*) override; void visit(ConstantExpression*) override {} void visit(LabelExpression*) override {} void visit(UUIDExpression*) override {} void visit(LabelAttributeExpression*) override {} - void visit(LabelTagPropertyExpression*) override {} void visit(VariableExpression*) override {} void visit(VersionedVariableExpression*) override {} void visit(TagPropertyExpression*) override {} diff --git a/tests/tck/features/expression/Depth.feature b/tests/tck/features/expression/Depth.feature index 045d8b57cd4..71bf5e3498e 100644 --- a/tests/tck/features/expression/Depth.feature +++ b/tests/tck/features/expression/Depth.feature @@ -1,7 +1,6 @@ # Copyright (c) 2021 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. -@skip Feature: Check Expression Depth Background: diff --git a/tests/tck/features/match/With.feature b/tests/tck/features/match/With.feature index eefe0b9a0c4..5d8252d8956 100644 --- a/tests/tck/features/match/With.feature +++ b/tests/tck/features/match/With.feature @@ -217,6 +217,23 @@ Feature: With clause Then the result should be, in any order, with relax comparison: | avg | max | | 90.0 | 90 | + When executing query: + """ + MATCH (:player {name:"Tim Duncan"})-[e:like]->(dst) + WITH dst AS b + RETURN b.player.age AS age, b + """ + Then the result should be, in any order, with relax comparison: + | age | b | + | 36 | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + | 41 | ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | + When executing query: + """ + MATCH (:player {name:"Tim Duncan"})-[e:like]->(dst) + WITH dst AS b + RETURN b.age AS age, b + """ + Then a SemanticError should be raised at runtime: To get the property of the vertex in `b.age', should use the format `var.tag.prop' @skip Scenario: with match return diff --git a/tests/tck/features/optimizer/CollapseProjectRule.feature b/tests/tck/features/optimizer/CollapseProjectRule.feature index ea6f339a162..46a572f6274 100644 --- a/tests/tck/features/optimizer/CollapseProjectRule.feature +++ b/tests/tck/features/optimizer/CollapseProjectRule.feature @@ -47,3 +47,20 @@ Feature: Collapse Project Rule | 6 | Project | 5 | | | 5 | TagIndexPrefixScan | 0 | | | 0 | Start | | | + When profiling query: + """ + MATCH (:player {name:"Tim Duncan"})-[e:like]->(dst) + WITH dst AS b + RETURN b.player.age AS age + """ + Then the result should be, in any order: + | age | + | 36 | + | 41 | + And the execution plan should be: + | id | name | dependencies | operator info | + | 11 | Project | 4 | | + | 4 | AppendVertices | 3 | | + | 3 | Traverse | 8 | | + | 8 | IndexScan | 2 | {"indexCtx": {"columnHints":{"scanType":"PREFIX","column":"name","beginValue":"\"Tim Duncan\"","endValue":"__EMPTY__","includeBegin":"true","includeEnd":"false"}}} | + | 2 | Start | | |