Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add roles for temporal types #15

Closed
wants to merge 0 commits into from
Closed

Add roles for temporal types #15

wants to merge 0 commits into from

Conversation

johannessen
Copy link
Owner

The earlier recommendation to use the DateTime modules that already exist on CPAN didn’t take into account that Neo4j defines multiple distinct temporal instant types. Using the DateTime module means that clean roundtrips aren’t possible.

The original proposal in perlbolt’s feat-v5.0 branch isn’t bad, but follows the Bolt protocol spec very closely, which is not a good match for Jolt v1/v2. It also requires 64 bit integers.

This proposal is close to the Bolt spec, but cleanly separates dates and times in order to accommodate Jolt. The resulting numbers are smaller, which should make it work fine even on 32 bit Perls, which are getting rare but they do still exist.

There are two points to note in this proposal:

  1. The methods are defined such that the Neo4j type can be determined by simply looking at which value components are missing. For example, if the time component (seconds) is missing in a temporal instant value, it must be a value of type DATE. The resulting set of methods gives a good amount of flexibility in the data structure.

  2. The role is separated from the generic implementation. This should make the goal of using roles easier to understand. It may also make it easier for implementors to write new classes performing the role, because the required interface is smaller.

@johannessen johannessen added Neo4j Neo4j / Cypher compliance Drivers Compatibility with type implementations labels Aug 6, 2023
@johannessen johannessen added this to the Version 2 milestone Aug 6, 2023
@johannessen

This comment was marked as outdated.

@johannessen johannessen marked this pull request as draft August 6, 2023 16:19
@johannessen johannessen marked this pull request as ready for review August 7, 2023 22:04
@johannessen
Copy link
Owner Author

johannessen commented Oct 22, 2023

(I pushed branches in the wrong order, leading GH to treat the PR as closed instead of merged. This PR was in fact properly merged in cff7afd.)

@johannessen johannessen deleted the datetime branch October 22, 2023 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Drivers Compatibility with type implementations Neo4j Neo4j / Cypher compliance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant