tursom/memgraph
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Variable expansion consolidaton

Browse Source 
Summary:
- Removed BreadthFirstAtom, using EdgeAtom only with a Type enum.
- Both variable expansions (breadth and depth first) now have mandatory inner node and edge Identifiers.
- Both variable expansions use inline property filtering and support inline lambdas.
- BFS and variable expansion now have the same planning process.
- Planner modified in the following ways:
	- Variable expansions support inline property filtering (two filters added to all_filters, one for inline, one for post-expand).
	- Asserting against existing_edge since we don't support that anymore.
	- Edge and node symbols bound after variable expansion to disallow post-expand filters to get inlined.
	- Some things simplified due to different handling.
- BreadthFirstExpand logical operator merged into ExpandVariable. Two Cursor classes remain and are dynamically chosen from.

As part of planned planner refactor we should ensure that a filter is applied only once. The current implementation is very suboptimal for property filtering in variable expansions.

@buda: we will start refactoring this these days. This current planner logic is too dense and complex. It is becoming technical debt. Most of the time I spent working on this has been spent figuring the planning out, and I still needed Teon's help at times. Implementing the correct and optimal version of query execution (avoiding multiple potentially expensive filterings) was out of reach also due to tech debt.

Reviewers: buda, teon.banek

Reviewed By: teon.banek

Subscribers: pullbot, buda

Differential Revision: https://phabricator.memgraph.io/D852
This commit is contained in:
florijan
2017-10-05 11:25:52 +02:00
parent d249ff11a5
commit 76fe8bfadf
26 changed files with 766 additions and 1103 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
tests/unit/query_plan_create_set_remove_delete.cpp
View File

@@ -302,7 +302,7 @@ TEST(QueryPlan, Delete) {
{
auto n = MakeScanAll(storage, symbol_table, "n");
auto r_m = MakeExpand(storage, symbol_table, n.op_, n.sym_, "r",
EdgeAtom::Direction::OUT, {}, false, "m", false);
EdgeAtom::Direction::OUT, {}, "m", false);
auto r_get = storage.Create<Identifier>("r");
symbol_table[*r_get] = r_m.edge_sym_;
auto delete_op = std::make_shared<plan::Delete>(
@@ -355,7 +355,7 @@ TEST(QueryPlan, DeleteTwiceDeleteBlockingEdge) {
auto n = MakeScanAll(storage, symbol_table, "n");
auto r_m = MakeExpand(storage, symbol_table, n.op_, n.sym_, "r",
EdgeAtom::Direction::BOTH, {}, false, "m", false);
EdgeAtom::Direction::BOTH, {}, "m", false);
// getter expressions for deletion
auto n_get = storage.Create<Identifier>("n");
@@ -478,7 +478,7 @@ TEST(QueryPlan, SetProperty) {
// scan (n)-[r]->(m)
auto n = MakeScanAll(storage, symbol_table, "n");
auto r_m = MakeExpand(storage, symbol_table, n.op_, n.sym_, "r",
EdgeAtom::Direction::OUT, {}, false, "m", false);
EdgeAtom::Direction::OUT, {}, "m", false);
// set prop1 to 42 on n and r
auto prop1 = dba->Property("prop1");
@@ -529,7 +529,7 @@ TEST(QueryPlan, SetProperties) {
// scan (n)-[r]->(m)
auto n = MakeScanAll(storage, symbol_table, "n");
auto r_m = MakeExpand(storage, symbol_table, n.op_, n.sym_, "r",
EdgeAtom::Direction::OUT, {}, false, "m", false);
EdgeAtom::Direction::OUT, {}, "m", false);
auto op = update ? plan::SetProperties::Op::UPDATE
: plan::SetProperties::Op::REPLACE;
@@ -632,7 +632,7 @@ TEST(QueryPlan, RemoveProperty) {
// scan (n)-[r]->(m)
auto n = MakeScanAll(storage, symbol_table, "n");
auto r_m = MakeExpand(storage, symbol_table, n.op_, n.sym_, "r",
EdgeAtom::Direction::OUT, {}, false, "m", false);
EdgeAtom::Direction::OUT, {}, "m", false);
auto n_p = PROPERTY_LOOKUP("n", prop1);
symbol_table[*n_p->expression_] = n.sym_;
@@ -708,9 +708,8 @@ TEST(QueryPlan, NodeFilterSet) {
// MATCH (n {prop: 42}) -[r]- (m)
auto scan_all = MakeScanAll(storage, symbol_table, "n");
scan_all.node_->properties_[prop] = LITERAL(42);
auto expand =
MakeExpand(storage, symbol_table, scan_all.op_, scan_all.sym_, "r",
EdgeAtom::Direction::BOTH, {}, false, "m", false);
auto expand = MakeExpand(storage, symbol_table, scan_all.op_, scan_all.sym_,
"r", EdgeAtom::Direction::BOTH, {}, "m", false);
auto *filter_expr =
EQ(storage.Create<PropertyLookup>(scan_all.node_->identifier_, prop),
LITERAL(42));
@@ -748,9 +747,8 @@ TEST(QueryPlan, FilterRemove) {
// MATCH (n) -[r]- (m) WHERE n.prop < 43
auto scan_all = MakeScanAll(storage, symbol_table, "n");
scan_all.node_->properties_[prop] = LITERAL(42);
auto expand =
MakeExpand(storage, symbol_table, scan_all.op_, scan_all.sym_, "r",
EdgeAtom::Direction::BOTH, {}, false, "m", false);
auto expand = MakeExpand(storage, symbol_table, scan_all.op_, scan_all.sym_,
"r", EdgeAtom::Direction::BOTH, {}, "m", false);
auto filter_prop = PROPERTY_LOOKUP("n", prop);
symbol_table[*filter_prop->expression_] = scan_all.sym_;
auto filter =
@@ -813,7 +811,7 @@ TEST(QueryPlan, Merge) {
// merge_match branch
auto r_m = MakeExpand(storage, symbol_table, std::make_shared<Once>(), n.sym_,
"r", EdgeAtom::Direction::BOTH, {}, false, "m", false);
"r", EdgeAtom::Direction::BOTH, {}, "m", false);
auto m_p = PROPERTY_LOOKUP("m", prop);
symbol_table[*m_p->expression_] = r_m.node_sym_;
auto m_set = std::make_shared<plan::SetProperty>(r_m.op_, m_p, LITERAL(1));