Skip to content

Commit

Permalink
address comments (#5287)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jievince
jievince authored Jan 30, 2023
1 parent cfb503e commit 0e95568
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 8 deletions.
40 changes: 40 additions & 0 deletions src/common/function/FunctionManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2056,6 +2056,46 @@ FunctionManager::FunctionManager() {
}
};
}
{
// `none_direct_src` always return the srcId of an edge key
// without considering the direction of the edge type.
// The encoding of the edge key is:
// type(1) + partId(3) + srcId(*) + edgeType(4) + edgeRank(8) + dstId(*) + placeHolder(1)
// More information of encoding could be found in `NebulaKeyUtils.h`
auto &attr = functions_["none_direct_src"];
attr.minArity_ = 1;
attr.maxArity_ = 1;
attr.isAlwaysPure_ = true;
attr.body_ = [](const auto &args) -> Value {
switch (args[0].get().type()) {
case Value::Type::NULLVALUE: {
return Value::kNullValue;
}
case Value::Type::EDGE: {
const auto &edge = args[0].get().getEdge();
return edge.src;
}
case Value::Type::VERTEX: {
const auto &v = args[0].get().getVertex();
return v.vid;
}
case Value::Type::LIST: {
const auto &listVal = args[0].get().getList();
auto &firstVal = listVal.values.front();
if (firstVal.isEdge()) {
return firstVal.getEdge().src;
} else if (firstVal.isVertex()) {
return firstVal.getVertex().vid;
} else {
return Value::kNullBadType;
}
}
default: {
return Value::kNullBadType;
}
}
};
}
{
auto &attr = functions_["rank"];
attr.minArity_ = 1;
Expand Down
14 changes: 10 additions & 4 deletions src/graph/util/ExpressionUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ Expression *ExpressionUtils::rewriteAttr2LabelTagProp(
return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter));
}

// rewrite rank(e) to e._rank
Expression *ExpressionUtils::rewriteRankFunc2LabelAttribute(
// rewrite rank(e) to e._rank, none_direct_src(e) to e._src, none_direct_dst(e) to e._dst
Expression *ExpressionUtils::rewriteEdgePropFunc2LabelAttribute(
const Expression *expr, const std::unordered_map<std::string, AliasType> &aliasTypeMap) {
ObjectPool *pool = expr->getObjPool();
auto matcher = [&aliasTypeMap](const Expression *e) -> bool {
Expand All @@ -162,7 +162,9 @@ Expression *ExpressionUtils::rewriteRankFunc2LabelAttribute(
auto *funcExpr = static_cast<const FunctionCallExpression *>(e);
auto funcName = funcExpr->name();
std::transform(funcName.begin(), funcName.end(), funcName.begin(), ::tolower);
if (funcName != "rank") return false;
if (funcName != "rank" && funcName != "none_direct_src" && funcName != "none_direct_dst") {
return false;
}
auto args = funcExpr->args()->args();
if (args.size() != 1) return false;
if (args[0]->kind() != Expression::Kind::kLabel) return false;
Expand All @@ -174,11 +176,15 @@ Expression *ExpressionUtils::rewriteRankFunc2LabelAttribute(
}
return true;
};
static const std::unordered_map<std::string, std::string> func2Attr = {
{"rank", "_rank"}, {"none_direct_src", "_src"}, {"none_direct_dst", "_dst"}};
auto rewriter = [pool](const Expression *e) -> Expression * {
auto funcExpr = static_cast<const FunctionCallExpression *>(e);
auto &funcName = funcExpr->name();
auto &attrName = func2Attr.at(funcName);
auto args = funcExpr->args()->args();
return LabelAttributeExpression::make(
pool, static_cast<LabelExpression *>(args[0]), ConstantExpression::make(pool, "_rank"));
pool, static_cast<LabelExpression *>(args[0]), ConstantExpression::make(pool, attrName));
};

return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter));
Expand Down
2 changes: 1 addition & 1 deletion src/graph/util/ExpressionUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class ExpressionUtils {
const Expression* expr, const std::unordered_map<std::string, AliasType>& aliasTypeMap);

// rewrite rank(e) to e._rank
static Expression* rewriteRankFunc2LabelAttribute(
static Expression* rewriteEdgePropFunc2LabelAttribute(
const Expression* expr, const std::unordered_map<std::string, AliasType>& aliasTypeMap);

// rewrite LabelAttr to tagProp
Expand Down
4 changes: 2 additions & 2 deletions src/graph/validator/MatchValidator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,8 +340,8 @@ Status MatchValidator::validateFilter(const Expression *filter,
// rewrite Attribute to LabelTagProperty
newFilter = ExpressionUtils::rewriteAttr2LabelTagProp(transformRes.value(),
whereClauseCtx.aliasesAvailable);
newFilter =
ExpressionUtils::rewriteRankFunc2LabelAttribute(newFilter, whereClauseCtx.aliasesAvailable);
newFilter = ExpressionUtils::rewriteEdgePropFunc2LabelAttribute(newFilter,
whereClauseCtx.aliasesAvailable);

whereClauseCtx.filter = newFilter;

Expand Down
3 changes: 2 additions & 1 deletion src/graph/visitor/DeduceTypeVisitor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,8 @@ void DeduceTypeVisitor::visit(FunctionCallExpression *expr) {
}

auto funName = expr->name();
if (funName == "id" || funName == "src" || funName == "dst") {
if (funName == "id" || funName == "src" || funName == "dst" || funName == "none_direct_src" ||
funName == "none_direct_dst") {
type_ = vidType_;
return;
}
Expand Down
102 changes: 102 additions & 0 deletions tests/tck/features/optimizer/PushFilterDownTraverseRule.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,86 @@ Feature: Push Filter down Traverse rule
| 2 | Dedup | 9 | |
| 9 | IndexScan | 3 | |
| 3 | Start | | |
When profiling query:
"""
MATCH (v:player)-[e:like]->(v2) WHERE rank(e) == 0 RETURN COUNT(*)
"""
Then the result should be, in any order:
| COUNT(*) |
| 81 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 7 | Aggregate | 6 | |
| 6 | Project | 5 | |
| 5 | AppendVertices | 10 | |
| 10 | Traverse | 2 | {"filter": "(like._rank==0)"} |
| 2 | Dedup | 9 | |
| 9 | IndexScan | 3 | |
| 3 | Start | | |
When profiling query:
"""
MATCH (v:player)-[e:like]->(v2) WHERE none_direct_dst(e) IN ["Tony Parker", "Tim Duncan"] RETURN e.likeness, v2.player.age
"""
Then the result should be, in any order:
| e.likeness | v2.player.age |
| 80 | 36 |
| 99 | 36 |
| 75 | 36 |
| 50 | 36 |
| 95 | 36 |
| 80 | 42 |
| 80 | 42 |
| 70 | 42 |
| 99 | 42 |
| 75 | 42 |
| 90 | 42 |
| 55 | 42 |
| 80 | 42 |
| 80 | 42 |
| 95 | 42 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 6 | Project | 5 | |
| 5 | AppendVertices | 10 | |
| 10 | Traverse | 9 | {"filter": "((like._dst==\"Tony Parker\") OR (like._dst==\"Tim Duncan\"))"} |
| 9 | IndexScan | 3 | |
| 3 | Start | | |
# The following two match statements is equivalent, so the minus of them should be empty.
When executing query:
"""
MATCH (v:player)-[e:like]->(v2) WHERE none_direct_dst(e) IN ["Tony Parker", "Tim Duncan"] RETURN *
MINUS
MATCH (v:player)-[e:like]->(v2) WHERE id(v2) IN ["Tony Parker", "Tim Duncan"] RETURN *
"""
Then the result should be, in any order:
| v | e | v2 |
When profiling query:
"""
MATCH (v:player)-[e:like]->(v2) WHERE none_direct_src(e) IN ["Tony Parker", "Tim Duncan"] RETURN e.likeness, v2.player.age
"""
Then the result should be, in any order:
| e.likeness | v2.player.age |
| 90 | 33 |
| 95 | 41 |
| 95 | 42 |
| 95 | 41 |
| 95 | 36 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 6 | Project | 5 | |
| 5 | AppendVertices | 10 | |
| 10 | Traverse | 9 | {"filter": "((like._src==\"Tony Parker\") OR (like._src==\"Tim Duncan\"))"} |
| 9 | IndexScan | 3 | |
| 3 | Start | | |
# The following two match statements is equivalent, so the minus of them should be empty.
When executing query:
"""
MATCH (v:player)-[e:like]->(v2) WHERE none_direct_src(e) IN ["Tony Parker", "Tim Duncan"] RETURN *
MINUS
MATCH (v:player)-[e:like]->(v2) WHERE id(v) IN ["Tony Parker", "Tim Duncan"] RETURN *
"""
Then the result should be, in any order:
| v | e | v2 |
When profiling query:
"""
MATCH (person:player)-[:like*1..2]-(friend:player)-[served:serve]->(friendTeam:team)
Expand Down Expand Up @@ -94,3 +174,25 @@ Feature: Push Filter down Traverse rule
| 12 | Traverse | 8 | {"filter": "((like.likeness+100)!=199)"} |
| 8 | IndexScan | 2 | |
| 2 | Start | | |
When profiling query:
"""
MATCH (v:player)-[e:like]->(v2)
WHERE v.player.age != 35 and (e.likeness + 100) != 199 and none_direct_dst(e) in ["Tony Parker", "Tim Duncan", "Yao Ming"]
RETURN e.likeness, v2.player.age as age
ORDER BY age
LIMIT 3
"""
Then the result should be, in any order:
| e.likeness | age |
| 80 | 36 |
| 75 | 36 |
| 50 | 36 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 11 | TopN | 10 | |
| 10 | Project | 9 | |
| 9 | Filter | 4 | {"condition": "($-.v.player.age!=35)" } |
| 4 | AppendVertices | 12 | |
| 12 | Traverse | 8 | {"filter": "(((like.likeness+100)!=199) AND ((like._dst==\"Tony Parker\") OR (like._dst==\"Tim Duncan\") OR (like._dst==\"Yao Ming\")))"} |
| 8 | IndexScan | 2 | |
| 2 | Start | | |

0 comments on commit 0e95568

Please sign in to comment.