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.

Sign up for GitHub

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

Eliminate vid predication Filter. #4249

Merged
merged 5 commits into from
May 12, 2022
Merged

Eliminate vid predication Filter. #4249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6323462
Eliminate vid predication Filter.
Shylock-Hg May 11, 2022
5101479
Merge branch 'master' into optimize/eliminate-vid-predication-Filter
Shylock-Hg May 11, 2022
806b45a
Fix cases.
Shylock-Hg May 11, 2022
4e66a65
Merge branch 'optimize/eliminate-vid-predication-Filter' of github.co…
Shylock-Hg May 11, 2022
a0bb8d0
Remove unused code.
Shylock-Hg May 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/graph/context/ast/CypherAstContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,11 @@ struct PatternContext {
};

struct NodeContext final : PatternContext {
NodeContext(QueryContext* q, Expression* b, GraphSpaceID g, NodeInfo* i)
: PatternContext(PatternKind::kNode), qctx(q), bindFilter(b), spaceId(g), info(i) {}
NodeContext(QueryContext* q, WhereClauseContext* b, GraphSpaceID g, NodeInfo* i)
: PatternContext(PatternKind::kNode), qctx(q), bindWhereClause(b), spaceId(g), info(i) {}

QueryContext* qctx;
Expression* bindFilter;
WhereClauseContext* bindWhereClause;
GraphSpaceID spaceId;
NodeInfo* info{nullptr};
std::unordered_set<std::string>* nodeAliasesAvailable;
Expand All @@ -221,11 +221,11 @@ struct NodeContext final : PatternContext {
};

struct EdgeContext final : PatternContext {
EdgeContext(QueryContext* q, Expression* b, GraphSpaceID g, EdgeInfo* i)
: PatternContext(PatternKind::kEdge), qctx(q), bindFilter(b), spaceId(g), info(i) {}
EdgeContext(QueryContext* q, WhereClauseContext* b, GraphSpaceID g, EdgeInfo* i)
: PatternContext(PatternKind::kEdge), qctx(q), bindWhereClause(b), spaceId(g), info(i) {}

QueryContext* qctx;
Expression* bindFilter;
WhereClauseContext* bindWhereClause;
GraphSpaceID spaceId;
EdgeInfo* info{nullptr};

Expand Down
4 changes: 2 additions & 2 deletions src/graph/planner/match/LabelIndexSeek.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ StatusOr<SubPlan> LabelIndexSeek::transformNode(NodeContext* nodeCtx) {
// and it should be converted to an `optRule` after the match validator is
// refactored
auto* pool = nodeCtx->qctx->objPool();
if (nodeCtx->bindFilter != nullptr) {
auto* filter = nodeCtx->bindFilter;
if (nodeCtx->bindWhereClause != nullptr && nodeCtx->bindWhereClause->filter != nullptr) {
auto* filter = nodeCtx->bindWhereClause->filter;
const auto& nodeAlias = nodeCtx->info->alias;
const auto& schemaName = nodeCtx->scanInfo.schemaNames.back();

Expand Down
3 changes: 1 addition & 2 deletions src/graph/planner/match/MatchClausePlanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@ StatusOr<SubPlan> MatchClausePlanner::transform(CypherClauseContextBase* clauseC
for (auto iter = pathInfos.begin(); iter < pathInfos.end(); ++iter) {
auto& nodeInfos = iter->nodeInfos;
MatchPathPlanner matchPathPlanner;
auto bindFilter = matchClauseCtx->where ? matchClauseCtx->where->filter : nullptr;
auto result = matchPathPlanner.transform(matchClauseCtx->qctx,
matchClauseCtx->space.id,
bindFilter,
matchClauseCtx->where.get(),
matchClauseCtx->aliasesAvailable,
nodeAliasesSeen,
*iter);
Expand Down
10 changes: 5 additions & 5 deletions src/graph/planner/match/MatchPathPlanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ static Expression* nodeId(ObjectPool* pool, const NodeInfo& node) {
StatusOr<SubPlan> MatchPathPlanner::transform(
QueryContext* qctx,
GraphSpaceID spaceId,
Expression* bindFilter,
WhereClauseContext* bindWhere,
const std::unordered_map<std::string, AliasType>& aliasesAvailable,
std::unordered_set<std::string> nodeAliasesSeen,
Path& path) {
Expand All @@ -135,7 +135,7 @@ StatusOr<SubPlan> MatchPathPlanner::transform(
edgeInfos,
qctx,
spaceId,
bindFilter,
bindWhere,
aliasesAvailable,
nodeAliasesSeen,
startFromEdge,
Expand All @@ -158,7 +158,7 @@ Status MatchPathPlanner::findStarts(
std::vector<EdgeInfo>& edgeInfos,
QueryContext* qctx,
GraphSpaceID spaceId,
Expression* bindFilter,
WhereClauseContext* bindWhereClause,
const std::unordered_map<std::string, AliasType>& aliasesAvailable,
std::unordered_set<std::string> nodeAliasesSeen,
bool& startFromEdge,
Expand All @@ -178,7 +178,7 @@ Status MatchPathPlanner::findStarts(
// Find the start plan node
for (auto& finder : startVidFinders) {
for (size_t i = 0; i < nodeInfos.size() && !foundStart; ++i) {
auto nodeCtx = NodeContext(qctx, bindFilter, spaceId, &nodeInfos[i]);
auto nodeCtx = NodeContext(qctx, bindWhereClause, spaceId, &nodeInfos[i]);
nodeCtx.nodeAliasesAvailable = &allNodeAliasesAvailable;
auto nodeFinder = finder();
if (nodeFinder->match(&nodeCtx)) {
Expand All @@ -197,7 +197,7 @@ Status MatchPathPlanner::findStarts(
}

if (i != nodeInfos.size() - 1) {
auto edgeCtx = EdgeContext(qctx, bindFilter, spaceId, &edgeInfos[i]);
auto edgeCtx = EdgeContext(qctx, bindWhereClause, spaceId, &edgeInfos[i]);
auto edgeFinder = finder();
if (edgeFinder->match(&edgeCtx)) {
auto plan = edgeFinder->transform(&edgeCtx);
Expand Down
4 changes: 2 additions & 2 deletions src/graph/planner/match/MatchPathPlanner.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class MatchPathPlanner final {

StatusOr<SubPlan> transform(QueryContext* qctx,
GraphSpaceID spaceId,
Expression* bindFilter,
WhereClauseContext* bindWhereClause,
const std::unordered_map<std::string, AliasType>& aliasesAvailable,
std::unordered_set<std::string> nodeAliasesSeen,
Path& path);
Expand All @@ -25,7 +25,7 @@ class MatchPathPlanner final {
std::vector<EdgeInfo>& edgeInfos,
QueryContext* qctx,
GraphSpaceID spaceId,
Expression* bindFilter,
WhereClauseContext* bindWhereClause,
const std::unordered_map<std::string, AliasType>& aliasesAvailable,
std::unordered_set<std::string> nodeAliases,
bool& startFromEdge,
Expand Down
8 changes: 4 additions & 4 deletions src/graph/planner/match/PropIndexSeek.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ bool PropIndexSeek::matchEdge(EdgeContext* edgeCtx) {
}

Expression* filter = nullptr;
if (edgeCtx->bindFilter != nullptr) {
if (edgeCtx->bindWhereClause != nullptr && edgeCtx->bindWhereClause->filter != nullptr) {
filter = MatchSolver::makeIndexFilter(
edge.types.back(), edge.alias, edgeCtx->bindFilter, edgeCtx->qctx, true);
edge.types.back(), edge.alias, edgeCtx->bindWhereClause->filter, edgeCtx->qctx, true);
}
if (filter == nullptr) {
if (edge.props != nullptr && !edge.props->items().empty()) {
Expand Down Expand Up @@ -124,9 +124,9 @@ bool PropIndexSeek::matchNode(NodeContext* nodeCtx) {

Expression* filterInWhere = nullptr;
Expression* filterInPattern = nullptr;
if (nodeCtx->bindFilter != nullptr) {
if (nodeCtx->bindWhereClause != nullptr && nodeCtx->bindWhereClause->filter != nullptr) {
filterInWhere = MatchSolver::makeIndexFilter(
node.labels.back(), node.alias, nodeCtx->bindFilter, nodeCtx->qctx);
node.labels.back(), node.alias, nodeCtx->bindWhereClause->filter, nodeCtx->qctx);
}
if (!node.labelProps.empty()) {
auto props = node.labelProps.back();
Expand Down
3 changes: 3 additions & 0 deletions src/graph/planner/match/SegmentsConnector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ SubPlan SegmentsConnector::cartesianProduct(QueryContext* qctx,
}

SubPlan SegmentsConnector::addInput(const SubPlan& left, const SubPlan& right, bool copyColNames) {
if (left.root == nullptr) {
return right;
}
SubPlan newPlan = left;
DCHECK(left.root->isSingleInput());
if (left.tail->isSingleInput()) {
Expand Down
8 changes: 6 additions & 2 deletions src/graph/planner/match/VertexIdSeek.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ StatusOr<SubPlan> VertexIdSeek::transformEdge(EdgeContext *edgeCtx) {
bool VertexIdSeek::matchNode(NodeContext *nodeCtx) {
auto &node = *nodeCtx->info;
// auto *matchClauseCtx = nodeCtx->matchClauseCtx;
if (nodeCtx->bindFilter == nullptr) {
if (nodeCtx->bindWhereClause == nullptr || nodeCtx->bindWhereClause->filter == nullptr) {
return false;
}

Expand All @@ -37,7 +37,7 @@ bool VertexIdSeek::matchNode(NodeContext *nodeCtx) {
}

VidExtractVisitor vidExtractVisitor(nodeCtx->qctx);
nodeCtx->bindFilter->accept(&vidExtractVisitor);
nodeCtx->bindWhereClause->filter->accept(&vidExtractVisitor);
auto vidResult = vidExtractVisitor.moveVidPattern();
if (vidResult.spec != VidExtractVisitor::VidPattern::Special::kInUsed) {
return false;
Expand All @@ -47,6 +47,10 @@ bool VertexIdSeek::matchNode(NodeContext *nodeCtx) {
if (nodeVid.second.kind == VidExtractVisitor::VidPattern::Vids::Kind::kIn) {
if (nodeVid.first == node.alias) {
nodeCtx->ids = std::move(nodeVid.second.vids);
// Eliminate Filter when it's complete predication about Vertex Id
if (ExpressionUtils::isVidPredication(nodeCtx->bindWhereClause->filter)) {
nodeCtx->bindWhereClause->filter = nullptr;
}
return true;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/graph/planner/match/WhereClausePlanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ StatusOr<SubPlan> WhereClausePlanner::transform(CypherClauseContextBase* ctx) {
}

auto* wctx = static_cast<WhereClauseContext*>(ctx);
SubPlan wherePlan;
if (wctx->filter) {
SubPlan wherePlan;
auto* newFilter = MatchSolver::doRewrite(wctx->qctx, wctx->aliasesAvailable, wctx->filter);
wherePlan.root = Filter::make(wctx->qctx, nullptr, newFilter, needStableFilter_);
wherePlan.tail = wherePlan.root;
Expand All @@ -46,7 +46,7 @@ StatusOr<SubPlan> WhereClausePlanner::transform(CypherClauseContextBase* ctx) {
return wherePlan;
}

return Status::OK();
return wherePlan;
}
} // namespace graph
} // namespace nebula
30 changes: 29 additions & 1 deletion src/graph/util/ExpressionUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,35 @@ bool ExpressionUtils::checkExprDepth(const Expression *expr) {
}
}
return true;
} // namespace graph
}

/*static*/ bool ExpressionUtils::isVidPredication(const Expression *expr) {
if (DCHECK_NOTNULL(expr)->kind() != Expression::Kind::kRelIn &&
expr->kind() != Expression::Kind::kRelEQ) {
return false;
}
const auto *relExpr = static_cast<const RelationalExpression *>(expr);
if (relExpr->left()->kind() != Expression::Kind::kFunctionCall) {
return false;
}
const auto *fCallExpr = static_cast<const FunctionCallExpression *>(relExpr->left());
if (fCallExpr->name() != "id" || fCallExpr->args()->numArgs() != 1 ||
fCallExpr->args()->args().front()->kind() != Expression::Kind::kLabel) {
return false;
}
if (expr->kind() == Expression::Kind::kRelIn) {
// id(V) IN [List]
if (relExpr->right()->kind() != Expression::Kind::kList) {
return false;
}
} else if (expr->kind() == Expression::Kind::kRelEQ) {
// id(V) = Value
if (relExpr->right()->kind() != Expression::Kind::kConstant) {
return false;
}
}
return true;
}

} // namespace graph
} // namespace nebula
4 changes: 4 additions & 0 deletions src/graph/util/ExpressionUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ class ExpressionUtils {
// Checks if the depth of the expression exceeds the maximum
// This is used in the parser to prevent OOM due to highly nested expression
static bool checkExprDepth(const Expression* expr);

// Whether the whole expression is vertex id predication
// e.g. id(v) == 1, id(v) IN [...]
static bool isVidPredication(const Expression* expr);
};

} // namespace graph
Expand Down
2 changes: 0 additions & 2 deletions src/graph/validator/test/QueryValidatorTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,6 @@ TEST_F(QueryValidatorTest, TestMatch) {
"RETURN type(r) AS Type, v2.person.name AS Name";
std::vector<PlanNode::Kind> expected = {
PK::kProject,
PK::kFilter,
PK::kProject,
PK::kAppendVertices,
PK::kTraverse,
Expand All @@ -1245,7 +1244,6 @@ TEST_F(QueryValidatorTest, TestMatch) {
"RETURN v1, v2";
std::vector<PlanNode::Kind> expected = {
PK::kProject,
PK::kFilter,
PK::kProject,
PK::kAppendVertices,
PK::kTraverse,
Expand Down
34 changes: 34 additions & 0 deletions tests/tck/features/match/SeekById.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,3 +365,37 @@ Feature: Match seek by id
| 'Aron Baynes' |
| 'Blake Griffin' |
| 'Grant Hill' |

Scenario: check plan
When profiling query:
"""
MATCH (v)
WHERE id(v) == 'Paul Gasol'
RETURN v.player.name AS Name, v.player.age AS Age
"""
Then the result should be, in any order:
| Name | Age |
| 'Paul Gasol' | 38 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 12 | Project | 2 | |
| 2 | AppendVertices | 16 | |
| 16 | Dedup | 7 | |
| 7 | PassThrough | 0 | |
| 0 | Start | | |
When profiling query:
"""
MATCH (v)
WHERE id(v) IN ['Paul Gasol']
RETURN v.player.name AS Name, v.player.age AS Age
"""
Then the result should be, in any order:
| Name | Age |
| 'Paul Gasol' | 38 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 12 | Project | 2 | |
| 2 | AppendVertices | 16 | |
| 16 | Dedup | 7 | |
| 7 | PassThrough | 0 | |
| 0 | Start | | |
34 changes: 34 additions & 0 deletions tests/tck/features/match/SeekById.intVid.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,3 +405,37 @@ Feature: Match seek by id
Then the result should be, in any order:
| v |
| (-100 :player{age: 32, name: "Tim"}) |

Scenario: check plan
When profiling query:
"""
MATCH (v)
WHERE id(v) == hash('Paul Gasol')
RETURN v.player.name AS Name, v.player.age AS Age
"""
Then the result should be, in any order:
| Name | Age |
| 'Paul Gasol' | 38 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 12 | Project | 2 | |
| 2 | AppendVertices | 16 | |
| 16 | Dedup | 7 | |
| 7 | PassThrough | 0 | |
| 0 | Start | | |
When profiling query:
"""
MATCH (v)
WHERE id(v) IN [hash('Paul Gasol')]
RETURN v.player.name AS Name, v.player.age AS Age
"""
Then the result should be, in any order:
| Name | Age |
| 'Paul Gasol' | 38 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 12 | Project | 2 | |
| 2 | AppendVertices | 16 | |
| 16 | Dedup | 7 | |
| 7 | PassThrough | 0 | |
| 0 | Start | | |
Loading