Support reversing ExpandVariable
Summary: This is needed in cases when the planner decides to start expanding from the other end. Reviewers: mislav.bradac, florijan Reviewed By: mislav.bradac Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D681
This commit is contained in:
Teon Banek
parent
bf13c00879
commit
ac9f6170d6
@ -527,7 +527,7 @@ bool Expand::ExpandCursor::PullNode(const EdgeAccessor &new_edge,
|
||||
}
|
||||
|
||||
ExpandVariable::ExpandVariable(Symbol node_symbol, Symbol edge_symbol,
|
||||
EdgeAtom::Direction direction,
|
||||
EdgeAtom::Direction direction, bool is_reverse,
|
||||
Expression *lower_bound, Expression *upper_bound,
|
||||
const std::shared_ptr<LogicalOperator> &input,
|
||||
Symbol input_symbol, bool existing_node,
|
||||
@ -535,7 +535,8 @@ ExpandVariable::ExpandVariable(Symbol node_symbol, Symbol edge_symbol,
|
||||
: ExpandCommon(node_symbol, edge_symbol, direction, input, input_symbol,
|
||||
existing_node, existing_edge, graph_view),
|
||||
lower_bound_(lower_bound),
|
||||
upper_bound_(upper_bound) {}
|
||||
upper_bound_(upper_bound),
|
||||
is_reverse_(is_reverse) {}
|
||||
|
||||
bool Expand::ExpandCursor::HandleExistingEdge(const EdgeAccessor &new_edge,
|
||||
Frame &frame) const {
|
||||
@ -746,8 +747,10 @@ class ExpandVariableCursor : public Cursor {
|
||||
if (edges_on_frame.size() < edges_.size())
|
||||
return EdgePlacementResult::MISMATCH;
|
||||
|
||||
int last_edge_index =
|
||||
self_.is_reverse_ ? 0 : static_cast<int>(edges_.size()) - 1;
|
||||
EdgeAccessor &edge_on_frame =
|
||||
edges_on_frame[edges_.size() - 1].Value<EdgeAccessor>();
|
||||
edges_on_frame[last_edge_index].Value<EdgeAccessor>();
|
||||
if (edge_on_frame != new_edge) return EdgePlacementResult::MISMATCH;
|
||||
|
||||
// the new_edge matches the corresponding frame element
|
||||
@ -763,8 +766,20 @@ class ExpandVariableCursor : public Cursor {
|
||||
// it is possible that there already exists an edge on the frame for
|
||||
// this level. if so first remove it
|
||||
debug_assert(edges_.size() > 0, "Edges are empty");
|
||||
edges_on_frame.resize(std::min(edges_on_frame.size(), edges_.size() - 1));
|
||||
edges_on_frame.emplace_back(new_edge);
|
||||
if (self_.is_reverse_) {
|
||||
// TODO: This is innefficient, we should look into replacing
|
||||
// vector with something else for TypedValue::List.
|
||||
size_t diff = edges_on_frame.size() -
|
||||
std::min(edges_on_frame.size(), edges_.size() - 1U);
|
||||
if (diff > 0U)
|
||||
edges_on_frame.erase(edges_on_frame.begin(),
|
||||
edges_on_frame.begin() + diff);
|
||||
edges_on_frame.insert(edges_on_frame.begin(), new_edge);
|
||||
} else {
|
||||
edges_on_frame.resize(
|
||||
std::min(edges_on_frame.size(), edges_.size() - 1U));
|
||||
edges_on_frame.emplace_back(new_edge);
|
||||
}
|
||||
return EdgePlacementResult::YIELD;
|
||||
}
|
||||
}
|
||||
@ -803,8 +818,18 @@ class ExpandVariableCursor : public Cursor {
|
||||
// elements as edges_ due to edge-uniqueness (when a whole layer
|
||||
// gets exhausted but no edges are valid). for that reason only
|
||||
// pop from edges_on_frame if they contain enough elements
|
||||
if (!self_.existing_edge_)
|
||||
edges_on_frame.resize(std::min(edges_on_frame.size(), edges_.size()));
|
||||
if (!self_.existing_edge_) {
|
||||
if (self_.is_reverse_) {
|
||||
auto diff = edges_on_frame.size() -
|
||||
std::min(edges_on_frame.size(), edges_.size());
|
||||
if (diff > 0) {
|
||||
edges_on_frame.erase(edges_on_frame.begin(),
|
||||
edges_on_frame.begin() + diff);
|
||||
}
|
||||
} else {
|
||||
edges_on_frame.resize(std::min(edges_on_frame.size(), edges_.size()));
|
||||
}
|
||||
}
|
||||
|
||||
// if we are here, we have a valid stack,
|
||||
// get the edge, increase the relevant iterator
|
||||
|
||||
@ -615,14 +615,17 @@ class ExpandVariable : public LogicalOperator, ExpandCommon {
|
||||
* Expansion length bounds are both inclusive (as in Neo's Cypher
|
||||
* implementation).
|
||||
*
|
||||
* @param is_reverse Set to `true` if the edges written on frame should expand
|
||||
* from `node_symbol` to `input_symbol`. Opposed to the usual expanding
|
||||
* from `input_symbol` to `node_symbol`.
|
||||
* @param lower_bound An optional indicator of the minimum number of edges
|
||||
* that get expanded (inclusive).
|
||||
* @param upper_bound An optional indicator of the maximum number of edges
|
||||
* that get expanded (inclusive).
|
||||
*/
|
||||
ExpandVariable(Symbol node_symbol, Symbol edge_symbol,
|
||||
EdgeAtom::Direction direction, Expression *lower_bound,
|
||||
Expression *upper_bound,
|
||||
EdgeAtom::Direction direction, bool is_reverse,
|
||||
Expression *lower_bound, Expression *upper_bound,
|
||||
const std::shared_ptr<LogicalOperator> &input,
|
||||
Symbol input_symbol, bool existing_node, bool existing_edge,
|
||||
GraphView graph_view = GraphView::AS_IS);
|
||||
@ -635,6 +638,9 @@ class ExpandVariable : public LogicalOperator, ExpandCommon {
|
||||
// both are optional, defaults are (1, inf)
|
||||
Expression *lower_bound_;
|
||||
Expression *upper_bound_;
|
||||
// True if the path should be written as expanding from node_symbol to
|
||||
// input_symbol.
|
||||
bool is_reverse_;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@ -877,6 +877,7 @@ LogicalOperator *PlanMatching(const Matching &matching,
|
||||
} else if (expansion.edge->has_range_) {
|
||||
last_op = new ExpandVariable(
|
||||
node_symbol, edge_symbol, expansion.direction,
|
||||
expansion.direction != expansion.edge->direction_,
|
||||
expansion.edge->lower_bound_, expansion.edge->upper_bound_,
|
||||
std::shared_ptr<LogicalOperator>(last_op), node1_symbol,
|
||||
existing_node, existing_edge, context.graph_view);
|
||||
|
||||
@ -153,7 +153,7 @@ TEST_F(QueryCostEstimator, Expand) {
|
||||
|
||||
TEST_F(QueryCostEstimator, ExpandVariable) {
|
||||
MakeOp<ExpandVariable>(NextSymbol(), NextSymbol(), EdgeAtom::Direction::IN,
|
||||
nullptr, nullptr, last_op_, NextSymbol(), false,
|
||||
false, nullptr, nullptr, last_op_, NextSymbol(), false,
|
||||
false);
|
||||
EXPECT_COST(CardParam::kExpandVariable * CostParam::kExpandVariable);
|
||||
}
|
||||
|
||||
@ -337,6 +337,9 @@ class QueryPlanExpandVariable : public testing::Test {
|
||||
* ops and plain Expand (depending on template param).
|
||||
* When creating plain Expand the bound arguments (lower, upper) are ignored.
|
||||
*
|
||||
* @param is_reverse Set to true if ExpandVariable should produce the list of
|
||||
* edges in reverse order. As if ExpandVariable starts from `node_to` and ends
|
||||
* with `node_from`.
|
||||
* @return the last created logical op.
|
||||
*/
|
||||
template <typename TExpansionOperator>
|
||||
@ -346,12 +349,13 @@ class QueryPlanExpandVariable : public testing::Test {
|
||||
std::experimental::optional<size_t> lower,
|
||||
std::experimental::optional<size_t> upper, Symbol edge_sym,
|
||||
bool existing_edge, const std::string &node_to,
|
||||
GraphView graph_view = GraphView::AS_IS) {
|
||||
GraphView graph_view = GraphView::AS_IS, bool is_reverse = false) {
|
||||
auto n_from = MakeScanAll(storage, symbol_table, node_from, input_op);
|
||||
auto filter_op = std::make_shared<Filter>(
|
||||
n_from.op_, storage.Create<query::LabelsTest>(
|
||||
n_from.node_->identifier_,
|
||||
std::vector<GraphDbTypes::Label>{labels[layer]}));
|
||||
n_from.op_,
|
||||
storage.Create<query::LabelsTest>(
|
||||
n_from.node_->identifier_,
|
||||
std::vector<GraphDbTypes::Label>{labels[layer]}));
|
||||
|
||||
auto n_to = NODE(node_to);
|
||||
auto n_to_sym = symbol_table.CreateSymbol(node_to, true);
|
||||
@ -363,8 +367,9 @@ class QueryPlanExpandVariable : public testing::Test {
|
||||
return bound ? LITERAL(static_cast<int64_t>(bound.value())) : nullptr;
|
||||
};
|
||||
return std::make_shared<ExpandVariable>(
|
||||
n_to_sym, edge_sym, direction, convert(lower), convert(upper),
|
||||
filter_op, n_from.sym_, false, existing_edge, graph_view);
|
||||
n_to_sym, edge_sym, direction, is_reverse, convert(lower),
|
||||
convert(upper), filter_op, n_from.sym_, false, existing_edge,
|
||||
graph_view);
|
||||
} else
|
||||
return std::make_shared<Expand>(n_to_sym, edge_sym, direction, filter_op,
|
||||
n_from.sym_, false, existing_edge,
|
||||
@ -404,43 +409,59 @@ class QueryPlanExpandVariable : public testing::Test {
|
||||
TEST_F(QueryPlanExpandVariable, OneVariableExpansion) {
|
||||
auto test_expand = [&](int layer, EdgeAtom::Direction direction,
|
||||
std::experimental::optional<size_t> lower,
|
||||
std::experimental::optional<size_t> upper) {
|
||||
std::experimental::optional<size_t> upper,
|
||||
bool reverse) {
|
||||
auto e = Edge("r", direction);
|
||||
return GetResults(AddMatch<ExpandVariable>(nullptr, "n", layer, direction,
|
||||
lower, upper, e, false, "m"),
|
||||
e);
|
||||
return GetResults(
|
||||
AddMatch<ExpandVariable>(nullptr, "n", layer, direction, lower, upper,
|
||||
e, false, "m", GraphView::AS_IS, reverse),
|
||||
e);
|
||||
};
|
||||
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::IN, 0, 0), (map_int{{0, 2}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, 0, 0), (map_int{{0, 2}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::BOTH, 0, 0), (map_int{{0, 2}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::IN, 1, 1), (map_int{}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, 1, 1), (map_int{{1, 4}}));
|
||||
EXPECT_EQ(test_expand(1, EdgeAtom::Direction::IN, 1, 1), (map_int{{1, 4}}));
|
||||
EXPECT_EQ(test_expand(1, EdgeAtom::Direction::OUT, 1, 1), (map_int{{1, 4}}));
|
||||
EXPECT_EQ(test_expand(1, EdgeAtom::Direction::BOTH, 1, 1), (map_int{{1, 8}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, 2, 2), (map_int{{2, 8}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, 2, 3), (map_int{{2, 8}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, 1, 2),
|
||||
(map_int{{1, 4}, {2, 8}}));
|
||||
for (int reverse = 0; reverse < 2; ++reverse) {
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::IN, 0, 0, reverse),
|
||||
(map_int{{0, 2}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, 0, 0, reverse),
|
||||
(map_int{{0, 2}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::BOTH, 0, 0, reverse),
|
||||
(map_int{{0, 2}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::IN, 1, 1, reverse),
|
||||
(map_int{}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, 1, 1, reverse),
|
||||
(map_int{{1, 4}}));
|
||||
EXPECT_EQ(test_expand(1, EdgeAtom::Direction::IN, 1, 1, reverse),
|
||||
(map_int{{1, 4}}));
|
||||
EXPECT_EQ(test_expand(1, EdgeAtom::Direction::OUT, 1, 1, reverse),
|
||||
(map_int{{1, 4}}));
|
||||
EXPECT_EQ(test_expand(1, EdgeAtom::Direction::BOTH, 1, 1, reverse),
|
||||
(map_int{{1, 8}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, 2, 2, reverse),
|
||||
(map_int{{2, 8}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, 2, 3, reverse),
|
||||
(map_int{{2, 8}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, 1, 2, reverse),
|
||||
(map_int{{1, 4}, {2, 8}}));
|
||||
|
||||
// the following tests also check edge-uniqueness (cyphermorphisim)
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::BOTH, 1, 2),
|
||||
(map_int{{1, 4}, {2, 12}}));
|
||||
EXPECT_EQ(test_expand(1, EdgeAtom::Direction::BOTH, 4, 4),
|
||||
(map_int{{4, 24}}));
|
||||
// the following tests also check edge-uniqueness (cyphermorphisim)
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::BOTH, 1, 2, reverse),
|
||||
(map_int{{1, 4}, {2, 12}}));
|
||||
EXPECT_EQ(test_expand(1, EdgeAtom::Direction::BOTH, 4, 4, reverse),
|
||||
(map_int{{4, 24}}));
|
||||
|
||||
// default bound values (lower default is 1, upper default is inf)
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, nullopt, 0), (map_int{}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, nullopt, 1),
|
||||
(map_int{{1, 4}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, nullopt, 2),
|
||||
(map_int{{1, 4}, {2, 8}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::BOTH, 7, nullopt),
|
||||
(map_int{{7, 24}, {8, 24}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::BOTH, 8, nullopt),
|
||||
(map_int{{8, 24}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::BOTH, 9, nullopt), (map_int{}));
|
||||
// default bound values (lower default is 1, upper default is inf)
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, nullopt, 0, reverse),
|
||||
(map_int{}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, nullopt, 1, reverse),
|
||||
(map_int{{1, 4}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::OUT, nullopt, 2, reverse),
|
||||
(map_int{{1, 4}, {2, 8}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::BOTH, 7, nullopt, reverse),
|
||||
(map_int{{7, 24}, {8, 24}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::BOTH, 8, nullopt, reverse),
|
||||
(map_int{{8, 24}}));
|
||||
EXPECT_EQ(test_expand(0, EdgeAtom::Direction::BOTH, 9, nullopt, reverse),
|
||||
(map_int{}));
|
||||
}
|
||||
}
|
||||
|
||||
TEST_F(QueryPlanExpandVariable, EdgeUniquenessSingleAndVariableExpansion) {
|
||||
|
||||
@ -211,4 +211,28 @@ TEST(TestVariableStartPlanner, MatchWithMatchReturn) {
|
||||
});
|
||||
}
|
||||
|
||||
TEST(TestVariableStartPlanner, MatchVariableExpand) {
|
||||
Dbms dbms;
|
||||
auto dba = dbms.active();
|
||||
// Graph (v1) -[:r1]-> (v2) -[:r2]-> (v3)
|
||||
auto v1 = dba->InsertVertex();
|
||||
auto v2 = dba->InsertVertex();
|
||||
auto v3 = dba->InsertVertex();
|
||||
auto r1 = dba->InsertEdge(v1, v2, dba->EdgeType("r1"));
|
||||
auto r2 = dba->InsertEdge(v2, v3, dba->EdgeType("r2"));
|
||||
dba->AdvanceCommand();
|
||||
// Test MATCH (n) -[r*]-> (m) RETURN r
|
||||
AstTreeStorage storage;
|
||||
auto edge = EDGE("r", Direction::OUT);
|
||||
edge->has_range_ = true;
|
||||
QUERY(MATCH(PATTERN(NODE("n"), edge, NODE("m"))), RETURN("r"));
|
||||
// We expect to get a single column with the following rows:
|
||||
TypedValue r1_list(std::vector<TypedValue>{r1}); // [r1]
|
||||
TypedValue r2_list(std::vector<TypedValue>{r2}); // [r2]
|
||||
TypedValue r1_r2_list(std::vector<TypedValue>{r1, r2}); // [r1, r2]
|
||||
CheckPlansProduce(2, storage, *dba, [&](const auto &results) {
|
||||
AssertRows(results, {{r1_list}, {r2_list}, {r1_r2_list}});
|
||||
});
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
Loading…
Reference in New Issue
Block a user