diff --git a/src/query/frontend/ast/ast.hpp b/src/query/frontend/ast/ast.hpp
index b39a0b12d..2e6f98aab 100644
--- a/src/query/frontend/ast/ast.hpp
+++ b/src/query/frontend/ast/ast.hpp
@@ -192,27 +192,6 @@ class AndOperator : public BinaryOperator {
using BinaryOperator::BinaryOperator;
};
-// This is separate operator so that we can implement different short-circuiting
-// semantics than regular AndOperator. At this point CypherMainVisitor shouldn't
-// concern itself with this, and should constructor only AndOperator-s. This is
-// used in query planner at the moment.
-class FilterAndOperator : public BinaryOperator {
- friend class AstTreeStorage;
-
- public:
- DEFVISITABLE(TreeVisitor<TypedValue>);
- bool Accept(HierarchicalTreeVisitor &visitor) override {
- if (visitor.PreVisit(*this)) {
- expression1_->Accept(visitor) && expression2_->Accept(visitor);
- }
- return visitor.PostVisit(*this);
- }
- CLONE_BINARY_EXPRESSION;
-
- protected:
- using BinaryOperator::BinaryOperator;
-};
-
class AdditionOperator : public BinaryOperator {
friend class AstTreeStorage;
diff --git a/src/query/frontend/ast/ast_visitor.hpp b/src/query/frontend/ast/ast_visitor.hpp
index 7aa7404d9..c92ad9c66 100644
--- a/src/query/frontend/ast/ast_visitor.hpp
+++ b/src/query/frontend/ast/ast_visitor.hpp
@@ -29,7 +29,6 @@ class MapLiteral;
class OrOperator;
class XorOperator;
class AndOperator;
-class FilterAndOperator;
class NotOperator;
class AdditionOperator;
class SubtractionOperator;
@@ -61,16 +60,16 @@ class Unwind;
class CreateIndex;
using TreeCompositeVisitor = ::utils::CompositeVisitor<
- Query, NamedExpression, OrOperator, XorOperator, AndOperator,
- FilterAndOperator, NotOperator, AdditionOperator, SubtractionOperator,
- MultiplicationOperator, DivisionOperator, ModOperator, NotEqualOperator,
- EqualOperator, LessOperator, GreaterOperator, LessEqualOperator,
- GreaterEqualOperator, InListOperator, ListMapIndexingOperator,
- ListSlicingOperator, IfOperator, UnaryPlusOperator, UnaryMinusOperator,
- IsNullOperator, ListLiteral, MapLiteral, PropertyLookup, LabelsTest,
- EdgeTypeTest, Aggregation, Function, All, Create, Match, Return, With,
- Pattern, NodeAtom, EdgeAtom, BreadthFirstAtom, Delete, Where, SetProperty,
- SetProperties, SetLabels, RemoveProperty, RemoveLabels, Merge, Unwind>;
+ Query, NamedExpression, OrOperator, XorOperator, AndOperator, NotOperator,
+ AdditionOperator, SubtractionOperator, MultiplicationOperator,
+ DivisionOperator, ModOperator, NotEqualOperator, EqualOperator,
+ LessOperator, GreaterOperator, LessEqualOperator, GreaterEqualOperator,
+ InListOperator, ListMapIndexingOperator, ListSlicingOperator, IfOperator,
+ UnaryPlusOperator, UnaryMinusOperator, IsNullOperator, ListLiteral,
+ MapLiteral, PropertyLookup, LabelsTest, EdgeTypeTest, Aggregation, Function,
+ All, Create, Match, Return, With, Pattern, NodeAtom, EdgeAtom,
+ BreadthFirstAtom, Delete, Where, SetProperty, SetProperties, SetLabels,
+ RemoveProperty, RemoveLabels, Merge, Unwind>;
using TreeLeafVisitor = ::utils::LeafVisitor<Identifier, PrimitiveLiteral,
ParameterLookup, CreateIndex>;
@@ -78,24 +77,24 @@ using TreeLeafVisitor = ::utils::LeafVisitor<Identifier, PrimitiveLiteral,
class HierarchicalTreeVisitor : public TreeCompositeVisitor,
public TreeLeafVisitor {
public:
- using TreeCompositeVisitor::PreVisit;
using TreeCompositeVisitor::PostVisit;
- using typename TreeLeafVisitor::ReturnType;
+ using TreeCompositeVisitor::PreVisit;
using TreeLeafVisitor::Visit;
+ using typename TreeLeafVisitor::ReturnType;
};
template <typename TResult>
using TreeVisitor = ::utils::Visitor<
TResult, Query, NamedExpression, OrOperator, XorOperator, AndOperator,
- FilterAndOperator, NotOperator, AdditionOperator, SubtractionOperator,
- MultiplicationOperator, DivisionOperator, ModOperator, NotEqualOperator,
- EqualOperator, LessOperator, GreaterOperator, LessEqualOperator,
- GreaterEqualOperator, InListOperator, ListMapIndexingOperator,
- ListSlicingOperator, IfOperator, UnaryPlusOperator, UnaryMinusOperator,
- IsNullOperator, ListLiteral, MapLiteral, PropertyLookup, LabelsTest,
- EdgeTypeTest, Aggregation, Function, All, ParameterLookup, Create, Match,
- Return, With, Pattern, NodeAtom, EdgeAtom, BreadthFirstAtom, Delete, Where,
- SetProperty, SetProperties, SetLabels, RemoveProperty, RemoveLabels, Merge,
- Unwind, Identifier, PrimitiveLiteral, CreateIndex>;
+ NotOperator, AdditionOperator, SubtractionOperator, MultiplicationOperator,
+ DivisionOperator, ModOperator, NotEqualOperator, EqualOperator,
+ LessOperator, GreaterOperator, LessEqualOperator, GreaterEqualOperator,
+ InListOperator, ListMapIndexingOperator, ListSlicingOperator, IfOperator,
+ UnaryPlusOperator, UnaryMinusOperator, IsNullOperator, ListLiteral,
+ MapLiteral, PropertyLookup, LabelsTest, EdgeTypeTest, Aggregation, Function,
+ All, ParameterLookup, Create, Match, Return, With, Pattern, NodeAtom,
+ EdgeAtom, BreadthFirstAtom, Delete, Where, SetProperty, SetProperties,
+ SetLabels, RemoveProperty, RemoveLabels, Merge, Unwind, Identifier,
+ PrimitiveLiteral, CreateIndex>;
} // namespace query
diff --git a/src/query/interpret/eval.hpp b/src/query/interpret/eval.hpp
index c7487b5a5..b74fcd405 100644
--- a/src/query/interpret/eval.hpp
+++ b/src/query/interpret/eval.hpp
@@ -96,7 +96,6 @@ class ExpressionEvaluator : public TreeVisitor<TypedValue> {
BINARY_OPERATOR_VISITOR(OrOperator, ||, OR);
BINARY_OPERATOR_VISITOR(XorOperator, ^, XOR);
- BINARY_OPERATOR_VISITOR(AndOperator, &&, AND);
BINARY_OPERATOR_VISITOR(AdditionOperator, +, +);
BINARY_OPERATOR_VISITOR(SubtractionOperator, -, -);
BINARY_OPERATOR_VISITOR(MultiplicationOperator, *, *);
@@ -116,7 +115,7 @@ class ExpressionEvaluator : public TreeVisitor<TypedValue> {
#undef BINARY_OPERATOR_VISITOR
#undef UNARY_OPERATOR_VISITOR
- TypedValue Visit(FilterAndOperator &op) override {
+ TypedValue Visit(AndOperator &op) override {
auto value1 = op.expression1_->Accept(*this);
if (value1.IsNull() || !value1.Value<bool>()) {
// If first expression is null or false, don't execute the second one.
diff --git a/src/query/plan/rule_based_planner.cpp b/src/query/plan/rule_based_planner.cpp
index 9de49c5c3..bd6dbfc89 100644
--- a/src/query/plan/rule_based_planner.cpp
+++ b/src/query/plan/rule_based_planner.cpp
@@ -3,6 +3,7 @@
#include <algorithm>
#include <functional>
#include <limits>
+#include <stack>
#include <unordered_set>
#include "utils/algorithm.hpp"
@@ -331,7 +332,6 @@ class ReturnBodyContext : public HierarchicalTreeVisitor {
VISIT_BINARY_OPERATOR(OrOperator)
VISIT_BINARY_OPERATOR(XorOperator)
VISIT_BINARY_OPERATOR(AndOperator)
- VISIT_BINARY_OPERATOR(FilterAndOperator)
VISIT_BINARY_OPERATOR(AdditionOperator)
VISIT_BINARY_OPERATOR(SubtractionOperator)
VISIT_BINARY_OPERATOR(MultiplicationOperator)
@@ -608,7 +608,7 @@ void AddMatching(const Match &match, SymbolTable &symbol_table,
}
// Iterates over `all_filters` joining them in one expression via
-// `FilterAndOperator`. Filters which use unbound symbols are skipped, as well
+// `AndOperator`. Filters which use unbound symbols are skipped, as well
// as those that fail the `predicate` function. The function takes a single
// argument, `FilterInfo`. All the joined filters are removed from
// `all_filters`.
@@ -622,8 +622,8 @@ Expression *ExtractFilters(const std::unordered_set<Symbol> &bound_symbols,
filters_it != all_filters.end();) {
if (HasBoundFilterSymbols(bound_symbols, *filters_it) &&
predicate(*filters_it)) {
- filter_expr = impl::BoolJoin<FilterAndOperator>(storage, filter_expr,
- filters_it->expression);
+ filter_expr = impl::BoolJoin<AndOperator>(storage, filter_expr,
+ filters_it->expression);
filters_it = all_filters.erase(filters_it);
} else {
filters_it++;
@@ -631,6 +631,24 @@ Expression *ExtractFilters(const std::unordered_set<Symbol> &bound_symbols,
}
return filter_expr;
}
+
+auto SplitExpressionOnAnd(Expression *expression) {
+ std::vector<Expression *> expressions;
+ std::stack<Expression *> pending_expressions;
+ pending_expressions.push(expression);
+ while (!pending_expressions.empty()) {
+ auto *current_expression = pending_expressions.top();
+ pending_expressions.pop();
+ if (auto *and_op = dynamic_cast<AndOperator *>(current_expression)) {
+ pending_expressions.push(and_op->expression1_);
+ pending_expressions.push(and_op->expression2_);
+ } else {
+ expressions.push_back(current_expression);
+ }
+ }
+ return expressions;
+}
+
} // namespace
namespace impl {
@@ -995,10 +1013,13 @@ void Filters::CollectPatternFilters(Pattern &pattern, SymbolTable &symbol_table,
// information for potential property and label indexing.
void Filters::CollectWhereFilter(Where &where,
const SymbolTable &symbol_table) {
- UsedSymbolsCollector collector(symbol_table);
- where.expression_->Accept(collector);
- all_filters_.emplace_back(FilterInfo{where.expression_, collector.symbols_});
- AnalyzeFilter(where.expression_, symbol_table);
+ auto where_filters = SplitExpressionOnAnd(where.expression_);
+ for (const auto &filter : where_filters) {
+ UsedSymbolsCollector collector(symbol_table);
+ filter->Accept(collector);
+ all_filters_.emplace_back(FilterInfo{filter, collector.symbols_});
+ AnalyzeFilter(filter, symbol_table);
+ }
}
// Converts a Query to multiple QueryParts. In the process new Ast nodes may be
diff --git a/src/query/plan/rule_based_planner.hpp b/src/query/plan/rule_based_planner.hpp
index 6d0023647..a9805a4c9 100644
--- a/src/query/plan/rule_based_planner.hpp
+++ b/src/query/plan/rule_based_planner.hpp
@@ -548,9 +548,10 @@ class RuleBasedPlanner {
const auto &next_node_symbol =
symbol_table.at(*bf_atom->next_node_identifier_);
// Inline BFS edge filtering together with its filter expression.
- auto *filter_expr = impl::BoolJoin<FilterAndOperator>(
- storage, impl::ExtractMultiExpandFilter(
- bound_symbols, node_symbol, all_filters, storage),
+ auto *filter_expr = impl::BoolJoin<AndOperator>(
+ storage,
+ impl::ExtractMultiExpandFilter(bound_symbols, node_symbol,
+ all_filters, storage),
bf_atom->filter_expression_);
last_op = new ExpandBreadthFirst(
node_symbol, edge_symbol, expansion.direction, edge_type,
diff --git a/tests/unit/query_expression_evaluator.cpp b/tests/unit/query_expression_evaluator.cpp
index 37815f167..3f2a6a5fa 100644
--- a/tests/unit/query_expression_evaluator.cpp
+++ b/tests/unit/query_expression_evaluator.cpp
@@ -101,25 +101,18 @@ TEST(ExpressionEvaluator, AndOperator) {
ASSERT_EQ(val2.Value<bool>(), false);
}
-TEST(ExpressionEvaluator, FilterAndOperator) {
+TEST(ExpressionEvaluator, AndOperatorShortCircuit) {
AstTreeStorage storage;
NoContextExpressionEvaluator eval;
{
- auto *op = storage.Create<FilterAndOperator>(
- storage.Create<PrimitiveLiteral>(true),
- storage.Create<PrimitiveLiteral>(true));
- auto value = op->Accept(eval.eval);
- EXPECT_EQ(value.Value<bool>(), true);
- }
- {
- auto *op = storage.Create<FilterAndOperator>(
- storage.Create<PrimitiveLiteral>(false),
- storage.Create<PrimitiveLiteral>(5));
+ auto *op =
+ storage.Create<AndOperator>(storage.Create<PrimitiveLiteral>(false),
+ storage.Create<PrimitiveLiteral>(5));
auto value = op->Accept(eval.eval);
EXPECT_EQ(value.Value<bool>(), false);
}
{
- auto *op = storage.Create<FilterAndOperator>(
+ auto *op = storage.Create<AndOperator>(
storage.Create<PrimitiveLiteral>(TypedValue::Null),
storage.Create<PrimitiveLiteral>(5));
auto value = op->Accept(eval.eval);
diff --git a/tests/unit/query_planner.cpp b/tests/unit/query_planner.cpp
index 411f37291..9b33d08d3 100644
--- a/tests/unit/query_planner.cpp
+++ b/tests/unit/query_planner.cpp
@@ -1399,4 +1399,18 @@ TEST(TestLogicalPlanner, MatchScanToExpand) {
ExpectFilter(), ExpectProduce());
}
+TEST(TestLogicalPlanner, MatchWhereAndSplit) {
+ // Test MATCH (n) -[r]- (m) WHERE n.prop AND r.prop RETURN m
+ Dbms dbms;
+ auto dba = dbms.active();
+ auto prop = PROPERTY_PAIR("prop");
+ AstTreeStorage storage;
+ QUERY(MATCH(PATTERN(NODE("n"), EDGE("r"), NODE("m"))),
+ WHERE(AND(PROPERTY_LOOKUP("n", prop), PROPERTY_LOOKUP("r", prop))),
+ RETURN("m"));
+ // We expect `n.prop` filter right after scanning `n`.
+ CheckPlan(storage, ExpectScanAll(), ExpectFilter(), ExpectExpand(),
+ ExpectFilter(), ExpectProduce());
+}
+
} // namespace