Skip to content

Commit

Permalink
fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@jievince
jievince committed Dec 6, 2022
1 parent 96f1c9d commit db893c8
Show file tree
Hide file tree
Showing 9 changed files with 181 additions and 101 deletions.
3 changes: 2 additions & 1 deletion src/common/expression/PredicateExpression.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ 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)
<< "actual kind: " << collection_->kind() << ", toString: " << toString();

if (collection_->kind() == Expression::Kind::kLabelTagProperty) {
result_ = !collection_->eval(ctx).isNull();
Expand Down
41 changes: 18 additions & 23 deletions src/graph/optimizer/rule/PushFilterDownTraverseRule.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

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

#include "common/expression/ConstantExpression.h"
#include "common/expression/Expression.h"
#include "graph/optimizer/OptContext.h"
#include "graph/optimizer/OptGroup.h"
Expand Down Expand Up @@ -45,9 +46,7 @@ bool PushFilterDownTraverseRule::match(OptContext* ctx, const MatchedResult& mat
PlanNode::Kind::kTraverse);
auto traverse =
static_cast<const Traverse*>(matched.dependencies[0].dependencies[0].node->node());
auto step = traverse->stepRange();
// step == nullptr means one step.
return step == nullptr;
return traverse->isOneStep();
}

StatusOr<OptRule::TransformResult> PushFilterDownTraverseRule::transform(
Expand All @@ -59,26 +58,26 @@ StatusOr<OptRule::TransformResult> PushFilterDownTraverseRule::transform(

auto* avGroupNode = matched.dependencies[0].node;
auto* av = static_cast<graph::AppendVertices*>(avGroupNode->node());
auto& avColNames = av->colNames();
auto& nodeAlias = avColNames.back();
UNUSED(nodeAlias);

auto* tvGroupNode = matched.dependencies[0].dependencies[0].node;
auto* tv = static_cast<graph::Traverse*>(tvGroupNode->node());
auto& tvColNames = tv->colNames();
auto& edgeAlias = tvColNames.back();
auto& edgeAlias = tv->edgeAlias();

auto qctx = ctx->qctx();
auto pool = qctx->objPool();

// Pick the expr looks like `$-.e[0].likeness
auto picker = [&edgeAlias](const Expression* e) -> bool {
auto varProps = graph::ExpressionUtils::collectAll(e,
{Expression::Kind::kTagProperty,
Expression::Kind::kEdgeProperty,
Expression::Kind::kInputProperty,
Expression::Kind::kVarProperty,
Expression::Kind::kDstProperty,
Expression::Kind::kSrcProperty});
// TODO(jie): Handle the strange exists expr. e.g. exists(e.likeness)
auto exprs = graph::ExpressionUtils::collectAll(e, {Expression::Kind::kPredicate});
for (auto* expr : exprs) {
if (static_cast<const PredicateExpression*>(expr)->name() == "exists") {
return false;
}
}

auto varProps = graph::ExpressionUtils::collectAll(
e, {Expression::Kind::kInputProperty, Expression::Kind::kVarProperty});
if (varProps.empty()) {
return false;
}
Expand All @@ -96,7 +95,7 @@ StatusOr<OptRule::TransformResult> PushFilterDownTraverseRule::transform(
if (!filterPicked) {
return TransformResult::noTransform();
}
auto* newfilterPicked =
auto* newFilterPicked =
graph::ExpressionUtils::rewriteEdgePropertyFilter(pool, edgeAlias, filterPicked->clone());

Filter* newFilter = nullptr;
Expand All @@ -122,13 +121,9 @@ StatusOr<OptRule::TransformResult> PushFilterDownTraverseRule::transform(
}

auto* eFilter = tv->eFilter();
Expression* newEFilter = nullptr;
if (eFilter) {
auto logicExpr = LogicalExpression::makeAnd(pool, newfilterPicked, eFilter->clone());
newEFilter = logicExpr;
} else {
newEFilter = newfilterPicked;
}
Expression* newEFilter = eFilter
? LogicalExpression::makeAnd(pool, newFilterPicked, eFilter->clone())
: newFilterPicked;

auto* newTv = static_cast<graph::Traverse*>(tv->clone());
newAv->setInputVar(newTv->outputVar());
Expand Down
13 changes: 13 additions & 0 deletions src/graph/optimizer/rule/PushFilterDownTraverseRule.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,19 @@
namespace nebula {
namespace opt {

/*
* Before:
* Filter(e.likeness > 78)
* |
* AppendVertices
* |
* Traverse
*
* After :
* AppendVertices
* |
* Traverse(eFilter_: *.likeness > 78)
*/
class PushFilterDownTraverseRule final : public OptRule {
public:
const Pattern &pattern() const override;
Expand Down
20 changes: 20 additions & 0 deletions src/graph/planner/plan/Query.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1538,6 +1538,10 @@ class Traverse final : public GetNeighbors {
return range_;
}

bool isOneStep() const {
return !range_;
}

// Contains zero step
bool zeroStep() const {
return range_ != nullptr && range_->min() == 0;
Expand All @@ -1555,6 +1559,17 @@ class Traverse final : public GetNeighbors {
return trackPrevPath_;
}

const std::string& nodeAlias() const {
auto& cols = this->colNames();
DCHECK_GE(cols.size(), 2);
return cols[cols.size() - 2];
}

const std::string& edgeAlias() const {
DCHECK(!this->colNames().empty());
return this->colNames().back();
}

void setStepRange(MatchStepRange* range) {
range_ = range;
}
Expand Down Expand Up @@ -1618,6 +1633,11 @@ class AppendVertices final : public GetVertices {
return trackPrevPath_;
}

const std::string nodeAlias() const {
DCHECK(!this->colNames().empty());
return this->colNames().back();
}

void setVertexFilter(Expression* vFilter) {
vFilter_ = vFilter;
}
Expand Down
71 changes: 38 additions & 33 deletions src/graph/util/ExpressionUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ Expression *ExpressionUtils::rewriteInExpr(const Expression *expr) {
return const_cast<Expression *>(expr);
}
std::vector<Expression *> operands;
operands.reserve(values.size());
for (const auto &v : values) {
operands.emplace_back(
RelationalExpression::makeEQ(pool, inExpr->left(), ConstantExpression::make(pool, v)));
Expand Down Expand Up @@ -1431,6 +1432,42 @@ bool ExpressionUtils::checkExprDepth(const Expression *expr) {
return true;
}

/*static*/
bool ExpressionUtils::isSingleLenExpandExpr(const std::string &edgeAlias, const Expression *expr) {
if (expr->kind() != Expression::Kind::kAttribute) {
return false;
}
auto attributeExpr = static_cast<const AttributeExpression *>(expr);
auto *left = attributeExpr->left();
auto *right = attributeExpr->right();

if (left->kind() != Expression::Kind::kSubscript) return false;
if (right->kind() != Expression::Kind::kConstant ||
!static_cast<const ConstantExpression *>(right)->value().isStr())
return false;

auto subscriptExpr = static_cast<const SubscriptExpression *>(left);
auto *listExpr = subscriptExpr->left();
auto *idxExpr = subscriptExpr->right();
if (listExpr->kind() != Expression::Kind::kInputProperty &&
listExpr->kind() != Expression::Kind::kVarProperty) {
return false;
}
if (static_cast<const PropertyExpression *>(listExpr)->prop() != edgeAlias) {
return false;
}

// NOTE(jie): Just handled `$-.e[0].likeness` for now, whileas the traverse is single length
// expand.
// TODO(jie): Handle `ALL(i IN e WHERE i.likeness > 78)`, whileas the traverse is var len
// expand.
if (idxExpr->kind() != Expression::Kind::kConstant ||
static_cast<const ConstantExpression *>(idxExpr)->value() != 0) {
return false;
}
return true;
}

// Transform expression `$-.e[0].likeness` to EdgePropertyExpression `like.likeness`
// for more friendly to push down
// \param pool object pool to hold ownership of objects alloacted
Expand All @@ -1440,39 +1477,7 @@ bool ExpressionUtils::checkExprDepth(const Expression *expr) {
const std::string &edgeAlias,
Expression *expr) {
graph::RewriteVisitor::Matcher matcher = [&edgeAlias](const Expression *e) -> bool {
if (e->kind() != Expression::Kind::kAttribute) {
return false;
}
auto attributeExpr = static_cast<const AttributeExpression *>(e);
auto *left = attributeExpr->left();
auto *right = attributeExpr->right();

if (left->kind() != Expression::Kind::kSubscript) return false;
if (right->kind() != Expression::Kind::kConstant ||
!static_cast<const ConstantExpression *>(right)->value().isStr())
return false;

auto subscriptExpr = static_cast<const SubscriptExpression *>(left);
auto *listExpr = subscriptExpr->left();
auto *idxExpr = subscriptExpr->right();
if (listExpr->kind() != Expression::Kind::kInputProperty &&
listExpr->kind() != Expression::Kind::kVarProperty) {
return false;
}
if (static_cast<const PropertyExpression *>(listExpr)->prop() != edgeAlias) {
return false;
}

// NOTE(jie): Just handled `$-.e[0].likeness` for now, whileas the traverse is single length
// expand.
// TODO(jie): Handle `ALL(i IN e WHERE i.likeness > 78)`, whileas the traverse is var len
// expand.
if (idxExpr->kind() != Expression::Kind::kConstant ||
static_cast<const ConstantExpression *>(idxExpr)->value() != 0) {
return false;
}

return true;
return isSingleLenExpandExpr(edgeAlias, e);
};
graph::RewriteVisitor::Rewriter rewriter = [pool](const Expression *e) -> Expression * {
DCHECK_EQ(e->kind(), Expression::Kind::kAttribute);
Expand Down
3 changes: 3 additions & 0 deletions src/graph/util/ExpressionUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,9 @@ class ExpressionUtils {
// e.g. id(v) == 1, id(v) IN [...]
static bool isVidPredication(const Expression* expr);

// Check if the expr looks like `$-.e[0].likeness`
static bool isSingleLenExpandExpr(const std::string& edgeAlias, const Expression* expr);

static Expression* rewriteEdgePropertyFilter(ObjectPool* pool,
const std::string& edgeAlias,
Expression* expr);
Expand Down
8 changes: 5 additions & 3 deletions src/storage/query/QueryBaseProcessor-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,9 +407,11 @@ nebula::cpp2::ErrorCode QueryBaseProcessor<REQ, RESP>::checkExp(
if (ret != nebula::cpp2::ErrorCode::SUCCEEDED) {
return ret;
}
ret = checkExp(predExp->filter(), returned, filtered, updated, allowNoexistentProp);
if (ret != nebula::cpp2::ErrorCode::SUCCEEDED) {
return ret;
if (predExp->hasFilter()) {
ret = checkExp(predExp->filter(), returned, filtered, updated, allowNoexistentProp);
if (ret != nebula::cpp2::ErrorCode::SUCCEEDED) {
return ret;
}
}
return nebula::cpp2::ErrorCode::SUCCEEDED;
}
Expand Down
58 changes: 27 additions & 31 deletions tests/tck/features/optimizer/PushFilterDownHashInnerJoinRule.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,19 @@ Feature: Push Filter down HashInnerJoin rule
| [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) |
| [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) |
And the execution plan should be:
| id | name | dependencies | operator info |
| 30 | Sort | 14 | |
| 14 | Project | 19 | |
| 19 | HashInnerJoin | 6,22 | |
| 6 | Project | 20 | |
| 20 | AppendVertices | 2 | |
| 2 | Dedup | 1 | |
| 1 | PassThrough | 3 | |
| 3 | Start | | |
| 22 | Project | 21 | |
| 21 | Filter | 10 | { "condition": "($-.e[0].likeness>0)" } |
| 10 | AppendVertices | 9 | |
| 9 | Traverse | 7 | |
| 7 | Argument | 8 | |
| 8 | Start | | |
| id | name | dependencies | operator info |
| 30 | Sort | 14 | |
| 14 | Project | 19 | |
| 19 | HashInnerJoin | 6,22 | |
| 6 | Project | 20 | |
| 20 | AppendVertices | 2 | |
| 2 | Dedup | 1 | |
| 1 | PassThrough | 3 | |
| 3 | Start | | |
| 22 | Project | 10 | |
| 10 | AppendVertices | 9 | |
| 9 | Traverse | 7 | {"edge filter": "(*.likeness>0)"} |
| 7 | Argument | | |
When profiling query:
"""
MATCH (v:player)
Expand Down Expand Up @@ -112,22 +110,20 @@ Feature: Push Filter down HashInnerJoin rule
| [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) |
| [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) |
And the execution plan should be:
| id | name | dependencies | operator info |
| 30 | Sort | 14 | |
| 14 | Project | 20 | |
| 20 | HashInnerJoin | 23,25 | |
| 23 | Project | 22 | |
| 22 | Filter | 21 | {"condition": "(v.player.age>0)"} |
| 21 | AppendVertices | 2 | |
| 2 | Dedup | 1 | |
| 1 | PassThrough | 3 | |
| 3 | Start | | |
| 25 | Project | 24 | |
| 24 | Filter | 10 | {"condition": "($-.e[0].likeness>0)"} |
| 10 | AppendVertices | 9 | |
| 9 | Traverse | 7 | |
| 7 | Argument | 8 | |
| 8 | Start | | |
| id | name | dependencies | operator info |
| 30 | Sort | 14 | |
| 14 | Project | 20 | |
| 20 | HashInnerJoin | 23,25 | |
| 23 | Project | 22 | |
| 22 | Filter | 21 | {"condition": "(v.player.age>0)"} |
| 21 | AppendVertices | 2 | |
| 2 | Dedup | 1 | |
| 1 | PassThrough | 3 | |
| 3 | Start | | |
| 25 | Project | 10 | |
| 10 | AppendVertices | 9 | |
| 9 | Traverse | 7 | {"edge filter": "(*.likeness>0)"} |
| 7 | Argument | | |
When profiling query:
"""
MATCH (v:player)
Expand Down
Loading

0 comments on commit db893c8

Please sign in to comment.