Skip to content

Commit

Permalink
Refactor pullAnds & pullOrs (vesoft-inc#406)
Browse files Browse the repository at this point in the history 
Co-authored-by: cpw <[email protected]>
Co-authored-by: Yee <[email protected]>
  • Loading branch information
@dutor @CPWstatic @yixinglu
3 people authored Nov 13, 2020
1 parent 93f4a29 commit 5ca5bbf
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 89 deletions.
8 changes: 6 additions & 2 deletions src/planner/planners/MatchVertexIndexSeekPlanner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Expression* MatchVertexIndexSeekPlanner::makeIndexFilter(const std::string &labe

Expression* MatchVertexIndexSeekPlanner::makeIndexFilter(const std::string &label,
const std::string &alias,
const Expression *filter,
Expression *filter,
QueryContext* qctx) {
static const std::unordered_set<Expression::Kind> kinds = {
Expression::Kind::kRelEQ,
Expand All @@ -81,7 +81,11 @@ Expression* MatchVertexIndexSeekPlanner::makeIndexFilter(const std::string &labe
if (kinds.count(kind) == 1) {
ands.emplace_back(filter);
} else if (kind == Expression::Kind::kLogicalAnd) {
ands = ExpressionUtils::pullAnds(filter);
auto *logic = static_cast<LogicalExpression*>(filter);
ExpressionUtils::pullAnds(logic);
for (auto &operand : logic->operands()) {
ands.emplace_back(operand.get());
}
} else {
return nullptr;
}
Expand Down
2 changes: 1 addition & 1 deletion src/planner/planners/MatchVertexIndexSeekPlanner.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class MatchVertexIndexSeekPlanner final : public Planner {

static Expression* makeIndexFilter(const std::string& label,
const std::string& alias,
const Expression* filter,
Expression* filter,
QueryContext* qctx);

MatchVertexIndexSeekPlanner() = default;
Expand Down
68 changes: 32 additions & 36 deletions src/util/ExpressionUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,50 +21,46 @@ std::unique_ptr<Expression> ExpressionUtils::foldConstantExpr(const Expression *
return newExpr;
}

std::vector<const Expression*> ExpressionUtils::pullAnds(const Expression *expr) {
Expression* ExpressionUtils::pullAnds(Expression *expr) {
DCHECK(expr->kind() == Expression::Kind::kLogicalAnd);
auto *root = static_cast<const LogicalExpression*>(expr);
std::vector<const Expression*> operands;

// TODO(dutor) Deal with n-ary operands
if (root->operand(0)->kind() != Expression::Kind::kLogicalAnd) {
operands.emplace_back(root->operand(0));
} else {
auto ands = pullAnds(root->operand(0));
operands.insert(operands.end(), ands.begin(), ands.end());
}

if (root->operand(1)->kind() != Expression::Kind::kLogicalAnd) {
operands.emplace_back(root->operand(1));
} else {
auto ands = pullAnds(root->operand(1));
operands.insert(operands.end(), ands.begin(), ands.end());
}

return operands;
auto *logic = static_cast<LogicalExpression*>(expr);
std::vector<std::unique_ptr<Expression>> operands;
pullAndsImpl(logic, operands);
logic->setOperands(std::move(operands));
return logic;
}

std::vector<const Expression*> ExpressionUtils::pullOrs(const Expression *expr) {
Expression* ExpressionUtils::pullOrs(Expression *expr) {
DCHECK(expr->kind() == Expression::Kind::kLogicalOr);
auto *root = static_cast<const LogicalExpression*>(expr);
std::vector<const Expression*> operands;
auto *logic = static_cast<LogicalExpression*>(expr);
std::vector<std::unique_ptr<Expression>> operands;
pullOrsImpl(logic, operands);
logic->setOperands(std::move(operands));
return logic;
}

// TODO(dutor) Deal with n-ary operands
if (root->operand(0)->kind() != Expression::Kind::kLogicalOr) {
operands.emplace_back(root->operand(0));
} else {
auto ands = pullOrs(root->operand(0));
operands.insert(operands.end(), ands.begin(), ands.end());
void
ExpressionUtils::pullAndsImpl(LogicalExpression *expr,
std::vector<std::unique_ptr<Expression>> &operands) {
for (auto &operand : expr->operands()) {
if (operand->kind() != Expression::Kind::kLogicalAnd) {
operands.emplace_back(std::move(operand));
continue;
}
pullAndsImpl(static_cast<LogicalExpression*>(operand.get()), operands);
}
}

if (root->operand(1)->kind() != Expression::Kind::kLogicalOr) {
operands.emplace_back(root->operand(1));
} else {
auto ands = pullOrs(root->operand(1));
operands.insert(operands.end(), ands.begin(), ands.end());
void
ExpressionUtils::pullOrsImpl(LogicalExpression *expr,
std::vector<std::unique_ptr<Expression>> &operands) {
for (auto &operand : expr->operands()) {
if (operand->kind() != Expression::Kind::kLogicalOr) {
operands.emplace_back(std::move(operand));
continue;
}
pullOrsImpl(static_cast<LogicalExpression*>(operand.get()), operands);
}

return operands;
}

} // namespace graph
Expand Down
8 changes: 6 additions & 2 deletions src/util/ExpressionUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,13 @@ class ExpressionUtils {
// Clone and fold constant expression
static std::unique_ptr<Expression> foldConstantExpr(const Expression* expr);

static std::vector<const Expression*> pullAnds(const Expression *expr);
static Expression* pullAnds(Expression *expr);
static void pullAndsImpl(LogicalExpression *expr,
std::vector<std::unique_ptr<Expression>> &operands);

static std::vector<const Expression*> pullOrs(const Expression *expr);
static Expression* pullOrs(Expression *expr);
static void pullOrsImpl(LogicalExpression *expr,
std::vector<std::unique_ptr<Expression>> &operands);
};

} // namespace graph
Expand Down
111 changes: 63 additions & 48 deletions src/util/test/ExpressionUtilsTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,23 +210,28 @@ TEST_F(ExpressionUtilsTest, PullAnds) {
auto *first = new ConstantExpression(true);
auto *second = new ConstantExpression(false);
LogicalExpression expr(Kind::kLogicalAnd, first, second);
auto ands = ExpressionUtils::pullAnds(&expr);
ASSERT_EQ(2UL, ands.size());
ASSERT_EQ(first, ands[0]);
ASSERT_EQ(second, ands[1]);
LogicalExpression expected(Kind::kLogicalAnd,
first->clone().release(),
second->clone().release());
ExpressionUtils::pullAnds(&expr);
ASSERT_EQ(expected, expr);
}
// true AND false AND true
{
auto *first = new ConstantExpression(true);
auto *second = new ConstantExpression(false);
auto *third = new ConstantExpression(true);
LogicalExpression expr(Kind::kLogicalAnd,
new LogicalExpression(Kind::kLogicalAnd, first, second), third);
auto ands = ExpressionUtils::pullAnds(&expr);
ASSERT_EQ(3UL, ands.size());
ASSERT_EQ(first, ands[0]);
ASSERT_EQ(second, ands[1]);
ASSERT_EQ(third, ands[2]);
new LogicalExpression(Kind::kLogicalAnd,
first,
second),
third);
LogicalExpression expected(Kind::kLogicalAnd);
expected.addOperand(first->clone().release());
expected.addOperand(second->clone().release());
expected.addOperand(third->clone().release());
ExpressionUtils::pullAnds(&expr);
ASSERT_EQ(expected, expr);
}
// true AND (false AND true)
{
Expand All @@ -235,12 +240,15 @@ TEST_F(ExpressionUtilsTest, PullAnds) {
auto *third = new ConstantExpression(true);
LogicalExpression expr(Kind::kLogicalAnd,
first,
new LogicalExpression(Kind::kLogicalAnd, second, third));
auto ands = ExpressionUtils::pullAnds(&expr);
ASSERT_EQ(3UL, ands.size());
ASSERT_EQ(first, ands[0]);
ASSERT_EQ(second, ands[1]);
ASSERT_EQ(third, ands[2]);
new LogicalExpression(Kind::kLogicalAnd,
second,
third));
LogicalExpression expected(Kind::kLogicalAnd);
expected.addOperand(first->clone().release());
expected.addOperand(second->clone().release());
expected.addOperand(third->clone().release());
ExpressionUtils::pullAnds(&expr);
ASSERT_EQ(expected, expr);
}
// (true OR false) AND (true OR false)
{
Expand All @@ -251,10 +259,11 @@ TEST_F(ExpressionUtilsTest, PullAnds) {
new ConstantExpression(true),
new ConstantExpression(false));
LogicalExpression expr(Kind::kLogicalAnd, first, second);
auto ands = ExpressionUtils::pullAnds(&expr);
ASSERT_EQ(2UL, ands.size());
ASSERT_EQ(first, ands[0]);
ASSERT_EQ(second, ands[1]);
LogicalExpression expected(Kind::kLogicalAnd);
expected.addOperand(first->clone().release());
expected.addOperand(second->clone().release());
ExpressionUtils::pullAnds(&expr);
ASSERT_EQ(expected, expr);
}
// true AND ((false AND true) OR false) AND true
{
Expand All @@ -267,11 +276,12 @@ TEST_F(ExpressionUtilsTest, PullAnds) {
auto *third = new ConstantExpression(true);
LogicalExpression expr(Kind::kLogicalAnd,
new LogicalExpression(Kind::kLogicalAnd, first, second), third);
auto ands = ExpressionUtils::pullAnds(&expr);
ASSERT_EQ(3UL, ands.size());
ASSERT_EQ(first, ands[0]);
ASSERT_EQ(second, ands[1]);
ASSERT_EQ(third, ands[2]);
LogicalExpression expected(Kind::kLogicalAnd);
expected.addOperand(first->clone().release());
expected.addOperand(second->clone().release());
expected.addOperand(third->clone().release());
ExpressionUtils::pullAnds(&expr);
ASSERT_EQ(expected, expr);
}
}

Expand All @@ -282,10 +292,11 @@ TEST_F(ExpressionUtilsTest, PullOrs) {
auto *first = new ConstantExpression(true);
auto *second = new ConstantExpression(false);
LogicalExpression expr(Kind::kLogicalOr, first, second);
auto ors = ExpressionUtils::pullOrs(&expr);
ASSERT_EQ(2UL, ors.size());
ASSERT_EQ(first, ors[0]);
ASSERT_EQ(second, ors[1]);
LogicalExpression expected(Kind::kLogicalOr,
first->clone().release(),
second->clone().release());
ExpressionUtils::pullOrs(&expr);
ASSERT_EQ(expected, expr);
}
// true OR false OR true
{
Expand All @@ -294,11 +305,12 @@ TEST_F(ExpressionUtilsTest, PullOrs) {
auto *third = new ConstantExpression(true);
LogicalExpression expr(Kind::kLogicalOr,
new LogicalExpression(Kind::kLogicalOr, first, second), third);
auto ors = ExpressionUtils::pullOrs(&expr);
ASSERT_EQ(3UL, ors.size());
ASSERT_EQ(first, ors[0]);
ASSERT_EQ(second, ors[1]);
ASSERT_EQ(third, ors[2]);
LogicalExpression expected(Kind::kLogicalOr);
expected.addOperand(first->clone().release());
expected.addOperand(second->clone().release());
expected.addOperand(third->clone().release());
ExpressionUtils::pullOrs(&expr);
ASSERT_EQ(expected, expr);
}
// true OR (false OR true)
{
Expand All @@ -308,11 +320,12 @@ TEST_F(ExpressionUtilsTest, PullOrs) {
LogicalExpression expr(Kind::kLogicalOr,
first,
new LogicalExpression(Kind::kLogicalOr, second, third));
auto ors = ExpressionUtils::pullOrs(&expr);
ASSERT_EQ(3UL, ors.size());
ASSERT_EQ(first, ors[0]);
ASSERT_EQ(second, ors[1]);
ASSERT_EQ(third, ors[2]);
LogicalExpression expected(Kind::kLogicalOr);
expected.addOperand(first->clone().release());
expected.addOperand(second->clone().release());
expected.addOperand(third->clone().release());
ExpressionUtils::pullOrs(&expr);
ASSERT_EQ(expected, expr);
}
// (true AND false) OR (true AND false)
{
Expand All @@ -323,10 +336,11 @@ TEST_F(ExpressionUtilsTest, PullOrs) {
new ConstantExpression(true),
new ConstantExpression(false));
LogicalExpression expr(Kind::kLogicalOr, first, second);
auto ors = ExpressionUtils::pullOrs(&expr);
ASSERT_EQ(2UL, ors.size());
ASSERT_EQ(first, ors[0]);
ASSERT_EQ(second, ors[1]);
LogicalExpression expected(Kind::kLogicalOr,
first->clone().release(),
second->clone().release());
ExpressionUtils::pullOrs(&expr);
ASSERT_EQ(expected, expr);
}
// true OR ((false OR true) AND false) OR true
{
Expand All @@ -339,11 +353,12 @@ TEST_F(ExpressionUtilsTest, PullOrs) {
auto *third = new ConstantExpression(true);
LogicalExpression expr(Kind::kLogicalOr,
new LogicalExpression(Kind::kLogicalOr, first, second), third);
auto ors = ExpressionUtils::pullOrs(&expr);
ASSERT_EQ(3UL, ors.size());
ASSERT_EQ(first, ors[0]);
ASSERT_EQ(second, ors[1]);
ASSERT_EQ(third, ors[2]);
LogicalExpression expected(Kind::kLogicalOr);
expected.addOperand(first->clone().release());
expected.addOperand(second->clone().release());
expected.addOperand(third->clone().release());
ExpressionUtils::pullOrs(&expr);
ASSERT_EQ(expected, expr);
}
}

Expand Down

0 comments on commit 5ca5bbf

Please sign in to comment.