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.

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

Avoid full scan of match when there is an Relational In predicate #4748

Merged
merged 3 commits into from
Oct 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 34 additions & 14 deletions src/common/expression/ContainerExpression.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
#ifndef COMMON_EXPRESSION_CONTAINEREXPRESSION_H_
#define COMMON_EXPRESSION_CONTAINEREXPRESSION_H_

#include "common/base/ObjectPool.h"
#include "common/expression/ConstantExpression.h"
#include "common/expression/ContainerExpression.h"
#include "common/expression/Expression.h"

namespace nebula {
Expand Down Expand Up @@ -63,7 +66,16 @@ class MapItemList final {
std::vector<Pair> items_;
};

class ListExpression final : public Expression {
class ContainerExpression : public Expression {
czpmango marked this conversation as resolved.
Show resolved Hide resolved
public:
virtual const std::vector<Expression *> getKeys() const = 0;
czpmango marked this conversation as resolved.
Show resolved Hide resolved
virtual size_t size() const = 0;

protected:
ContainerExpression(ObjectPool *pool, Expression::Kind kind) : Expression(pool, kind) {}
};

class ListExpression final : public ContainerExpression {
public:
ListExpression &operator=(const ListExpression &rhs) = delete;
ListExpression &operator=(ListExpression &&) = delete;
Expand All @@ -84,15 +96,15 @@ class ListExpression final : public Expression {
items_[index] = item;
}

std::vector<Expression *> get() {
const std::vector<Expression *> getKeys() const override {
return items_;
}

void setItems(std::vector<Expression *> items) {
items_ = items;
}

size_t size() const {
size_t size() const override {
return items_.size();
}

Expand All @@ -116,9 +128,9 @@ class ListExpression final : public Expression {

private:
friend ObjectPool;
explicit ListExpression(ObjectPool *pool) : Expression(pool, Kind::kList) {}
explicit ListExpression(ObjectPool *pool) : ContainerExpression(pool, Kind::kList) {}

ListExpression(ObjectPool *pool, ExpressionList *items) : Expression(pool, Kind::kList) {
ListExpression(ObjectPool *pool, ExpressionList *items) : ContainerExpression(pool, Kind::kList) {
items_ = items->get();
}

Expand All @@ -131,7 +143,7 @@ class ListExpression final : public Expression {
Value result_;
};

class SetExpression final : public Expression {
class SetExpression final : public ContainerExpression {
public:
SetExpression &operator=(const SetExpression &rhs) = delete;
SetExpression &operator=(SetExpression &&) = delete;
Expand All @@ -152,15 +164,15 @@ class SetExpression final : public Expression {
items_[index] = item;
}

std::vector<Expression *> get() {
const std::vector<Expression *> getKeys() const override {
return items_;
}

void setItems(std::vector<Expression *> items) {
items_ = items;
}

size_t size() const {
size_t size() const override {
return items_.size();
}

Expand All @@ -184,9 +196,9 @@ class SetExpression final : public Expression {

private:
friend ObjectPool;
explicit SetExpression(ObjectPool *pool) : Expression(pool, Kind::kSet) {}
explicit SetExpression(ObjectPool *pool) : ContainerExpression(pool, Kind::kSet) {}

SetExpression(ObjectPool *pool, ExpressionList *items) : Expression(pool, Kind::kSet) {
SetExpression(ObjectPool *pool, ExpressionList *items) : ContainerExpression(pool, Kind::kSet) {
items_ = items->get();
}

Expand All @@ -199,7 +211,7 @@ class SetExpression final : public Expression {
Value result_;
};

class MapExpression final : public Expression {
class MapExpression final : public ContainerExpression {
public:
MapExpression &operator=(const MapExpression &rhs) = delete;
MapExpression &operator=(MapExpression &&) = delete;
Expand Down Expand Up @@ -230,7 +242,15 @@ class MapExpression final : public Expression {
return items_;
}

size_t size() const {
const std::vector<Expression *> getKeys() const override {
std::vector<Expression *> keys;
for (const auto &item : items_) {
keys.emplace_back(ConstantExpression::make(pool_, item.first));
}
return keys;
}

size_t size() const override {
return items_.size();
}

Expand All @@ -254,9 +274,9 @@ class MapExpression final : public Expression {

private:
friend ObjectPool;
explicit MapExpression(ObjectPool *pool) : Expression(pool, Kind::kMap) {}
explicit MapExpression(ObjectPool *pool) : ContainerExpression(pool, Kind::kMap) {}

MapExpression(ObjectPool *pool, MapItemList *items) : Expression(pool, Kind::kMap) {
MapExpression(ObjectPool *pool, MapItemList *items) : ContainerExpression(pool, Kind::kMap) {
items_ = items->get();
}

Expand Down
51 changes: 49 additions & 2 deletions src/graph/util/ExpressionUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@

#include "graph/util/ExpressionUtils.h"

#include "ExpressionUtils.h"
#include "common/base/ObjectPool.h"
#include "common/expression/ArithmeticExpression.h"
#include "common/expression/ConstantExpression.h"
#include "common/expression/ContainerExpression.h"
#include "common/expression/Expression.h"
#include "common/expression/PropertyExpression.h"
#include "common/function/AggFunctionManager.h"
Expand Down Expand Up @@ -201,6 +204,50 @@ Expression *ExpressionUtils::rewriteParameter(const Expression *expr, QueryConte
return graph::RewriteVisitor::transform(expr, matcher, rewriter);
}

Expression *ExpressionUtils::rewriteInnerInExpr(const Expression *expr) {
czpmango marked this conversation as resolved.
Show resolved Hide resolved
auto matcher = [](const Expression *e) -> bool {
if (e->kind() != Expression::Kind::kRelIn) {
return false;
}
auto rhs = static_cast<const RelationalExpression *>(e)->right();
if (rhs->kind() != Expression::Kind::kList && rhs->kind() != Expression::Kind::kSet) {
return false;
}
auto items = static_cast<const ContainerExpression *>(rhs)->getKeys();
for (const auto *item : items) {
if (!ExpressionUtils::isEvaluableExpr(item)) {
return false;
}
}
return true;
};
auto rewriter = [](const Expression *e) -> Expression * {
DCHECK_EQ(e->kind(), Expression::Kind::kRelIn);
const auto re = static_cast<const RelationalExpression *>(e);
auto lhs = re->left();
auto rhs = re->right();
DCHECK(rhs->kind() == Expression::Kind::kList || rhs->kind() == Expression::Kind::kSet);
auto ce = static_cast<const ContainerExpression *>(rhs);
auto pool = e->getObjPool();
auto *rewrittenExpr = LogicalExpression::makeOr(pool);
// Pointer to a single-level expression
Expression *singleExpr = nullptr;
auto items = ce->getKeys();
for (auto i = 0u; i < items.size(); ++i) {
auto *ee = RelationalExpression::makeEQ(pool, lhs->clone(), items[i]->clone());
rewrittenExpr->addOperand(ee);
if (i == 0) {
singleExpr = ee;
} else {
singleExpr = nullptr;
}
}
return singleExpr ? singleExpr : rewrittenExpr;
};

return graph::RewriteVisitor::transform(expr, matcher, rewriter);
}

Expression *ExpressionUtils::rewriteAgg2VarProp(const Expression *expr) {
ObjectPool *pool = expr->getObjPool();
auto matcher = [](const Expression *e) -> bool {
Expand Down Expand Up @@ -368,10 +415,10 @@ std::vector<Expression *> ExpressionUtils::getContainerExprOperands(const Expres
std::vector<Expression *> containerOperands;
switch (containerExpr->kind()) {
case Expression::Kind::kList:
containerOperands = static_cast<ListExpression *>(containerExpr)->get();
containerOperands = static_cast<ListExpression *>(containerExpr)->getKeys();
break;
case Expression::Kind::kSet: {
containerOperands = static_cast<SetExpression *>(containerExpr)->get();
containerOperands = static_cast<SetExpression *>(containerExpr)->getKeys();
break;
}
case Expression::Kind::kMap: {
Expand Down
3 changes: 3 additions & 0 deletions src/graph/util/ExpressionUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ class ExpressionUtils {
// Rewrites ParameterExpression to ConstantExpression
static Expression* rewriteParameter(const Expression* expr, QueryContext* qctx);

// Rewrite RelInExpr with only one operand in expression tree
static Expression* rewriteInnerInExpr(const Expression* expr);

// Rewrites relational expression, gather all evaluable expressions in the left operands and move
// them to the right
static Expression* rewriteRelExpr(const Expression* expr);
Expand Down
3 changes: 2 additions & 1 deletion src/graph/validator/MatchValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,8 @@ Status MatchValidator::buildEdgeInfo(const MatchPath *path,
// Rewrite expression to fit semantic, check type and check used aliases.
Status MatchValidator::validateFilter(const Expression *filter,
WhereClauseContext &whereClauseCtx) {
auto transformRes = ExpressionUtils::filterTransform(filter);
auto *newFilter = graph::ExpressionUtils::rewriteInnerInExpr(filter);
auto transformRes = ExpressionUtils::filterTransform(newFilter);
NG_RETURN_IF_ERROR(transformRes);
// rewrite Attribute to LabelTagProperty
whereClauseCtx.filter = ExpressionUtils::rewriteAttr2LabelTagProp(
Expand Down
29 changes: 29 additions & 0 deletions tests/tck/features/match/IndexSelecting.feature
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,35 @@ Feature: Index selecting for match statement
| 2 | AppendVertices | 5 | |
| 5 | IndexScan | 0 | {"indexCtx": {"columnHints":{"scanType":"PREFIX"}}} |
| 0 | Start | | |
When profiling query:
"""
MATCH (v:player) WHERE v.player.name in ["Yao Ming"] RETURN v.player.name AS name
czpmango marked this conversation as resolved.
Show resolved Hide resolved
"""
Then the result should be, in any order, with relax comparison:
| name |
| "Yao Ming" |
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 | | |
When profiling query:
"""
MATCH (v:player) WHERE v.player.name in ["Yao Ming", "Tim Duncan"] RETURN v.player.name AS name
"""
Then the result should be, in any order, with relax comparison:
| name |
| "Tim Duncan" |
| "Yao Ming" |
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 | | |
When profiling query:
"""
MATCH (v:player) WHERE v.player.name == "Tim Duncan" and v.player.name < "Zom" RETURN v.player.name AS name
Expand Down