Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Param MATCH vid seek fix #4024

Merged
merged 12 commits into from
Apr 1, 2022
2 changes: 1 addition & 1 deletion src/graph/planner/match/VertexIdSeek.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ bool VertexIdSeek::matchNode(NodeContext *nodeCtx) {
return false;
}

VidExtractVisitor vidExtractVisitor;
VidExtractVisitor vidExtractVisitor(matchClauseCtx->qctx);
matchClauseCtx->where->filter->accept(&vidExtractVisitor);
auto vidResult = vidExtractVisitor.moveVidPattern();
if (vidResult.spec != VidExtractVisitor::VidPattern::Special::kInUsed) {
Expand Down
51 changes: 34 additions & 17 deletions src/graph/visitor/VidExtractVisitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
*
* This source code is licensed under Apache 2.0 License.
*/

#include "graph/visitor/VidExtractVisitor.h"

#include "graph/context/QueryContext.h"
#include "graph/util/ExpressionUtils.h"

namespace nebula {
Expand Down Expand Up @@ -152,8 +152,9 @@ void VidExtractVisitor::visit(RelationalExpression *expr) {
return;
}
if (expr->left()->kind() != Expression::Kind::kFunctionCall ||
expr->right()->kind() != Expression::Kind::kList ||
!ExpressionUtils::isEvaluableExpr(expr->right())) {
!(expr->right()->kind() == Expression::Kind::kList ||
expr->right()->kind() == Expression::Kind::kAttribute) ||
!ExpressionUtils::isEvaluableExpr(expr->right(), qctx_)) {
vidPattern_ = VidPattern{};
return;
}
Expand All @@ -165,12 +166,11 @@ void VidExtractVisitor::visit(RelationalExpression *expr) {
return;
}

auto *listExpr = static_cast<ListExpression *>(expr->right());
QueryExpressionContext ctx;
vidPattern_ =
VidPattern{VidPattern::Special::kInUsed,
{{fCallExpr->args()->args().front()->toString(),
{VidPattern::Vids::Kind::kIn, listExpr->eval(ctx(nullptr)).getList()}}}};
auto rightListValue = expr->right()->eval(graph::QueryExpressionContext(qctx_->ectx())());
DCHECK(rightListValue.isList());
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{fCallExpr->args()->args().front()->toString(),
{VidPattern::Vids::Kind::kIn, rightListValue.getList()}}}};
} else if (expr->kind() == Expression::Kind::kRelEQ) {
// id(V) == vid
if (expr->left()->kind() == Expression::Kind::kLabelAttribute) {
Expand All @@ -188,7 +188,9 @@ void VidExtractVisitor::visit(RelationalExpression *expr) {
return;
}
if (expr->left()->kind() != Expression::Kind::kFunctionCall ||
expr->right()->kind() != Expression::Kind::kConstant) {
(expr->right()->kind() != Expression::Kind::kConstant &&
expr->right()->kind() != Expression::Kind::kVar &&
expr->right()->kind() != Expression::Kind::kSubscript)) {
vidPattern_ = VidPattern{};
return;
}
Expand All @@ -198,14 +200,29 @@ void VidExtractVisitor::visit(RelationalExpression *expr) {
vidPattern_ = VidPattern{};
return;
}
const auto *constExpr = static_cast<const ConstantExpression *>(expr->right());
if (!SchemaUtil::isValidVid(constExpr->value())) {
vidPattern_ = VidPattern{};
return;
if (expr->right()->kind() == Expression::Kind::kConstant) {
const auto *constExpr = static_cast<const ConstantExpression *>(expr->right());
if (!SchemaUtil::isValidVid(constExpr->value())) {
vidPattern_ = VidPattern{};
return;
}
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{fCallExpr->args()->args().front()->toString(),
{VidPattern::Vids::Kind::kIn, List({constExpr->value()})}}}};
} else if ((expr->right()->kind() == Expression::Kind::kVar ||
expr->right()->kind() == Expression::Kind::kSubscript) &&
ExpressionUtils::isEvaluableExpr(expr->right(), qctx_)) {
auto rValue = expr->right()->eval(graph::QueryExpressionContext(qctx_->ectx())());
if (SchemaUtil::isValidVid(rValue)) {
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{fCallExpr->args()->args().front()->toString(),
{VidPattern::Vids::Kind::kIn, List({rValue})}}}};
return;
} else {
vidPattern_ = VidPattern{};
return;
}
}
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
{{fCallExpr->args()->args().front()->toString(),
{VidPattern::Vids::Kind::kIn, List({constExpr->value()})}}}};
} else {
if (ExpressionUtils::isPropertyExpr(expr->left())) {
vidPattern_ = VidPattern{VidPattern::Special::kInUsed,
Expand Down
4 changes: 3 additions & 1 deletion src/graph/visitor/VidExtractVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <memory>

#include "common/expression/ExprVisitor.h"
#include "graph/context/QueryContext.h"
#include "graph/util/SchemaUtil.h"

namespace nebula {
Expand All @@ -25,7 +26,7 @@ namespace graph {
// other vid make it eval to TRUE!
class VidExtractVisitor final : public ExprVisitor {
public:
VidExtractVisitor() = default;
explicit VidExtractVisitor(const QueryContext *qctx = nullptr) : qctx_(qctx) {}

struct VidPattern {
enum class Special {
Expand Down Expand Up @@ -110,6 +111,7 @@ class VidExtractVisitor final : public ExprVisitor {
void visitBinaryExpr(BinaryExpression *expr);

VidPattern vidPattern_{};
const QueryContext *qctx_{nullptr};
};

std::ostream &operator<<(std::ostream &os, const VidExtractVisitor::VidPattern &vp);
Expand Down
35 changes: 34 additions & 1 deletion tests/tck/features/yield/parameter.feature
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Feature: Parameter

Background:
Given a graph with space named "nba"
Given parameters: {"p1":1,"p2":true,"p3":"Tim Duncan","p4":3.3,"p5":[1,true,3],"p6":{"a":3,"b":false,"c":"Tim Duncan"},"p7":{"a":{"b":{"c":"Tim Duncan","d":[1,2,3,true,"Tim Duncan"]}}}}
Given parameters: {"p1":1,"p2":true,"p3":"Tim Duncan","p4":3.3,"p5":[1,true,3],"p6":{"a":3,"b":false,"c":"Tim Duncan"},"p7":{"a":{"b":{"c":"Tim Duncan","d":[1,2,3,true,"Tim Duncan"]}}},"p8":"Manu Ginobili"}

Scenario: return parameters
When executing query:
Expand All @@ -17,6 +17,39 @@ Feature: Parameter
| 2 | false | "Tim Duncanef" | 4.1 | [1,true,3] | 3 | true |

Scenario: cypher with parameters
When executing query:
"""
MATCH (v) WHERE id(v)==$p3
RETURN id(v) AS v
"""
Then the result should be, in any order:
| v |
| "Tim Duncan" |
When executing query:
"""
MATCH (v) WHERE id(v) IN [$p3,$p8]
RETURN id(v) AS v
"""
Then the result should be, in any order:
| v |
| "Tim Duncan" |
| "Manu Ginobili" |
When executing query:
"""
MATCH (v) WHERE id(v) == $p7.a.b.d[4]
RETURN id(v) AS v
"""
Then the result should be, in any order:
| v |
| "Tim Duncan" |
When executing query:
"""
MATCH (v) WHERE id(v) IN $p7.a.b.d
RETURN id(v) AS v
"""
Then the result should be, in any order:
| v |
| "Tim Duncan" |
When executing query:
"""
MATCH (v:player)-[:like]->(n) WHERE id(v)==$p3 and n.player.age>$p1+29
Expand Down