Summary:
This is still very much in progress. No advanced checks are done to
prevent planning unimplemented things. Basic Cartesian product should
work, for example `MATCH (a), (b) CREATE (a)-[:r]->(c)-[:r]->(b)`. But
anything more advanced may lead to undefined behaviour of the planner
and therefore execution. Use at your own risk!
Add ModifiedSymbols method to LogicalOperator
For planning Cartesian, we need information on which symbols are filled
by operator sub-trees. Currently, this is used to set symbols which
should be transferred over network. Later, they should be used to detect
whether filter expressions use symbols modified from Cartesian branches.
Then we will be able to ensure correct dependency of filters and their
behaviour.
Prepare DistributedPlan for multiple worker plans
Since Cartesian branches need to be split and handled by each worker, we
now dispatch multiple plans to workers.
Reviewers: florijan, msantl, buda
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1208
Summary:
On the master cleanups are hooked directly into the transaction engine.
This is beneficial because the master might have bigger caches and we
want to clear them as soon as possible.
On the workers there is a periodic RPC call to the master about living
transactions, which takes care of releasing local caches. This is
suboptimal because long transactions will prevent cache GC (like with
data GC). It is however fairly simple.
Note that all cleanup is not done automatically and `RemotePull` has
been reduced accordingly. @msantl, please verify correctness and
consider if the code can be additionally simplified.
Reviewers: teon.banek, msantl
Reviewed By: msantl
Subscribers: pullbot, msantl
Differential Revision: https://phabricator.memgraph.io/D1202
Summary:
Pulls left op cursor and keeps the result, and then for each pull of
the right op cursor, adds all the left op results to produce a cartesian
product.
Reviewers: teon.banek, florijan
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1201
Summary:
Added a hidden flag, a int32 value, that represents the number of
milliseconds the thread that pulls remote results should sleep in case when the
local results are exhausted but there are still some remote results that are
pending.
Reviewers: teon.banek, florijan
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1198
Summary:
The same behaviour can now be achieved by passing a nullptr for input
operator.
Reviewers: florijan, msantl
Reviewed By: msantl
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1195
Summary:
This is a basic take on planning distributed writes. Main logic is in handling
the Accumulate operator, which requires Synchronize operation if the results
are used after modification.
Tests for planning distributed writes have been added.
Reviewers: florijan, msantl
Reviewed By: msantl
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1172
Summary:
This avoids the unnecessary work of storing symbols which can never be
read after an aggregation is complete. With regards to distributed, a
major benefit is gained in reducing what is transferred over the
network. Hopefully, this doesn't break some obscure case where we
actually needed to remember all used symbols.
Reviewers: florijan, mislav.bradac, msantl
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1158
Summary:
Queries such as `RETURN 1` should only be run on a single machine. This
change assumes that a query should only be distributed if it contains at
least one `ScanAll` operator, i.e. a `MATCH` clause.
Reviewers: florijan, msantl
Reviewed By: msantl
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1154
Summary:
Remote pulls can now be async. Note that
`RemotePullRpcClients::RemotePull` takes references to data structures
which should not be temporary in the caller. Still, maybe safer to make
copies?
Changed `RpcWorkerClients` API to make that possible.
Reviewers: dgleich, msantl, teon.banek
Reviewed By: msantl
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1150
Summary: PullRemoteCursor will pull all clients in a RoundRobin fashion until all clients are exhausted and there are no more results to return.
Reviewers: teon.banek, florijan
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1147
Summary:
Remove AdvanceCommand operator declaration.
Add query/plan/distributed source files.
Add hacked cloning of LogicalOperator via serialization.
Add virtual Default methods to CompositeVisitor.
Use BOOST_CLASS_EXPORT_KEY in ast.hpp and operator.hpp.
This is needed in a single binary to correctly serialize polymorphic
classes. The previous implementation worked because the memgraph_lib was
linked with test binaries, but nobody was actually serializing things
inside the single memgraph binary itself.
Print PullRemote symbols in tests/manual/query_planner
Add names to implicitly created aggregation symbols
Reviewers: florijan, msantl
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1137
Summary:
With the added support for serialization, we should be able to transfer
plans across distributed workers.
The planner tests has been extended to test serialization. Operators
should be mostly tested, but the expression they contain aren't
completely. The quick solution is to use typeid for rudimentary
expression equality testing. The more involved solution of comparing
the expression tree for equality is the correct choice. It should be
done in the near future.
Reviewers: florijan, msantl
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1122
Summary:
Previously, we would have a `DCHECK` which crashes the application. This
was evident when testing a queries, such as:
MATCH (n) DELETE n SET n.prop = 42
Since the argument to update clauses is evaluated during execution, it
makes it very difficult to prevent such errors during semantic analysis.
For example:
MATCH (n)--(m) WITH collect(n) as ns, m
DETACH DELETE ns[m.prop] SET head(ns).prop = 42
Test query updates on deleted graph elements
Reviewers: florijan, dgleich
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1114
Summary:
GraphDb is refactored to become an API exposing different parts
necessary for the database to function. These different parts can have
different implementations in SingleNode or distributed Master/Server
GraphDb implementations.
Interally GraphDb is implemented using two class heirarchies. One
contains all the members and correct wiring for each situation. The
other takes care of initialization and shutdown. This architecture is
practical because it can guarantee that the initialization of the
object structure is complete, before initializing state.
Reviewers: buda, mislav.bradac, dgleich, teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1093
Summary:
Union query combinator implementation consists of:
* adjustments to the AST and `cypher_main_visitor`
* enabling `QueryStripper` to parse multiple `return` statements (not stopping after first)
* symbol generation for union results
* union logical operator
* query plan generator adjustments
Reviewers: teon.banek, mislav.bradac
Reviewed By: teon.banek
Subscribers: pullbot, buda
Differential Revision: https://phabricator.memgraph.io/D1038
Summary:
What's done:
- `RecordAccessor` can represent remote data
- `GraphDbAccessor` manages remote data
- Cleanup: different `EdgeAccessor lazyness (@dgleich: take a look), unused methods, documentation...
- `TODO` placeholders for remote implementation
What's not done:
- RPC and data transfer
- how exactly remote errors are handled
- not sure if any MVCC Record info for remote data should be tracked
- WAL and RPC Deltas properly handled (Gleich working on extracting `Wal::Op`)
This implementation should not break single-node execution, and should provide good abstractions and placeholders for distributed. Once that's satisfied, it should land.
Reviewers: dgleich, buda, mislav.bradac
Reviewed By: dgleich
Subscribers: dgleich, pullbot
Differential Revision: https://phabricator.memgraph.io/D1030
Summary:
This change generates multiple PropertyFilters for expressions such as
`n.prop1 = m.prop2`. When choosing one PropertyFilter, we want to also
remove the other one, because they represent the same original
expression. Therefore, the removal is no longer based on FilterInfo
equality, but on the original expression equality. Additionally,
FilterInfo and PropertyFilter equality operators have been removed to
avoid any pretense they do what you expect or want.
Reviewers: florijan, msantl
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1021
Summary:
Referring to the TCK failure on:
```
MATCH (a {name: 'Andres'})<-[:FATHER]-(child)
RETURN {foo: a.name='Andres', kids: collect(child.name)}
```
In the planner we'd only treat a list|map as a group_by if it contained
no aggregations. That's changed so that if a map contains both aggregations
and non-aggregations, then non-aggregations are treated as individual
group_by expressions.
Reviewers: teon.banek
Reviewed By: teon.banek
Subscribers: buda, pullbot, teon.banek
Differential Revision: https://phabricator.memgraph.io/D1004
Summary:
In preparation for distributed storage we need to have labels/properties/edgetypes uniquely identifiable by their ids, which will be global in near future.
The old design has to be abandoned because it's not possible to keep track of global labels/properties/edgetypes while they are local pointers.
Reviewers: mislav.bradac, florijan
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D993
Summary:
Previously, named path symbols remained untracked as `new_symbols` during planning. This meant that
operator `Optional` would be left unaware of those symbols, and therefore not reset them to `Null`
if optional matching failed.
Test Optional operator will be aware of path symbols
Reviewers: florijan, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D974
Summary:
Warnings I ignored:
Creates a new stacktrace object and then dumps it
102.570384 (102.495075) E[1]: [src/utils/exceptions.hpp:116]: (performance) Variable 'stacktrace_' is assigned in constructor body. Consider performing initialization in initialization list.
102.570390 (102.495081) E[1]: [src/utils/exceptions.hpp:127]: (performance) Variable 'stacktrace_' is assigned in constructor body. Consider performing initialization in initialization list.
Used all over the codebase without explicit cast
102.570412 (102.495103) E[1]: [src/utils/stacktrace.hpp:14]: (style) Class 'Line' has a constructor with 1 argument that is not explicit.
Not really used anywhere before initialized:
102.570526 (102.495217) E[1]: [src/data_structures/concurrent/skiplist.hpp:467]: (warning) Member variable 'Accessor::preds' is not initialized in the constructor.
102.570530 (102.495221) E[1]: [src/data_structures/concurrent/skiplist.hpp:467]: (warning) Member variable 'Accessor::succs' is not initialized in the constructor.
Implicit conversions between types are used all over the codebase:
102.570548 (102.495239) E[1]: [src/storage/property_value.hpp:41]: (style) Class 'PropertyValue' has a constructor with 1 argument that is not explicit.
102.570552 (102.495243) E[1]: [src/storage/property_value.hpp:42]: (style) Class 'PropertyValue' has a constructor with 1 argument that is not explicit.
102.570557 (102.495248) E[1]: [src/storage/property_value.hpp:43]: (style) Class 'PropertyValue' has a constructor with 1 argument that is not explicit.
102.570561 (102.495252) E[1]: [src/storage/property_value.hpp:44]: (style) Class 'PropertyValue' has a constructor with 1 argument that is not explicit.
102.570566 (102.495257) E[1]: [src/storage/property_value.hpp:47]: (style) Class 'PropertyValue' has a constructor with 1 argument that is not explicit.
102.570570 (102.495261) E[1]: [src/storage/property_value.hpp:50]: (style) Class 'PropertyValue' has a constructor with 1 argument that is not explicit.
102.570578 (102.495269) E[1]: [src/storage/property_value.hpp:53]: (style) Class 'PropertyValue' has a constructor with 1 argument that is not explicit.
102.570582 (102.495273) E[1]: [src/storage/property_value.hpp:57]: (style) Class 'PropertyValue' has a constructor with 1 argument that is not explicit.
102.570591 (102.495282) E[1]: [src/query/typed_value.hpp:80]: (style) Class 'TypedValue' has a constructor with 1 argument that is not explicit.
102.570596 (102.495287) E[1]: [src/query/typed_value.hpp:81]: (style) Class 'TypedValue' has a constructor with 1 argument that is not explicit.
102.570601 (102.495292) E[1]: [src/query/typed_value.hpp:82]: (style) Class 'TypedValue' has a constructor with 1 argument that is not explicit.
102.570605 (102.495296) E[1]: [src/query/typed_value.hpp:83]: (style) Class 'TypedValue' has a constructor with 1 argument that is not explicit.
102.570609 (102.495300) E[1]: [src/query/typed_value.hpp:89]: (style) Class 'TypedValue' has a constructor with 1 argument that is not explicit.
102.570614 (102.495305) E[1]: [src/query/typed_value.hpp:92]: (style) Class 'TypedValue' has a constructor with 1 argument that is not explicit.
102.570618 (102.495309) E[1]: [src/query/typed_value.hpp:95]: (style) Class 'TypedValue' has a constructor with 1 argument that is not explicit.
102.570623 (102.495314) E[1]: [src/query/typed_value.hpp:98]: (style) Class 'TypedValue' has a constructor with 1 argument that is not explicit.
102.570627 (102.495318) E[1]: [src/query/typed_value.hpp:102]: (style) Class 'TypedValue' has a constructor with 1 argument that is not explicit.
102.570632 (102.495323) E[1]: [src/query/typed_value.hpp:105]: (style) Class 'TypedValue' has a constructor with 1 argument that is not explicit.
102.570636 (102.495327) E[1]: [src/query/typed_value.hpp:108]: (style) Class 'TypedValue' has a constructor with 1 argument that is not explicit.
102.570641 (102.495332) E[1]: [src/query/typed_value.hpp:109]: (style) Class 'TypedValue' has a constructor with 1 argument that is not explicit.
102.570645 (102.495336) E[1]: [src/communication/bolt/v1/decoder/decoded_value.hpp:88]: (style) Class 'DecodedValue' has a constructor with 1 argument that is not explicit.
102.570650 (102.495341) E[1]: [src/communication/bolt/v1/decoder/decoded_value.hpp:89]: (style) Class 'DecodedValue' has a constructor with 1 argument that is not explicit.
102.570654 (102.495345) E[1]: [src/communication/bolt/v1/decoder/decoded_value.hpp:90]: (style) Class 'DecodedValue' has a constructor with 1 argument that is not explicit.
102.570659 (102.495350) E[1]: [src/communication/bolt/v1/decoder/decoded_value.hpp:91]: (style) Class 'DecodedValue' has a constructor with 1 argument that is not explicit.
102.570663 (102.495354) E[1]: [src/communication/bolt/v1/decoder/decoded_value.hpp:94]: (style) Class 'DecodedValue' has a constructor with 1 argument that is not explicit.
102.570668 (102.495359) E[1]: [src/communication/bolt/v1/decoder/decoded_value.hpp:97]: (style) Class 'DecodedValue' has a constructor with 1 argument that is not explicit.
102.570672 (102.495363) E[1]: [src/communication/bolt/v1/decoder/decoded_value.hpp:100]: (style) Class 'DecodedValue' has a constructor with 1 argument that is not explicit.
102.570677 (102.495368) E[1]: [src/communication/bolt/v1/decoder/decoded_value.hpp:104]: (style) Class 'DecodedValue' has a constructor with 1 argument that is not explicit.
102.570681 (102.495372) E[1]: [src/communication/bolt/v1/decoder/decoded_value.hpp:107]: (style) Class 'DecodedValue' has a constructor with 1 argument that is not explicit.
102.570690 (102.495381) E[1]: [src/communication/bolt/v1/decoder/decoded_value.hpp:110]: (style) Class 'DecodedValue' has a constructor with 1 argument that is not explicit.
102.570694 (102.495385) E[1]: [src/communication/bolt/v1/decoder/decoded_value.hpp:113]: (style) Class 'DecodedValue' has a constructor with 1 argument that is not explicit.
CypherParser:
102.570767 (102.495458) E[1]: [src/query/frontend/opencypher/generated/CypherParser.h:69]: (style) Class 'CypherParser' has a constructor with 1 argument that is not explicit.
102.570772 (102.495463) E[1]: [src/query/frontend/opencypher/generated/CypherLexer.h:40]: (style) Class 'CypherLexer' has a constructor with 1 argument that is not explicit.
102.570776 (102.495467) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:86]: (style) The scope of the variable '_la' can be reduced.
102.570781 (102.495472) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:311]: (style) The scope of the variable '_la' can be reduced.
102.570785 (102.495476) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:402]: (style) The scope of the variable '_la' can be reduced.
102.570789 (102.495480) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:497]: (style) The scope of the variable '_la' can be reduced.
102.570797 (102.495488) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:778]: (style) The scope of the variable '_la' can be reduced.
102.570802 (102.495493) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:895]: (style) The scope of the variable '_la' can be reduced.
102.570806 (102.495497) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:991]: (style) The scope of the variable '_la' can be reduced.
102.570811 (102.495502) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:1190]: (style) The scope of the variable '_la' can be reduced.
102.570815 (102.495506) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:1274]: (style) The scope of the variable '_la' can be reduced.
102.570820 (102.495511) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:1393]: (style) The scope of the variable '_la' can be reduced.
102.570824 (102.495515) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:1570]: (style) The scope of the variable '_la' can be reduced.
102.570829 (102.495520) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:1695]: (style) The scope of the variable '_la' can be reduced.
102.570834 (102.495525) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:1800]: (style) The scope of the variable '_la' can be reduced.
102.570839 (102.495530) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:1903]: (style) The scope of the variable '_la' can be reduced.
102.570843 (102.495534) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:2019]: (style) The scope of the variable '_la' can be reduced.
102.570848 (102.495539) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:2228]: (style) The scope of the variable '_la' can be reduced.
102.570852 (102.495543) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:2542]: (style) The scope of the variable '_la' can be reduced.
102.570857 (102.495548) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:2797]: (style) The scope of the variable '_la' can be reduced.
102.570861 (102.495552) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:2966]: (style) The scope of the variable '_la' can be reduced.
102.570866 (102.495557) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:3067]: (style) The scope of the variable '_la' can be reduced.
102.570870 (102.495561) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:3289]: (style) The scope of the variable '_la' can be reduced.
102.570875 (102.495566) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:3295]: (style) The scope of the variable 'alt' can be reduced.
102.570879 (102.495570) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:3419]: (style) The scope of the variable '_la' can be reduced.
102.570884 (102.495575) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:3596]: (style) The scope of the variable '_la' can be reduced.
102.570888 (102.495579) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:3688]: (style) The scope of the variable '_la' can be reduced.
102.570893 (102.495584) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:3963]: (style) The scope of the variable '_la' can be reduced.
102.570897 (102.495588) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:4452]: (style) The scope of the variable '_la' can be reduced.
102.570902 (102.495593) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:4586]: (style) The scope of the variable '_la' can be reduced.
102.570906 (102.495597) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:4813]: (style) The scope of the variable '_la' can be reduced.
102.570911 (102.495602) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:4943]: (style) The scope of the variable '_la' can be reduced.
102.570918 (102.495609) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:5026]: (style) The scope of the variable '_la' can be reduced.
102.570923 (102.495614) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:5569]: (style) The scope of the variable '_la' can be reduced.
102.570928 (102.495619) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:5664]: (style) The scope of the variable '_la' can be reduced.
102.570932 (102.495623) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:5755]: (style) The scope of the variable '_la' can be reduced.
102.570937 (102.495628) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:5888]: (style) The scope of the variable '_la' can be reduced.
102.570941 (102.495632) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:6045]: (style) The scope of the variable '_la' can be reduced.
102.570946 (102.495637) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:6142]: (style) The scope of the variable '_la' can be reduced.
102.570950 (102.495641) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:6347]: (style) The scope of the variable '_la' can be reduced.
102.570955 (102.495646) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:6523]: (style) The scope of the variable '_la' can be reduced.
102.570959 (102.495650) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:6614]: (style) The scope of the variable '_la' can be reduced.
102.570964 (102.495655) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:6899]: (style) The scope of the variable '_la' can be reduced.
102.570968 (102.495659) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:6992]: (style) The scope of the variable '_la' can be reduced.
102.570973 (102.495664) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:7147]: (style) The scope of the variable '_la' can be reduced.
102.570977 (102.495668) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:7680]: (style) The scope of the variable '_la' can be reduced.
102.570982 (102.495673) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:7759]: (style) The scope of the variable '_la' can be reduced.
102.570986 (102.495677) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:7938]: (style) The scope of the variable '_la' can be reduced.
102.570991 (102.495682) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:8126]: (style) The scope of the variable '_la' can be reduced.
102.570995 (102.495686) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:8220]: (style) The scope of the variable '_la' can be reduced.
102.571000 (102.495691) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:8313]: (style) The scope of the variable '_la' can be reduced.
102.571004 (102.495695) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:8491]: (style) The scope of the variable '_la' can be reduced.
102.571009 (102.495700) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:8703]: (style) The scope of the variable '_la' can be reduced.
102.571013 (102.495704) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:8783]: (style) The scope of the variable '_la' can be reduced.
102.571018 (102.495709) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:8914]: (style) The scope of the variable '_la' can be reduced.
102.571022 (102.495713) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:9119]: (style) The scope of the variable '_la' can be reduced.
102.571027 (102.495718) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:9220]: (style) The scope of the variable '_la' can be reduced.
102.571034 (102.495725) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:9414]: (style) The scope of the variable '_la' can be reduced.
102.571039 (102.495730) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:9660]: (style) The scope of the variable '_la' can be reduced.
102.571043 (102.495734) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:10008]: (style) The scope of the variable '_la' can be reduced.
102.571048 (102.495739) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:10158]: (style) The scope of the variable '_la' can be reduced.
102.571052 (102.495743) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:10250]: (style) The scope of the variable '_la' can be reduced.
102.571057 (102.495748) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:10370]: (style) The scope of the variable '_la' can be reduced.
102.571061 (102.495752) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:10637]: (style) The scope of the variable '_la' can be reduced.
102.571065 (102.495756) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:10749]: (style) The scope of the variable '_la' can be reduced.
102.571070 (102.495761) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:10815]: (style) The scope of the variable '_la' can be reduced.
102.571075 (102.495766) E[1]: [src/query/frontend/opencypher/generated/CypherParser.cpp:10881]: (style) The scope of the variable '_la' can be reduced.
We know that we represented it correctly in memory:
102.571079 (102.495770) E[1]: [src/communication/bolt/v1/decoder/decoder.hpp:252]: (portability) Casting between integer* and double* which have an incompatible binary data representation.
Cont assigned but not used after:
102.571101 (102.495792) E[1]: [src/query/frontend/ast/ast.hpp:1008]: (style) Variable 'cont' is assigned a value that is never used.
Reviewers: teon.banek, buda, mferencevic
Reviewed By: teon.banek
Subscribers: mferencevic, pullbot
Differential Revision: https://phabricator.memgraph.io/D967
Summary:
Move QueryParts and Filters to a new file.
Reorganize FilterInfo struct.
Remove label filter if we do indexed scan by label.
Remove property filter used in indexed scan.
Reviewers: florijan
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D915
Summary: This change increases the planning time, but should reduce memory consumption.
Reviewers: florijan, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D901
Summary: This is not a very important functionality, but it turned out simple to do, so let's add it to have a consistent query support.
Reviewers: buda, teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D862
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
Summary:
- The new BFS syntax implemented as proposed.
- AST BreadthFirstAtom now uses EdgeAtom members: has_range_{true}, upper_bound_, lower_bound_
- Edges data structure now handles all the edge filtering (single or multiple edges), to ease planning. Additional edge filtering (additional Filter op in the plan) is removed. AST EdgeTypeTest is no longer used and is removed.
Current state is stable but there are things left to do:
- BFS property filtering.
- BFS lower_bound_ support.
- Support for lambdas in variable length expansion. This includes obligatory (even if not user_defined) inner_node and inner_edge symbols for easier handling.
- Code-sharing between BFS and variable length expansions.
I'll add asana tasks (and probably start working on them immediately) when/if this lands.
Reviewers: buda, teon.banek, mislav.bradac
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D836
Summary: - modified all utils/algorithm functions to be inline and in the utils namespace
Reviewers: teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D830
Summary:
Test planner splits MATCH ... WHERE
Remove distinction between FilterAnd and AndOperator
Reviewers: florijan, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D814
Summary:
Three TODOs resolved.
1. around line 897 - we currently don't support expansion into existing variable length edges (there is a TODO in symbol_generator.cpp:213), so this should not be done at the moment.
2. around line 1025 - This TODO was on review and nobody commented, so I'm removing it. Should have done that when the diff landed.
3. around line 1560 - This does not seem possible. Edge-uniqueness checks happen within a single `[OPTIONAL ] MATCH`. If it is OPTIONAL (the case interesting here), then the uniqueness check also gets planned under the optional branch. So, if an optional fails, the uniqueness check will get skipped, as opposed to getting executed over a Null. I added an edge-case test to verify this (and checked with the planner test).
Reviewers: buda, teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D807
Summary:
This is done when the generated AST will be cached.
Remove LiteralsPlugger.
Reviewers: florijan, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D788
Summary:
Replaced std::list with std::vector in all plan operators. Performance increase in harness tests is not visible. Defined a custom test:
```
unwind range(0, 1000000) as x
create ({a: tointeger(rand() * 100), b: tointeger(rand() * 100), c: tointeger(rand() * 100), d: tointeger(rand() * 10), e: tointeger(rand() * 10), f: tointeger(rand() * 10)});
match (n) return min(n.a), max(n.b), sum(n.c), n.d, n.e, n.f
match (n) with distinct n.a AS a, n.b AS b, n.c AS c, n.d AS d, n.e AS e, n.f AS f return count(*)
```
In that test performance gains are 9.8% on the aggregation query (mean 0.83s vs 092s) and 34% (mean 2.15s vs 3.25s) on the distinct query. Doubt we'll see much on any of the LDBC tests because they don't stress those operators nearly as much.
Reviewers: buda, teon.banek, mislav.bradac
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D778
Summary:
Antlr grammar has been updated to support putting edge types after the BFS
symbol. Planner collects edge type filters for BFS and inlines them in the
operator by joining the filter with the user input BFS filter itself. This
requires no change from the standpoint of the operator. On the other hand, in
order to use the faster lookup by a single edge type, `ExpandBreadthFirst`
operator now accept an optional edge type. The edge type is passed from the
planner only if the user is filtering by a single type.
Unit tests as well as tck have been updated.
Reviewers: florijan, mislav.bradac
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D777
Summary: Reduces latency on LDBC query 9 from 7.9sec to 6.8sec (14%). That query has 650k rows in ORDER BY, 3 ordering elements and 10ish values get returned (both of them are now accumulated into vectors).
Reviewers: buda, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D775
Summary:
Add function First to utils.
Insert EdgeType into Expand during planning.
Reviewers: florijan, mislav.bradac
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D769
Summary: The constness of the DbAccessor interferes with caching the results.
Reviewers: florijan, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D771
Summary:
Benchmark planning and estimating indexed ScanAll. According to the benchmark,
caching speeds up the whole process of planning and estimation by a factor of
2. Most of the performance gain is in the `CostEstimator` itself, due to plenty
of calls to `VerticesCount` when estimating all of the generated plans.
Reviewers: mislav.bradac, florijan
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D765
Summary:
- The `Edges` data structure now handles common ops, including providing an iterator over edges whose "other" vertex is know.
- This should improve performance on dense_expand tests in the harness without other side-effects.
- query::plan::Expand operator modified not to check for existing-node stuff since that now gets handled by the `Edges` data structure.
- `Edges::Iterator` implemented only for const iterators since that suffices for now. Can implement non-const if the need arrises.
Reviewers: buda, mislav.bradac, teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D763
Summary:
This allows for inserting dummy DbAccessor in tests. Unfortunate side
effect of this change is that the whole implementation had to be moved
from cpp to hpp.
Also templatize remaining RuleBasedPlanner implementation
Reviewers: florijan, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D704
Summary:
Added:
- map support in PropertyValue
- conversion of map TypedValue to PropertyValue if appropriate flag is set (undocumented because it's private)
- ordering of map PropertyValue in LabelPropertyIndex
- issue raised regarding list and value property modifications in storage (currently unsupported)
Maybe I missed some feature or whatever?
Reviewers: mislav.bradac, buda, teon.banek
Reviewed By: mislav.bradac, buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D692
Summary:
This change should allow for passing a different PlanningContext and/or
GraphDbAccessor. In turn, we can write tests which pass a dummy context
for decoupled testing of the planning process (from the rest of the
system).
Reviewers: mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D700
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
Summary:
Matching variants are generated iteratively and limited. Additionally,
When generating expansions, if there is nothing to continue on the
expanded nodes, simply append all the remaining expansions to the
currently generated one. This should speed up queries with large number
of `MATCH` clauses.
Reviewers: mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D612
Summary: Not strictly neccessary, but it's been itching me. It took an hour.
Reviewers: buda, mislav.bradac
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D648
Summary:
Now all ScanAll and Expand ops are covered by the cost estimator. For ScanAll with indices cost estimation is pretty good, for new Expand ops it is tragically bad (Expand to the power of expansion depth, plus arbitrary filtering). Static cost estimation is wrong wrong wrong.
Currently cost estimation of even trivial plans that use indices is wrong because the planner leaves filtering expressions that are implicitly handled by the index in the operator tree, IIRC. Tasking Teon to revise this, even though I'm not sure how bad an influence this has on cost estimation and it's use in plan choosing.
Reviewers: mislav.bradac, teon.banek, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D633
Summary:
This diff contains a bug fix for the expansion operators that are currently on dev.
More importantly, it proposes end-to-end testing for edge-cases for which it's a
pain to write single-phase tests. In my opinion this is OK, you're all reviewers so
you can comment.
The test relies on left-to-right query execution. We need this guarantee in tests
like this. I propose renaming "RuleBasedPlanner" to "LeftToRightPlanner" to make
this explicit. As Teon is not here at the moment, will make this a task/discussion.
Reviewers: buda, mislav.bradac, teon.banek, lion
Reviewed By: mislav.bradac
Subscribers: mferencevic, pullbot
Differential Revision: https://phabricator.memgraph.io/D626
Summary:
Not complete (but review can start):
- implementation should be done
- still need to finish tests
- documentation missing
Reviewers: mislav.bradac, teon.banek, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D616
Summary:
`UNWIND` can come before `MATCH`, so it needs to break query parts. If
it didn't, a query part would incorrectly grab all the matches and plan
them incorrectly. A test for such a case has been added.
Reviewers: florijan
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D598
Summary:
Add All expression to Ast
Evaluate All expression
Visit All and generate symbols
Handle All when collecting context during planning
Reviewers: florijan, mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D587
Summary:
Variable expansion logical operator added. Some functionalities are missing:
- taking into account optional matching when expanding into existing symbol
- accepting Expression bounds (current implementation takes size_t)
Also, a TODO is added for handling optional matching in the uniqueness operator (with an Asana task)
All this will be done in the following diff, this is already substantial.
Also, please consider if we want to have all those `VLOG`s in the code. Not very pretty. And I think that `VLOG` is not compiled-away in release build, will put an asana task.
Reviewers: teon.banek, mislav.bradac, buda
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D569
Summary:
Allow expressions for variable length path bounds
Replace test which expected a syntax exception
Since we now allow variable length to have an arbitrary expression, the
test case is obsolete. It was replaced with something that excepts an
expression which wasn't allowed before.
Reviewers: florijan, mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: lion, pullbot
Differential Revision: https://phabricator.memgraph.io/D568
Summary:
- GraphDbAccessor - index range API added
- index api tests refactored
- skiplist minor cleanup.
Reviewers: teon.banek, buda, mislav.bradac
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D533
Summary:
Add optional bounds to PropertyFilter and collect them
Relation operators (e.g. `<`, `>` ...) should be used to produce
scanning the index by a range of values. For that reason, PropertyFilter
is extended to store either the equality expression or range bounds.
The `AnalyzeFilter` function is extended to look for those operators and
see if their top level expression contains a property lookup. If it
does, a filter with a bound is generated.
Test for property comparison preventing index use
Reviewers: florijan, mislav.bradac, buda
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D529
Summary:
This change allows for lazy calculation of Cartesian product, by using
an iterator. Using lazy evaluation, we can easily limit the number of
generated products and therefore the number of generated query plans.
Reviewers: florijan, mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D554
Summary:
Add Filters class for storing additional info
Add FindOr to utils/algorithm.hpp
Use all collected labels when scanning by them
Collect label filters inside WHERE
Document the Filters class
Reviewers: florijan, mislav.bradac, buda
Reviewed By: florijan
Subscribers: pullbot, lion
Differential Revision: https://phabricator.memgraph.io/D515
Summary:
The new Bound class does not have comparison operators defined. The
reason being, we want to support having values which we may not want to
compare. For example, having an Expression which should first be
evaluated and then compared.
Reviewers: florijan, mislav.bradac, buda
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D520
Summary:
Add ScanAllByLabelPropertyRange operator
This operator uses the label + property indexing feature to iterate over
the vertices. The property value of each vertex is checked whether it is
inside the given range of values. The range is inclusive from both
sides. If the value isn't in range, the vertex is filtered out.
This manual filtering should be replaced by a database API when it
becomes available.
Add ScanAllByLabelPropertyValue operator
Reviewers: florijan, mislav.bradac, buda
Reviewed By: florijan, mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D503
Summary:
This fixes an issue when aggregations and/or group by expressions
weren't picked up from certain operators. In addition to that, we would
segfault in cases when the `has_aggregation_` is empty. For example,
function calls without arguments: `RETURN PI()`.
Test aggregations inside some operators
Reviewers: florijan, mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D425
Summary:
Permute query parts.
Permute matching only by selecting the starting node.
Flip the expansion when expanding from the other node.
Split planner into rule_based_planner and variable_start_planner
Use symbol hash when collecting expansion nodes
Multiple node atoms may point to the same symbol, and we could generate
multiple starting positions per atom which are the same. Using symbol
hash and equality prevents generating those redundant plans.
Correctly permute optional and merge matchings
Test VariableStartPlanner
Reviewers: florijan, mislav.bradac, buda, lion
Reviewed By: florijan, buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D417
Summary:
We would redundantly generate an Expansion for the first node if it was part of
an expand. For example, the pattern `(n) -[r]- (m)` would generate
`Expansion{n}` and `Expansion{n, r, m}`, when only the latter is enough. This
change corrects that behaviour by dropping the first Expansion.
Reviewers: florijan, mislav.bradac
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D412
Summary:
This change modifies the planning API to be more general, in order to support
picking different planning strategies. The current planning strategy has been
named RuleBasedPlanner.
Reviewers: florijan, mislav.bradac
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D411
Summary:
Since the vertices iterable used in ScanAllCursor may be lazily
generated, it needs to be recreated, instead of simply calling
`begin()`. In our current implementation, we use cppitertools which do
not have move assignment implemented. Because of that, a hackish
in-place destruction and construction is used to reset the iterable.
Reviewers: florijan, mislav.bradac, dgleich, buda
Reviewed By: dgleich
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D401
Summary:
Mention the non-existent function name in semantic error. Don't merge optional
matches into one Matching, because it is an error to treat multiple optional
matches as a single optional match. Document new structures and functions. Add
not so smart ScanAllByLabel generation.
Reviewers: mislav.bradac, buda, florijan, lion
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D394
Summary:
Replace NodeAtom with Symbol inside ScanAll. Move ScanAllCursor outside of
ScanAll class and make it generic with regards to vertices it produces.
Reviewers: mislav.bradac, florijan
Reviewed By: mislav.bradac, florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D387
Summary:
This fixes a bug when the MATCH clause would follow an OPTIONAL MATCH.
In case when the optional part would fail to generate results, expanding
would cause an error.
Reviewers: florijan, mislav.bradac, buda
Reviewed By: mislav.bradac, buda
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D372
Summary:
openCypher expects removing/setting properties and labels on Null
vertices/edges does not produce an error. Instead, Nulls are simply
skipped.
Reviewers: florijan, mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D375
Summary:
Merge utils/visitor directory into single file.
Rename Visitor to HierarchicalVisitor.
Add regular Visitor.
Split HierarchicalVisitor into LeafVisitor and CompositeVisitor.
Add more documentation on visitor pattern.
Make PostVisit and Visit return bool.
Reviewers: florijan, mislav.bradac, buda
Reviewed By: mislav.bradac
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D364
Summary:
Make Symbol members read only.
Check WITH/RETURN * in SymbolGenerator.
Test semantic checks for WITH/RETURN *.
Sort expanded user identifiers by name.
Test planning WITH/RETURN *.
Reviewers: buda, florijan, mislav.bradac
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D357
Summary:
Check symbols in property maps after visiting Match.
Plan Filters as soon as possible.
Take AstTreeStorage in MakeLogicalPlan instead of Query.
Plan generic Filter instead of specialized operators.
Remove traces of EdgeFilter and NodeFilter.
Reviewers: buda, mislav.bradac, florijan
Reviewed By: mislav.bradac, florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D344
Summary:
The GraphDbAccessor and KeyIndex APIs can now also return records for the current transaction+command graph state. This is necessary to correctly implement MERGE. The new logic is has increased the MVCC+Accessor related chaos and should be revised when refactoring MVCC (as planned).
Previous index testing was separated into VertexIndex and EdgeIndex testing. This is inappropriate since most of the logic is exaclty the same. Also it was not clearly defined what gets tested via the GraphDbAccessor API, and what directly through the KeyIndex API. This has also been refactored, but it needs additional work (Gleich).
Reviewers: buda, dgleich
Reviewed By: buda, dgleich
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D306
Summary:
Support ListLiteral in test macros.
Test planning Unwind.
Support UNWIND in test macros.
Test SymbolGenerator for UNWIND clause.
Use namespace in QueryPlan Unwind test.
Reviewers: mislav.bradac, buda, florijan
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D331
Summary:
Support OPTIONAL MATCH in test macros.
Test planning Optional.
Reviewers: florijan, mislav.bradac
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D322
Summary:
Check symbols in Merge.
Support MERGE macro in query tests.
Test SymbolGenerator with MERGE.
Test planning Merge.
Reviewers: florijan, mislav.bradac
Reviewed By: florijan
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D317
