Skip to content

Commit

Permalink
rewrite param in subgraph & path (#5500)
Browse files Browse the repository at this point in the history
* check param in subgraph

* rewrite param in path

---------

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>
  • Loading branch information
nevermore3 and Sophie-Xie authored Apr 17, 2023
1 parent 46710df commit adb13b2
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 11 deletions.
21 changes: 17 additions & 4 deletions src/graph/validator/FindPathValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,28 @@ Status FindPathValidator::validateWhere(WhereClause* where) {
return Status::OK();
}
// Not Support $-、$var、$$.tag.prop、$^.tag.prop、agg
auto expr = where->filter();
if (ExpressionUtils::findAny(expr,
auto filterExpr = where->filter();
if (ExpressionUtils::findAny(filterExpr,
{Expression::Kind::kSrcProperty,
Expression::Kind::kDstProperty,
Expression::Kind::kVarProperty,
Expression::Kind::kInputProperty})) {
return Status::SemanticError("Not support `%s' in where sentence.", expr->toString().c_str());
return Status::SemanticError("Not support `%s' in where sentence.",
filterExpr->toString().c_str());
}
where->setFilter(ExpressionUtils::rewriteLabelAttr2EdgeProp(expr));

auto undefinedParams = graph::ExpressionUtils::ExtractInnerVars(filterExpr, qctx_);
if (!undefinedParams.empty()) {
return Status::SemanticError(
"Undefined parameters: " +
std::accumulate(++undefinedParams.begin(),
undefinedParams.end(),
*undefinedParams.begin(),
[](auto& lhs, auto& rhs) { return lhs + ", " + rhs; }));
}
auto* newFilter = graph::ExpressionUtils::rewriteParameter(filterExpr, qctx_);

where->setFilter(ExpressionUtils::rewriteLabelAttr2EdgeProp(newFilter));
auto filter = where->filter();

auto typeStatus = deduceExprType(filter);
Expand Down
24 changes: 18 additions & 6 deletions src/graph/validator/GetSubgraphValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,29 @@ Status GetSubgraphValidator::validateWhere(WhereClause* where) {
if (where == nullptr) {
return Status::OK();
}
auto* expr = where->filter();
if (ExpressionUtils::findAny(expr,
auto* filterExpr = where->filter();
if (ExpressionUtils::findAny(filterExpr,
{Expression::Kind::kAggregate,
Expression::Kind::kSrcProperty,
Expression::Kind::kVarProperty,
Expression::Kind::kInputProperty,
Expression::Kind::kLogicalOr})) {
return Status::SemanticError("Not support `%s' in where sentence.", expr->toString().c_str());
return Status::SemanticError("Not support `%s' in where sentence.",
filterExpr->toString().c_str());
}

where->setFilter(ExpressionUtils::rewriteLabelAttr2EdgeProp(expr));
auto undefinedParams = graph::ExpressionUtils::ExtractInnerVars(filterExpr, qctx_);
if (!undefinedParams.empty()) {
return Status::SemanticError(
"Undefined parameters: " +
std::accumulate(++undefinedParams.begin(),
undefinedParams.end(),
*undefinedParams.begin(),
[](auto& lhs, auto& rhs) { return lhs + ", " + rhs; }));
}
auto* newFilter = graph::ExpressionUtils::rewriteParameter(filterExpr, qctx_);

where->setFilter(ExpressionUtils::rewriteLabelAttr2EdgeProp(newFilter));
auto filter = where->filter();
auto typeStatus = deduceExprType(filter);
NG_RETURN_IF_ERROR(typeStatus);
Expand Down Expand Up @@ -169,11 +181,11 @@ Status GetSubgraphValidator::validateWhere(WhereClause* where) {
}

auto condition = filter->clone();
if (ExpressionUtils::findAny(expr, {Expression::Kind::kDstProperty})) {
if (ExpressionUtils::findAny(filter, {Expression::Kind::kDstProperty})) {
auto visitor = ExtractFilterExprVisitor::makePushGetVertices(qctx_->objPool());
filter->accept(&visitor);
if (!visitor.ok()) {
return Status::SemanticError("Push target vertices filter error: " + expr->toString());
return Status::SemanticError("Push target vertices filter error: " + filter->toString());
}
subgraphCtx_->edgeFilter = visitor.remainedExpr();
auto tagFilter = visitor.extractedExpr() ? visitor.extractedExpr() : filter;
Expand Down
39 changes: 38 additions & 1 deletion tests/tck/features/yield/parameter.feature
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Feature: Parameter
Background:
Given an empty graph
And load "nba" csv data to a new space
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", "p9":["Tim Duncan","Tony Parker"]}
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", "p9":["Tim Duncan","Tony Parker"], "p10":90}

Scenario: [param-test-001] without define param
When executing query:
Expand Down Expand Up @@ -269,6 +269,21 @@ Feature: Parameter
MATCH (v:player) where v.player.age < $unknown_distance RETURN v
"""
Then a SemanticError should be raised at runtime: Undefined parameters: unknown_distance
When executing query:
"""
GET SUBGRAPH FROM 'Tim Duncan' WHERE like.likeness < $unknown_distance YIELD edges as e
"""
Then a SemanticError should be raised at runtime: Undefined parameters: unknown_distance
When executing query:
"""
FIND ALL PATH FROM 'Tim Duncan' TO 'Tony Parker' OVER like WHERE like.likeness > $unknown_distance YIELD path as p
"""
Then a SemanticError should be raised at runtime: Undefined parameters: unknown_distance
When executing query:
"""
FIND SHORTEST PATH FROM 'Tim Duncan' TO 'Tony Parker' OVER like WHERE like.likeness > $unknown_distance YIELD path as p
"""
Then a SemanticError should be raised at runtime: Undefined parameters: unknown_distance
When executing query:
"""
MATCH (v:player) RETURN v LIMIT $p6
Expand Down Expand Up @@ -345,6 +360,28 @@ Feature: Parameter
| v |
| BAD_TYPE |
| BAD_TYPE |
When executing query:
"""
GET SUBGRAPH FROM 'Tim Duncan' WHERE like.likeness > $p10 YIELD edges AS e
"""
Then the result should be, in any order:
| e |
| [[:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}], [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}], [:like "Dejounte Murray"->"Tim Duncan" @0 {likeness: 99}], [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}]] |
| [[:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}], [:like "Dejounte Murray"->"Manu Ginobili" @0 {likeness: 99}], [:like "Dejounte Murray"->"Tony Parker" @0 {likeness: 99}]] |
When executing query:
"""
FIND ALL PATH FROM 'Tim Duncan' TO 'Tony Parker' OVER like WHERE like.likeness > $p10-1 YIELD path AS p
"""
Then the result should be, in any order, with relax comparison:
| p |
| <("Tim Duncan")-[:like@0 {likeness: 95}]->("Tony Parker")> |
| <("Tim Duncan")-[:like@0 {likeness: 95}]->("Manu Ginobili")-[:like@0 {likeness: 90}]->("Tim Duncan")-[:like@0 {likeness: 95}]->("Tony Parker")> |
When executing query:
"""
FIND ALL PATH FROM 'Tim Duncan' TO 'Tony Parker' OVER like WHERE like.likeness > $p5[10] YIELD path AS p
"""
Then the result should be, in any order:
| p |
Scenario: [param-test-013] DML
Given an empty graph
Expand Down

0 comments on commit adb13b2

Please sign in to comment.