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

Consider changing from ZonedDateTime to OffsetDateTime in date2 plugin #248

Open
kaharlichenko opened this issue Dec 24, 2015 · 6 comments
Labels

Comments

@kaharlichenko
Copy link

To my mind it would be more appropriate to use OffsetDateTime instead of ZonedDateTime to represent timestamptz.

Here's an excerpt from ZonedDateTime javadoc:

This class stores all date and time fields, to a precision of nanoseconds, and a time-zone, with a zone offset used to handle ambiguous local date-times.

The notable thing here is "and a time-zone" part.

Here's an excerpt from Postgres manual 8.5.3. Time Zones:

All timezone-aware dates and times are stored internally in UTC. They are converted to local time in the zone specified by the TimeZone configuration parameter before being displayed to the client.

As you can see, there's a discrepancy between ZonedDateTime and timestamptz handling of the time zone. The class does store the time zone (e.g. Australia/Sydney) while the column does not and relies on the database configuration option.

So if you have an instance of ZonedDateTime, store it in the database, then fetch it back you are not guaranteed to get the same time zone as the original class instance.


On the other hand even if you use OffsetDateTime you are not guaranteed to get the same time offset (e.g. +03:00) back either. But anyways it is a bit better, because either way or another you get back the very same time instant that you stored if you use OffsetDateTime which might not be the case with ZonedDateTime.

@tminglei
Copy link
Owner

@madkinder slick-pg supports them both. You can check it. ;-)

But because of #220, the time zone info may be lost.

@kaharlichenko
Copy link
Author

Well, the base case of using OffsetDateTime directly does indeed work. But here's the case that I'm having troubles with.

Given the column definition

"targeting_period" TSTZRANGE NOT NULL

and column mapping definition

def targetingPeriod = column[Option[Range[OffsetDateTime]]]("targeting_period", O.Default(None))

I get the following error

could not find implicit value for parameter tt: slick.ast.TypedType[Option[com.github.tminglei.slickpg.Range[java.time.OffsetDateTime]]]

It does work when I change OffsetDateTime to java.sql.Timestamp, but I that's not exactly what I wanted the mapping to do. Is it possible to get a nullable tstzrange mapped to an optional range of OffsetDateTime?


As for the the original question. Even though slick-pg supports both OffsetDateTime and ZonedDateTime for base cases, it would be nice to somehow make it explicit that the time zone is lost when using ZonedDateTime and mapping to timestamptz. It could be done in the docs, emitting a warning at compile time or any other means you like. I couldn't figure that out without digging in the documentation of both Postgres and javadoc.

@tminglei
Copy link
Owner

Hi @madkinder, slick-pg's date2 addon supports both ZonedDateTime and OffsetDateTime, but not in range support.
For how to support ZonedDateTime/OffsetDateTime range, you can refer to its existing codes here.

slick-pg is a personal project. Given I haven't enough time, you're encouraged to dig into codes/tests. For the inconvenience, I'm sorry!

@callicles
Copy link
Contributor

Hi guys ! Would be cool to update the docs with the compatibility of OffsetDateTime. I can make a pull request if you wish

@tminglei
Copy link
Owner

tminglei commented Sep 6, 2016

@callicles yes, it's welcome!

callicles added a commit to callicles/slick-pg that referenced this issue Oct 12, 2016
callicles added a commit to callicles/slick-pg that referenced this issue Oct 12, 2016
callicles added a commit to callicles/slick-pg that referenced this issue Oct 12, 2016
@wsargent
Copy link

wsargent commented Jan 5, 2020

Apparently if you want to keep the offset, you should add a distinct value in seconds, and then map the offset from there https://stackoverflow.com/questions/54189839/how-to-store-offsetdatetime-to-postgresql-timestamp-with-time-zone-column

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants