Skip to content

Commit

Permalink
Push filter down AppendVertices
Browse files Browse the repository at this point in the history
Add tck

small change

fix tck

fix tck

Fix tck
  • Loading branch information
czpmango committed Apr 14, 2023
1 parent f79c3b8 commit e0d12ae
Show file tree
Hide file tree
Showing 19 changed files with 391 additions and 136 deletions.
3 changes: 2 additions & 1 deletion src/common/expression/RelationalExpression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ const Value& RelationalExpression::eval(ExpressionContext& ctx) {
case Kind::kRelREG: {
if (lhs.isBadNull() || rhs.isBadNull()) {
result_ = Value::kNullBadType;
} else if ((!lhs.isNull() && !lhs.isStr()) || (!rhs.isNull() && !rhs.isStr())) {
} else if ((!lhs.isNull() && !lhs.empty() && !lhs.isStr()) ||
(!rhs.isNull() && !lhs.empty() && !rhs.isStr())) {
result_ = Value::kNullBadType;
} else if (lhs.isStr() && rhs.isStr()) {
try {
Expand Down
1 change: 1 addition & 0 deletions src/graph/optimizer/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ nebula_add_library(
rule/RemoveAppendVerticesBelowJoinRule.cpp
rule/EmbedEdgeAllPredIntoTraverseRule.cpp
rule/PushFilterThroughAppendVerticesRule.cpp
rule/PushFilterDownAppendVerticesRule.cpp
)

nebula_add_subdirectory(test)
3 changes: 3 additions & 0 deletions src/graph/optimizer/rule/EmbedEdgeAllPredIntoTraverseRule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ bool isEdgeAllPredicate(const Expression* e,
return false;
}
auto ves = graph::ExpressionUtils::collectAll(pe->filter(), {Expression::Kind::kAttribute});
if (ves.empty()) {
return false;
}
for (const auto& ve : ves) {
auto iv = static_cast<const AttributeExpression*>(ve)->left();

Expand Down
118 changes: 118 additions & 0 deletions src/graph/optimizer/rule/PushFilterDownAppendVerticesRule.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/* Copyright (c) 2023 vesoft inc. All rights reserved.
*
* This source code is licensed under Apache 2.0 License.
*/

#include "graph/optimizer/rule/PushFilterDownAppendVerticesRule.h"

#include "common/expression/Expression.h"
#include "graph/optimizer/OptContext.h"
#include "graph/optimizer/OptGroup.h"
#include "graph/planner/plan/PlanNode.h"
#include "graph/planner/plan/Query.h"
#include "graph/util/ExpressionUtils.h"
#include "graph/visitor/ExtractFilterExprVisitor.h"

using nebula::Expression;
using nebula::graph::AppendVertices;
using nebula::graph::Filter;
using nebula::graph::PlanNode;
using nebula::graph::QueryContext;

namespace nebula {
namespace opt {

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

PushFilterDownAppendVerticesRule::PushFilterDownAppendVerticesRule() {
RuleSet::QueryRules().addRule(this);
}

const Pattern &PushFilterDownAppendVerticesRule::pattern() const {
static Pattern pattern =
Pattern::create(PlanNode::Kind::kFilter, {Pattern::create(PlanNode::Kind::kAppendVertices)});
return pattern;
}

StatusOr<OptRule::TransformResult> PushFilterDownAppendVerticesRule::transform(
OptContext *ctx, const MatchedResult &matched) const {
auto filterGroupNode = matched.node;
auto appendVerticesGroupNode = matched.dependencies.front().node;
auto filter = static_cast<const Filter *>(filterGroupNode->node());
auto appendVertices = static_cast<const AppendVertices *>(appendVerticesGroupNode->node());
auto qctx = ctx->qctx();
auto pool = qctx->objPool();
auto condition = graph::ExpressionUtils::rewriteVertexPropertyFilter(
pool, appendVertices->colNames().back(), filter->condition()->clone());

auto visitor = graph::ExtractFilterExprVisitor::makePushGetVertices(
pool, appendVertices->space(), qctx->schemaMng());
condition->accept(&visitor);
if (!visitor.ok()) {
return TransformResult::noTransform();
}

auto remainedExpr = std::move(visitor).remainedExpr();
OptGroupNode *newFilterGroupNode = nullptr;
PlanNode *newFilter = nullptr;
if (remainedExpr != nullptr) {
auto *found = graph::ExpressionUtils::findAny(remainedExpr, {Expression::Kind::kTagProperty});
if (found != nullptr) { // Some tag property expression don't push down
// TODO(shylock): we could push down a part.
return TransformResult::noTransform();
}
newFilter = Filter::make(qctx, nullptr, remainedExpr);
newFilter->setOutputVar(filter->outputVar());
newFilter->setInputVar(filter->inputVar());
newFilterGroupNode = OptGroupNode::create(ctx, newFilter, filterGroupNode->group());
}

auto newAppendVerticesFilter = condition;
if (condition->isLogicalExpr()) {
const auto *le = static_cast<const LogicalExpression *>(condition);
if (le->operands().size() == 1) {
newAppendVerticesFilter = le->operands().front();
}
}
if (appendVertices->filter() != nullptr) {
auto logicExpr = LogicalExpression::makeAnd(
pool, newAppendVerticesFilter, appendVertices->filter()->clone());
newAppendVerticesFilter = logicExpr;
}

auto newAppendVertices = static_cast<AppendVertices *>(appendVertices->clone());
newAppendVertices->setFilter(newAppendVerticesFilter);

OptGroupNode *newAppendVerticesGroupNode = nullptr;
if (newFilterGroupNode != nullptr) {
// Filter(A&&B)<-AppendVertices(C) => Filter(A)<-AppendVertices(B&&C)
auto newGroup = OptGroup::create(ctx);
newAppendVerticesGroupNode = newGroup->makeGroupNode(newAppendVertices);
newFilterGroupNode->dependsOn(newGroup);
newFilter->setInputVar(newAppendVertices->outputVar());
} else {
// Filter(A)<-AppendVertices(C) => AppendVertices(A&&C)
newAppendVerticesGroupNode =
OptGroupNode::create(ctx, newAppendVertices, filterGroupNode->group());
newAppendVertices->setOutputVar(filter->outputVar());
newAppendVertices->setColNames(appendVertices->colNames());
}

for (auto dep : appendVerticesGroupNode->dependencies()) {
newAppendVerticesGroupNode->dependsOn(dep);
}

TransformResult result;
result.eraseCurr = true;
result.newGroupNodes.emplace_back(newFilterGroupNode ? newFilterGroupNode
: newAppendVerticesGroupNode);
return result;
}

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

} // namespace opt
} // namespace nebula
31 changes: 31 additions & 0 deletions src/graph/optimizer/rule/PushFilterDownAppendVerticesRule.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/* Copyright (c) 2023 vesoft inc. All rights reserved.
*
* This source code is licensed under Apache 2.0 License.
*/

#ifndef GRAPH_OPTIMIZER_RULE_PUSHLIMITDOWNAPPENDVERTICES_H_
#define GRAPH_OPTIMIZER_RULE_PUSHLIMITDOWNAPPENDVERTICES_H_

#include "graph/optimizer/OptRule.h"

namespace nebula {
namespace opt {

class PushFilterDownAppendVerticesRule final : public OptRule {
public:
const Pattern &pattern() const override;

StatusOr<TransformResult> transform(OptContext *ctx, const MatchedResult &matched) const override;

std::string toString() const override;

private:
PushFilterDownAppendVerticesRule();

static std::unique_ptr<OptRule> kInstance;
};

} // namespace opt
} // namespace nebula

#endif // GRAPH_OPTIMIZER_RULE_PUSHFILTERDOWNAPPENDVERTICES_H_
17 changes: 8 additions & 9 deletions tests/tck/features/bugfix/PushFilterDownProject.feature
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ Feature: Test push filter down project
| count(*) |
| 2 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 9 | Aggregate | 12 | |
| 12 | Project | 11 | |
| 11 | Filter | 5 | {"condition": "(($-.n1.player.age-($-.n1.player.age+(($-.n1.player.age%$-.n1.player.age)+($-.n1.player.age+$-.n1.player.age))))<=$-.n1.player.age)"} |
| 5 | AppendVertices | 4 | |
| 4 | Traverse | 2 | |
| 2 | Dedup | 1 | |
| 1 | PassThrough | 3 | |
| 3 | Start | | |
| id | name | dependencies | operator info |
| 9 | Aggregate | 12 | |
| 12 | Project | 5 | |
| 5 | AppendVertices | 4 | {"filter": "((player.age-(player.age+((player.age%player.age)+(player.age+player.age))))<=player.age)"} |
| 4 | Traverse | 2 | |
| 2 | Dedup | 1 | |
| 1 | PassThrough | 3 | |
| 3 | Start | | |
2 changes: 0 additions & 2 deletions tests/tck/features/expression/RelationalExpr.feature
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,6 @@ Feature: RelationalExpression
| ("Steve Nash" :player{age: 45, name: "Steve Nash"}) |
And the execution plan should be:
| id | name | dependencies | operator info |
| 9 | Project | 8 | |
| 8 | Filter | 2 | |
| 2 | AppendVertices | 6 | |
| 6 | IndexScan | 0 | {"indexCtx": {"columnHints":{"scanType":"RANGE"}}} |
| 0 | Start | | |
2 changes: 0 additions & 2 deletions tests/tck/features/expression/UnaryExpr.feature
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ Feature: UnaryExpression
| ("Shaquille O'Neal" :player{age: 47, name: "Shaquille O'Neal"}) |
And the execution plan should be:
| id | name | dependencies | operator info |
| 9 | Project | 8 | |
| 8 | Filter | 2 | |
| 2 | AppendVertices | 6 | |
| 6 | IndexScan | 0 | |
| 0 | Start | | |
5 changes: 0 additions & 5 deletions tests/tck/features/match/Base.feature
Original file line number Diff line number Diff line change
Expand Up @@ -814,11 +814,6 @@ Feature: Basic match
MATCH (v:player{name:"abc"})
"""
Then a SyntaxError should be raised at runtime: syntax error near `)'
When executing query:
"""
MATCH (v:player) where v.player.name RETURN v
"""
Then a ExecutionError should be raised at runtime: Failed to evaluate condition: v.player.name. For boolean conditions, please write in their full forms like <condition> == <true/false> or <condition> IS [NOT] NULL.

Scenario: Scan
When executing query:
Expand Down
17 changes: 5 additions & 12 deletions tests/tck/features/match/IndexSelecting.feature
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ Feature: Index selecting for match statement
And the execution plan should be:
| id | name | dependencies | operator info |
| 6 | Project | 2 | |
| 2 | Filter | 5 | |
| 2 | AppendVertices | 5 | |
| 5 | IndexScan | 0 | {"indexCtx": {"columnHints":{"scanType":"PREFIX"}}} |
| 0 | Start | | |
Expand All @@ -82,7 +81,6 @@ Feature: Index selecting for match statement
And the execution plan should be:
| id | name | dependencies | operator info |
| 6 | Project | 2 | |
| 2 | Filter | 5 | |
| 2 | AppendVertices | 5 | |
| 5 | IndexScan | 0 | {"indexCtx": {"columnHints":{"scanType":"PREFIX"}}} |
| 0 | Start | | |
Expand All @@ -95,8 +93,7 @@ Feature: Index selecting for match statement
| "Tim Duncan" |
And the execution plan should be:
| id | name | dependencies | operator info |
| 9 | Project | 7 | |
| 7 | Filter | 2 | |
| 9 | Project | 2 | |
| 2 | AppendVertices | 6 | |
| 6 | IndexScan | 0 | {"indexCtx": {"columnHints":{"scanType":"PREFIX"}}} |
| 0 | Start | | |
Expand All @@ -109,8 +106,7 @@ Feature: Index selecting for match statement
| "Tim Duncan" |
And the execution plan should be:
| id | name | dependencies | operator info |
| 9 | Project | 7 | |
| 7 | Filter | 2 | |
| 9 | Project | 2 | |
| 2 | AppendVertices | 6 | |
| 6 | IndexScan | 0 | {"indexCtx": {"columnHints":{"scanType":"PREFIX"}}} |
| 0 | Start | | |
Expand All @@ -123,8 +119,7 @@ Feature: Index selecting for match statement
| "Tim Duncan" |
And the execution plan should be:
| id | name | dependencies | operator info |
| 9 | Project | 7 | |
| 7 | Filter | 2 | |
| 9 | Project | 2 | |
| 2 | AppendVertices | 6 | |
| 6 | IndexScan | 0 | {"indexCtx": {"columnHints":{"scanType":"PREFIX"}}} |
| 0 | Start | | |
Expand All @@ -139,8 +134,7 @@ Feature: Index selecting for match statement
| "Tim Duncan" |
And the execution plan should be:
| id | name | dependencies | operator info |
| 9 | Project | 7 | |
| 7 | Filter | 2 | |
| 9 | Project | 2 | |
| 2 | AppendVertices | 6 | |
| 6 | IndexScan | 0 | {"indexCtx": {"columnHints":{"scanType":"RANGE"}}} |
| 0 | Start | | |
Expand All @@ -162,8 +156,7 @@ Feature: Index selecting for match statement
| "Tim Duncan" |
And the execution plan should be:
| id | name | dependencies | operator info |
| 9 | Project | 7 | |
| 7 | Filter | 2 | |
| 9 | Project | 2 | |
| 2 | AppendVertices | 6 | |
| 6 | IndexScan | 0 | {"indexCtx": {"columnHints":{"scanType":"PREFIX"}}} |
| 0 | Start | | |
Expand Down
Loading

0 comments on commit e0d12ae

Please sign in to comment.