Temporal types #474
Merged
Temporal types #474
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Format with type was never used, all values have always been formatted "by value". This commit just forces all `Value` implementations to have `#toString()`.
Driver is now able to send and receive instances of Date type. They are represented as `java.time.LocalDate` objects.
It is useless because current driver version is a pre-release version.
Driver is now able to send and receive instances of Time type. They are represented as `java.time.OffsetTime` objects.
Driver is now able to send and receive instances of LocalTime type. They are represented as `java.time.LocalTime` objects.
Driver is now able to send and receive instances of LocalDateTime type. They are represented as `java.time.LocalDateTime` objects.
Driver is now able to send and receive instances of DateTime type. They are represented as `java.time.ZonedDateTime` objects. `ZonedDateTime` instances contain time zone information either as numeric offset or as a zone id/name.
Driver is now able to send and receive instances of Duration type. They are represented as instances of custom `org.neo4j.driver.v1.types.Duration` interface. It's needed because Cypher's duration contains months, days, seconds and nanoseconds. There exists no default class in `java.time` package to represent a combination of these temporal values.
Removed "_TyCon" suffix, made code use `#toString()` instead of `#typeName()`.
Instead of direct constructor invocation. So creation of value implementations and internal classes only live in `Values` helper class and is not duplicated.
By extracting generic test methods.
To assert that driver can send and receive DateTime with time zone information as offset and zone name.
By using method references and lambdas instead of static final function implementations.
zhenlineo
reviewed
Mar 8, 2018
| if ( value instanceof LocalTime ) { return value( (LocalTime) value ); } | ||
| if ( value instanceof LocalDateTime ) { return value( (LocalDateTime) value ); } | ||
| if ( value instanceof ZonedDateTime ) { return value( (ZonedDateTime) value ); } | ||
| if ( value instanceof Duration ) { return value( (Duration) value ); } |
There was a problem hiding this comment.
It seems this Duration is our org.neo4j.driver.v1.types.Duration type, do we also want to support java native Duration and/or Period?
| private final long months; | ||
| private final long days; | ||
| private final long seconds; | ||
| private final long nanoseconds; |
There was a problem hiding this comment.
Maybe instead of having months, days, seconds, nanoseconds 4 field, we could have Period period and Duration duration? Then the calculation such as add and minus would be easier?
There was a problem hiding this comment.
We receive months and days as longs from the database but java.time.Period only allows ints
| * Value that represents a duration can be created using {@link Values#duration(long, long, long, long)} method. | ||
| */ | ||
| @Immutable | ||
| public interface Duration extends TemporalAmount |
There was a problem hiding this comment.
Maybe IsoStandardDuration?
There was a problem hiding this comment.
Renamed to IsoDuration in bb31fc0
lutovich
force-pushed
the
1.6-temporal-types
branch
2 times, most recently
from
4023f8a to
a5f149e
Compare
So it does not clash with `java.time.Duration` and specifies that it's a combination of date-based amount and time-based amount.
Driver represents duration as a custom `IsoDuration` interface. It's a combination of date-based amount and time-based amount. This commit makes driver accept `java.time.Period` and `java.time.Duration` as parameters and convert them to `IsoDuration`.
lutovich
force-pushed
the
1.6-temporal-types
branch
from
a5f149e to
a355fc4
Compare
ali-ince
approved these changes
Mar 15, 2018
There was a problem hiding this comment.
Changes look good to me.
@technige used WITH $x AS x RETURN x.hour, x.minute, x.second kind of queries in ITs which I think is a good way to check consistency across all layers. We may also consider it for the rest of the drivers.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR makes driver support new temporal Cypher types:
java.time.LocalDatejava.time.OffsetTimejava.time.LocalTimejava.time.LocalDateTimejava.time.ZonedDateTimeorg.neo4j.driver.v1.types.Durationinterface containing months, days, seconds and nanosecondsAlso simplified couple things around the
Valueclass hierarchy.