TODO
Functionality and API
Bugfix: The
get
method behaves unpredictably for queries that have fields with conflicting indexes and keys such asRETURN 1, 0
. It should be possible to distinguish between a key and an index by inspecting the scalar's FLAGS (namely,POK
/IOK
; see "EXAMPLES" in Devel::Peek and JSON::PP's_looks_like_number
). Thenget(0)
would mean index0
andget('0')
would mean key0
. The JS driver uses this concept.Implement spatial and temporal types.
Add timers to Neo4j::Driver::ResultSummary (see
Neo4j::Bolt
).
Experimental features
ServerInfo
: Refactor the existing code into a package of its own for clarity. Create an instance on creation of the driver's first session, so that it's relatively cheap and can safely be handed down to the ResultSummary. Get server version with Bolt."Parameter syntax conversion" in Neo4j::Driver: make stable and move filter implementation to
Transaction
in preparation of Bolt v3 support"meta" in Neo4j::Driver::Record: remove
"Calling in list context" in Neo4j::Driver::Session: make illegal
"Calling in list context" in Neo4j::Driver::Transaction: make illegal
"Execute multiple statements at once" in Neo4j::Driver::Transaction: This feature should no longer be exposed to the client. It complicates the API significantly and is not that big of an optimisation anyway, because results are typically fetched before the next statement is run.
"Return results in graph format" in Neo4j::Driver::Transaction, "graph" in Neo4j::Driver::Record: make driver config option
"Path as alternating array" in Neo4j::Driver::Type::Path: make stable
Tests, code quality, documentation
Test roundtrip of special numeric values (very large integers, -0.0, ±Inf, ±NaN).
Try to modify the way the data is stored to verify the tests fix in 0.13. In particular, the URLs maybe should be parsed by the new type classes instead of
deep_bless
.Convert tests in
xt/compatibility
to use Module::Load, so that testing/dev dependencies are fewer.Improve test coverage:
Many yet uncovered code paths are obviously fine, but difficult or impossible to cover. In some of these cases, it may be possible to refactor the code, such as by banking on autovivification (i.e. don't defend against undefined
$a
in expressions like$a->{b}
; see "Using References" in perlref).The
deep_bless
subs contain a lot of assertions and other checks that are not normally necessary. Since this logic seems to work fine, it should be simplified. (This may also make it easier to unroll the recursion later.)The current policy of not documenting deprecated methods is informed by the principle to "design interfaces that are: consistent; easy to use correctly; hard to use incorrectly". Perhaps simply listing the deprecated method names with short note like "deprecated in 0.13" would be an acceptable addition that also fulfils the pod coverage requirements.
Documenting each attribute in Neo4j::Driver::SummaryCounters as individual methods might be a quick way to bring up the pod coverage stats a bit.
Making sure all private methods (e. g. in Neo4j::Driver::Transport::*) have names beginning with
_
might help to improve pod coverage.
List possible
croak
output in "DIAGNOSTICS" in Neo4j::Driver, allowing for indexing by search engines."notifications" in Neo4j::Driver::ResultSummary: clarify docs on list context
Optimise the simulator for
$hash = 0
. Use of<<
causes statements to end with\n
, which the simulator could filter out. The internals test "transaction: REST 404 error handling" should run a distinct statement.
Other ideas for specific modules
Neo4j::Driver
Make the URL a config option, so that it can be queried (and changed).
Allow passing config options directly to the constructor, e. g. in place of the URL (
Neo4j::Driver->new({ url=>"bolt:", timeout=>30 })
).
Neo4j::Driver::Session
Sessions of some of the official drivers can have at most one transaction running at a time. This restriction is not necessary for this Perl driver when HTTP is used, but perhaps we should implement it anyway for consistency.
DBI uses
begin_work
in place ofbegin_transaction
as a method name. Even thoughbegin_work
is shorter, it should not be the main method name becausebegin_transaction
is well established in the official Neo4j drivers, and after all this is notDBI
. However, as this is the only (almost) exact equivalence withDBI
that we have, we should perhaps offerbegin_work
as an experimental alias at least.Consider whether to offer transaction functions. If available, these should consist of subrefs passed to methods called
write_transaction
andread_transaction
. These access modes are only an optimisation for Enterprise features. We don't target those at present, butread_transaction
could then eventually be routed to a high-performance read-only server once clusters are supported. It would make sense to offer both methods right away even though initially they'd work exactly the same.
Neo4j::Driver::Transaction
Consider supporting re-using
Record
objects for query parameters inrun
. The Java and C# drivers do this.Run statements lazily: Just like with the official drivers, statements passed to
run
should be gathered until their results are actually accessed. Then, and only then, all statements gathered so far should be sent to the server using a single request. Challenges of this approach include that notifications are not associated with a single statement, so there must be an option to disable this behaviour; indeed, disabled should probably be the default when stats are requested. Additionally, there are some bugs with multiple statements (see testsnon-arrayref individual statement
andinclude empty statement
). Since stats are now requested by default, this item might mean investing time in developing an optimisation feature that is almost never used. Since the server is often run on localhost anyway where latency is very close to zero, this item should not have high priority.
Neo4j::Driver::StatementResult
Neo4j::Driver::Record
Consider whether to implement methods to query the list of fields for this record (
keys
,has
,size
) and/or a mapping function for all fields (map
/forEach
). Given that this data should easily be available through the StatementResult object, these seem slightly superfluous though.Implement
graph
; see https://github.com/neo4j/neo4j/blob/3.5/community/server/src/main/java/org/neo4j/server/rest/transactional/GraphExtractionWriter.java, https://github.com/neo4j/neo4j-python-driver/wiki/1.6-changelog#160a1.
Neo4j::Driver::ResultColumns
The
list
implementation is quite ugly and probably has bugs with regards to index/key collisions.
Neo4j::Driver::ResultSummary
Profile the server-side performance penalty of requesting stats for various kinds of queries. If the penalty turns out to be high, stats should perhaps have to be requested explicitly by clients (rather than being obtained by default, as with 0.13 and higher). However, using Bolt always provides stats, and different APIs for HTTP and Bolt seem like a bad idea.
Neo4j::Driver::SummaryCounters
use Class::Accessor::Fast 0.34;
Neo4j::Driver::Transport::HTTP
Profile whether X-Stream has any performance impact. Remove if in doubt, as we don't implement streaming (and have no plans to in the future, considering that Bolt should be used when speed is important).
Make REST::Client code more robust by checking the private method
{_res}->status_line
return value (possibly usingref()
) and fall back to just the status code if necessary.An alternative to using
$client->{_res}->status_line
(to get the HTTP error message) might be to call$client->getUseragent->add_handler(response_header => sub { $status_line = shift->status_line })
. However, this is probably slower and would likely need to be run for each and every POST including those with2xx
status codes, which might not be acceptable.Consider unrolling
deep_bless
recursion. Based on initial profiling, this may save up to about 5% CPU time (for a specific HTTP test query cached in RAM, performance went from about 2700/s to 2850/s when skipping the call todeep_bless
entirely). However, when accessing the database, the bottleneck is typically I/O (querying Neo4j itself instead of the RAM-cached response let the performance for the very same query drop down to 650/s when executed over HTTP). So this optimisation may not be worth it (OTOH, Bolt performance was something like 7000/s, so optimisingdeep_bless
may be more useful there).Consider making the JSON module configurable. For example, clients might wish to use Mojo::JSON instead of JSON::MaybeXS; it also prefers Cpanel::JSON::XS where available, but in Pure Perl it may be faster than JSON::PP. Also, there is JSON::Tiny.
Neo4j::Driver::Type::*
The
eq
andne
operators should be overloaded to allow for ID comparison on nodes and relationships.Consider whether to use
use overload '%{}'
to allow direct access to e. g. properties using the pre-0.13 hashref syntax. See https://metacpan.org/pod/overload#Two-face-References for an example. perltie might perhaps also work.