Skip to content

Commit

Permalink
Port: remove useless appendVertices below innner/left join (#5014)
Browse files Browse the repository at this point in the history 
* remove useless appendVertices below innner/left join

* fix tck

* fix tck

* fix tck
  • Loading branch information
@jievince
jievince authored Dec 7, 2022
1 parent d9752a0 commit 4014c9b
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 66 deletions.
9 changes: 6 additions & 3 deletions src/common/expression/PredicateExpression.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ std::unordered_map<std::string, PredicateExpression::Type> PredicateExpression::
const Value& PredicateExpression::evalExists(ExpressionContext& ctx) {
DCHECK(collection_->kind() == Expression::Kind::kAttribute ||
collection_->kind() == Expression::Kind::kSubscript ||
collection_->kind() == Expression::Kind::kLabelTagProperty)
collection_->kind() == Expression::Kind::kLabelTagProperty ||
collection_->kind() == Expression::Kind::kTagProperty)
<< "actual kind: " << collection_->kind() << ", toString: " << toString();

if (collection_->kind() == Expression::Kind::kLabelTagProperty) {
result_ = !collection_->eval(ctx).isNull();
if (collection_->kind() == Expression::Kind::kLabelTagProperty ||
collection_->kind() == Expression::Kind::kTagProperty) {
auto v = collection_->eval(ctx);
result_ = (!v.isNull()) && (!v.empty());
return result_;
}

Expand Down
2 changes: 1 addition & 1 deletion src/graph/optimizer/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ nebula_add_library(
rule/PushLimitDownScanEdgesRule.cpp
rule/RemoveProjectDedupBeforeGetDstBySrcRule.cpp
rule/PushFilterDownTraverseRule.cpp
rule/OptimizeLeftJoinPredicateRule.cpp
rule/RemoveAppendVerticesBelowJoinRule.cpp
)

nebula_add_subdirectory(test)
92 changes: 65 additions & 27 deletions ...er/rule/OptimizeLeftJoinPredicateRule.cpp → ...ule/RemoveAppendVerticesBelowJoinRule.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// This source code is licensed under Apache 2.0 License.

#include "graph/optimizer/rule/OptimizeLeftJoinPredicateRule.h"
#include "graph/optimizer/rule/RemoveAppendVerticesBelowJoinRule.h"

#include "graph/optimizer/OptContext.h"
#include "graph/optimizer/OptGroup.h"
Expand All @@ -16,28 +16,28 @@ using nebula::graph::QueryContext;
namespace nebula {
namespace opt {

std::unique_ptr<OptRule> OptimizeLeftJoinPredicateRule::kInstance =
std::unique_ptr<OptimizeLeftJoinPredicateRule>(new OptimizeLeftJoinPredicateRule());
std::unique_ptr<OptRule> RemoveAppendVerticesBelowJoinRule::kInstance =
std::unique_ptr<RemoveAppendVerticesBelowJoinRule>(new RemoveAppendVerticesBelowJoinRule());

OptimizeLeftJoinPredicateRule::OptimizeLeftJoinPredicateRule() {
RemoveAppendVerticesBelowJoinRule::RemoveAppendVerticesBelowJoinRule() {
RuleSet::QueryRules().addRule(this);
}

const Pattern& OptimizeLeftJoinPredicateRule::pattern() const {
const Pattern& RemoveAppendVerticesBelowJoinRule::pattern() const {
static Pattern pattern = Pattern::create(
PlanNode::Kind::kHashLeftJoin,
{PlanNode::Kind::kHashLeftJoin, PlanNode::Kind::kHashInnerJoin},
{Pattern::create(PlanNode::Kind::kUnknown),
Pattern::create(PlanNode::Kind::kProject,
{Pattern::create(PlanNode::Kind::kAppendVertices,
{Pattern::create(PlanNode::Kind::kTraverse)})})});
return pattern;
}

StatusOr<OptRule::TransformResult> OptimizeLeftJoinPredicateRule::transform(
StatusOr<OptRule::TransformResult> RemoveAppendVerticesBelowJoinRule::transform(
OptContext* octx, const MatchedResult& matched) const {
auto* leftJoinGroupNode = matched.node;
auto* leftJoinGroup = leftJoinGroupNode->group();
auto* leftJoin = static_cast<graph::HashLeftJoin*>(leftJoinGroupNode->node());
auto* joinGroupNode = matched.node;
auto* joinGroup = joinGroupNode->group();
auto* join = static_cast<graph::HashJoin*>(joinGroupNode->node());

auto* projectGroupNode = matched.dependencies[1].node;
auto* project = static_cast<graph::Project*>(projectGroupNode->node());
Expand All @@ -53,8 +53,25 @@ StatusOr<OptRule::TransformResult> OptimizeLeftJoinPredicateRule::transform(

auto& tvEdgeAlias = traverse->edgeAlias();

auto& leftExprs = leftJoin->hashKeys();
auto& rightExprs = leftJoin->probeKeys();
auto& leftExprs = join->hashKeys();
auto& rightExprs = join->probeKeys();

std::vector<const Expression*> referAVNodeAliasExprs;
for (auto* rightExpr : rightExprs) {
auto propExprs = graph::ExpressionUtils::collectAll(
rightExpr, {Expression::Kind::kVarProperty, Expression::Kind::kInputProperty});
for (auto* expr : propExprs) {
auto* propExpr = static_cast<const PropertyExpression*>(expr);
if (propExpr->prop() == avNodeAlias) {
referAVNodeAliasExprs.push_back(propExpr);
}
}
}
// If avNodeAlias is referred by more than one expr,
// we cannot remove the append vertices which generate the avNodeAlias column
if (referAVNodeAliasExprs.size() > 1) {
return TransformResult::noTransform();
}

bool found = false;
size_t rightExprIdx = 0;
Expand Down Expand Up @@ -89,11 +106,28 @@ StatusOr<OptRule::TransformResult> OptimizeLeftJoinPredicateRule::transform(
return TransformResult::noTransform();
}

auto columns = project->columns()->columns();
referAVNodeAliasExprs.clear();
for (auto* column : columns) {
auto propExprs = graph::ExpressionUtils::collectAll(
column->expr(), {Expression::Kind::kVarProperty, Expression::Kind::kInputProperty});
for (auto* expr : propExprs) {
auto* propExpr = static_cast<const PropertyExpression*>(expr);
if (propExpr->prop() == avNodeAlias) {
referAVNodeAliasExprs.push_back(propExpr);
}
}
}
// If avNodeAlias is referred by more than one expr,
// we cannot remove the append vertices which generate the avNodeAlias column
if (referAVNodeAliasExprs.size() > 1) {
return TransformResult::noTransform();
}

found = false;
size_t prjIdx = 0;
auto* columns = project->columns();
for (size_t i = 0; i < columns->size(); ++i) {
const auto* col = columns->columns()[i];
for (size_t i = 0; i < columns.size(); ++i) {
const auto* col = columns[i];
if (col->expr()->kind() != Expression::Kind::kInputProperty) {
continue;
}
Expand All @@ -111,7 +145,7 @@ StatusOr<OptRule::TransformResult> OptimizeLeftJoinPredicateRule::transform(

auto* pool = octx->qctx()->objPool();
// Let the new project generate expr `none_direct_dst($-.tvEdgeAlias)`,
// and let the new left join use it as right expr
// and let the new left/inner join use it as right expr
auto* args = ArgumentList::make(pool);
args->addArgument(InputPropertyExpression::make(pool, tvEdgeAlias));
auto* newPrjExpr = FunctionCallExpression::make(pool, "none_direct_dst", args);
Expand All @@ -137,8 +171,12 @@ StatusOr<OptRule::TransformResult> OptimizeLeftJoinPredicateRule::transform(
newRightExprs.emplace_back(rightExprs[i]->clone());
}
}
auto* newLeftJoin =
graph::HashLeftJoin::make(octx->qctx(), nullptr, nullptr, leftExprs, newRightExprs);
graph::HashJoin* newJoin = nullptr;
if (join->kind() == PlanNode::Kind::kHashLeftJoin) {
newJoin = graph::HashLeftJoin::make(octx->qctx(), nullptr, nullptr, leftExprs, newRightExprs);
} else {
newJoin = graph::HashInnerJoin::make(octx->qctx(), nullptr, nullptr, leftExprs, newRightExprs);
}

TransformResult result;
result.eraseAll = true;
Expand All @@ -148,19 +186,19 @@ StatusOr<OptRule::TransformResult> OptimizeLeftJoinPredicateRule::transform(
auto* newProjectGroupNode = newProjectGroup->makeGroupNode(newProject);
newProjectGroupNode->setDeps(appendVerticesGroupNode->dependencies());

newLeftJoin->setLeftVar(leftJoin->leftInputVar());
newLeftJoin->setRightVar(newProject->outputVar());
newLeftJoin->setOutputVar(leftJoin->outputVar());
auto* newLeftJoinGroupNode = OptGroupNode::create(octx, newLeftJoin, leftJoinGroup);
newLeftJoinGroupNode->dependsOn(leftJoinGroupNode->dependencies()[0]);
newLeftJoinGroupNode->dependsOn(newProjectGroup);
newJoin->setLeftVar(join->leftInputVar());
newJoin->setRightVar(newProject->outputVar());
newJoin->setOutputVar(join->outputVar());
auto* newJoinGroupNode = OptGroupNode::create(octx, newJoin, joinGroup);
newJoinGroupNode->dependsOn(joinGroupNode->dependencies()[0]);
newJoinGroupNode->dependsOn(newProjectGroup);

result.newGroupNodes.emplace_back(newLeftJoinGroupNode);
result.newGroupNodes.emplace_back(newJoinGroupNode);
return result;
}

std::string OptimizeLeftJoinPredicateRule::toString() const {
return "OptimizeLeftJoinPredicateRule";
std::string RemoveAppendVerticesBelowJoinRule::toString() const {
return "RemoveAppendVerticesBelowJoinRule";
}

} // namespace opt
Expand Down
8 changes: 4 additions & 4 deletions ...izer/rule/OptimizeLeftJoinPredicateRule.h → .../rule/RemoveAppendVerticesBelowJoinRule.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
namespace nebula {
namespace opt {
// Before:
// HashLeftJoin({id(v)}, {id(v)})
// HashLeft/InnerJoin({id(v)}, {id(v)})
// | |
// ... Project
// | |
Expand All @@ -19,15 +19,15 @@ namespace opt {
// ... Traverse(e)
//
// After:
// HashLeftJoin({id(v)}, {$-.v})
// HashLeft/InnerJoin({id(v)}, {$-.v})
// | |
// ... Project(..., none_direct_dst(e) AS v)
// | |
// AppendVertices(v) Traverse(e)
// |
// ...
//
class OptimizeLeftJoinPredicateRule final : public OptRule {
class RemoveAppendVerticesBelowJoinRule final : public OptRule {
public:
const Pattern &pattern() const override;

Expand All @@ -37,7 +37,7 @@ class OptimizeLeftJoinPredicateRule final : public OptRule {
std::string toString() const override;

private:
OptimizeLeftJoinPredicateRule();
RemoveAppendVerticesBelowJoinRule();

static std::unique_ptr<OptRule> kInstance;
};
Expand Down
19 changes: 9 additions & 10 deletions tests/tck/features/match/MultiQueryParts.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,14 @@ Feature: Multi Query Parts
MATCH (v3:player)-[:like]->(v1)<-[e5]-(v4) where id(v3) == "Tim Duncan" return *
"""
Then the result should be, in any order, with relax comparison:
| v1 | v2 | e3 | v4 | v3 | e5 |
| ("Tony Parker") | ("Tony Parker") | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | ("Tim Duncan") | ("Tim Duncan") | [:teammate "Tim Duncan"->"Tony Parker" @0 {}] |
| ("Tony Parker") | ("Manu Ginobili") | [:like "Manu Ginobili"->"Tim Duncan" @0 {likeness: 90}] | ("Tim Duncan") | ("Tim Duncan") | [:teammate "Tim Duncan"->"Tony Parker" @0 {}] |
| ("Tony Parker") | ("Tony Parker") | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | ("Manu Ginobili") | ("Tim Duncan") | [:teammate "Manu Ginobili"->"Tony Parker" @0 {}] |
| ("Tony Parker") | ("Tony Parker") | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | ("Manu Ginobili") | ("Tim Duncan") | [:teammate "Manu Ginobili"->"Tony Parker" @0 {}] |
| ("Tony Parker") | ("Tim Duncan" ) | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | ("Manu Ginobili") | ("Tim Duncan") | [:teammate "Manu Ginobili"->"Tony Parker" @0 {}] |
| ("Tony Parker") | ("Tim Duncan" ) | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | ("Manu Ginobili") | ("Tim Duncan") | [:teammate "Manu Ginobili"->"Tony Parker" @0 {}] |
| ("Tony Parker") | ("Tony Parker") | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | ("LaMarcus Aldridge") | ("Tim Duncan") | [:like "LaMarcus Aldridge"->"Tony Parker" @0 {}] |
| v1 | v2 | e3 | v4 | v3 | e5 |
| ("Tony Parker") | ("Tony Parker") | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | ("Tim Duncan") | ("Tim Duncan") | [:teammate "Tim Duncan"->"Tony Parker" @0] |
| ("Tony Parker") | ("Manu Ginobili") | [:like "Manu Ginobili"->"Tim Duncan" @0 {likeness: 90}] | ("Tim Duncan") | ("Tim Duncan") | [:teammate "Tim Duncan"->"Tony Parker" @0] |
| ("Tony Parker") | ("Tony Parker") | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | ("Manu Ginobili") | ("Tim Duncan") | [:teammate "Manu Ginobili"->"Tony Parker" @0] |
| ("Tony Parker") | ("Tony Parker") | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | ("Manu Ginobili") | ("Tim Duncan") | [:teammate "Manu Ginobili"->"Tony Parker" @0] |
| ("Tony Parker") | ("Tim Duncan" ) | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | ("Manu Ginobili") | ("Tim Duncan") | [:teammate "Manu Ginobili"->"Tony Parker" @0] |
| ("Tony Parker") | ("Tim Duncan" ) | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | ("Manu Ginobili") | ("Tim Duncan") | [:teammate "Manu Ginobili"->"Tony Parker" @0] |
| ("Tony Parker") | ("Tony Parker") | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | ("LaMarcus Aldridge") | ("Tim Duncan") | [:like "LaMarcus Aldridge"->"Tony Parker" @0] |
# The redudant Project after HashInnerJoin is removed now
And the execution plan should be:
| id | name | dependencies | profiling data | operator info |
Expand All @@ -166,8 +166,7 @@ Feature: Multi Query Parts
| 2 | Dedup | 1 | | |
| 1 | PassThrough | 3 | | |
| 3 | Start | | | |
| 14 | Project | 13 | | |
| 13 | AppendVertices | 12 | | |
| 14 | Project | 12 | | |
| 12 | Traverse | 21 | | |
| 21 | Traverse | 9 | | |
| 9 | Dedup | 8 | | |
Expand Down
27 changes: 12 additions & 15 deletions tests/tck/features/optimizer/PrunePropertiesRule.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,7 @@ Feature: Prune Properties rule
| 11 | Project | 10 | |
| 10 | AppendVertices | 9 | { "props": "[{\"props\":[\"name\", \"age\", \"_tag\"],\"tagId\":9}, {\"props\":[\"name\", \"speciality\", \"_tag\"],\"tagId\":8}, {\"props\":[\"name\", \"_tag\"],\"tagId\":10}]" } |
| 9 | Traverse | 8 | { "vertexProps": "[{\"props\":[\"name\"],\"tagId\":9}]" } |
| 8 | Argument | 0 | |
| 0 | Start | | |
| 8 | Argument | | |
When profiling query:
"""
MATCH (m:player{name:"Tim Duncan"})-[:like]-(n)--()
Expand All @@ -377,19 +376,17 @@ Feature: Prune Properties rule
| scount |
| 270 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 12 | Aggregate | 13 | |
| 13 | HashInnerJoin | 15, 11 | |
| 15 | Project | 4 | |
| 4 | Traverse | 3 | { "vertexProps": "" } |
| 3 | Traverse | 14 | { "vertexProps": "" } |
| 14 | IndexScan | 2 | |
| 2 | Start | | |
| 11 | Project | 10 | |
| 10 | AppendVertices | 9 | { "props": "[{\"props\":[\"_tag\"],\"tagId\":8}, {\"props\":[\"_tag\"],\"tagId\":9}, {\"props\":[\"_tag\"],\"tagId\":10}]" } |
| 9 | Traverse | 8 | { "vertexProps": "" } |
| 8 | Argument | 0 | |
| 0 | Start | | |
| id | name | dependencies | operator info |
| 12 | Aggregate | 13 | |
| 13 | HashInnerJoin | 15, 11 | |
| 15 | Project | 4 | |
| 4 | Traverse | 3 | { "vertexProps": "" } |
| 3 | Traverse | 14 | { "vertexProps": "" } |
| 14 | IndexScan | 2 | |
| 2 | Start | | |
| 11 | Project | 9 | |
| 9 | Traverse | 8 | { "vertexProps": "" } |
| 8 | Argument | | |

@distonly
Scenario: return function
Expand Down
Loading

0 comments on commit 4014c9b

Please sign in to comment.