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

flush vscode ...
  • Loading branch information
czpmango committed Apr 14, 2023
1 parent f79c3b8 commit 6b3083e
Show file tree
Hide file tree
Showing 20 changed files with 405 additions and 151 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

0 comments on commit 6b3083e

Please sign in to comment.