hypersistence-utils icon indicating copy to clipboard operation
hypersistence-utils copied to clipboard

Add Support for Range of Instant

Open benstpierre opened this issue 4 years ago • 13 comments

Would it make sense to Have a Range of Instants? We have a range of localdate time but Range of instant is equally useful?

benstpierre avatar Jan 08 '21 02:01 benstpierre

What PostgreSQL column type do you think it's best to represent an Instant? I suppose it should be a big interest storing milliseconds since epoch, right?

vladmihalcea avatar Jan 08 '21 06:01 vladmihalcea

The project I am working on now uses Hibernate 5.4.27.Final and when I set HBM2DDL_AUTO to auto it generates timestamp columns for Instant. We are on Postgres 12.5 with org.hibernate.dialect.PostgreSQL10Dialect. Seems like timestamp is a good match.

benstpierre avatar Jan 08 '21 06:01 benstpierre

Using Hibernate hbm2ddl is a terrible idea unless you are doing it for the very first incremental script. Flyway is the way.

Timestamp is not a good match for Instant. LocalDateTime is.

vladmihalcea avatar Jan 08 '21 07:01 vladmihalcea

Yes this is exactly what I am doing... using hbm2ddl for my very first script. I will then take a dump of the schema and use it for my V1__ script in flyway when the app goes to production.

benstpierre avatar Jan 08 '21 07:01 benstpierre

Hibernate supports mapping a Java type to multiple DB types via SQL descriptors. If you send me a Pull Request that implements this issue, I'll review it.

vladmihalcea avatar Jan 08 '21 07:01 vladmihalcea

We found that timestamp with time zone best matched Instant. This type converted to a +00:00 offset timestamp. Timestamp without timezone matched best with LocalDateTime in our experience

Kurru avatar Apr 01 '22 21:04 Kurru

The PostgreSQL timestamp with timezone is best mapped to OffsetDateTime.

vladmihalcea avatar Apr 01 '22 22:04 vladmihalcea

Perhaps, but my answer was that Instant mapped well to timestamp with time zone for us. Sure theres an additional +00:00 offset, but thats (seemly) harmless and still represents an Instant quite well. The inverse may not be true.

Kurru avatar Apr 02 '22 18:04 Kurru

@Kurru Have you considered forking the Range and the PostgreSQLRangeType classes yourself and changed ZonedDateTime to Instant instead?

Right now the default deserialization of a tstzrange type in the PostgreSQLRangeType gets mapped to a Range<ZonedDateTime>. If you want to you can convert it manually afterwards in the client code, but changing the current implementation to use Instant would be backwards incompatible for current clients of the Range class.

asdm90 avatar Aug 18 '22 12:08 asdm90

@Kurru Have you considered forking the Range and the PostgreSQLRangeType classes yourself and changed ZonedDateTime to Instant instead?

We dont use ranges, just java Instant and timestamp with time zone in postgres

Kurru avatar Aug 23 '22 19:08 Kurru

@Kurru The issue is about Range of Instant. The Java Instant is covered by Hibernate.

vladmihalcea avatar Aug 24 '22 03:08 vladmihalcea

+1

Support for Java Instant is coming up in JPA 3.2. But it has been in Hibernate for quite some time. I don't think the Hibernate implementation will change as a result of JPA 3.2; it is already compliant, I think.

What PostgreSQL column type do you think it's best to represent an Instant? I suppose it should be a big integer storing milliseconds since epoch, right?

Narh. In my case Instant on the Java side and simple TIMESTAMP on the database side. Hibernate 6 handles this wonderfully. It knows that it needs to force-convert to UTC. (no need to set hibernate.jdbc.time_zone=UTC as Hibernate takes this a given when you are using Instant).

Personally, I find it excellent that Instant is finally finding its way into the JPA Spec. We've been forced to use weird "proxies" for way too long. I claim that the majority of the use-case for timestamp is of the type where you want to record the time when something happened. By definition, in a persistence layer, that would is an Instant (not a ZonedDateTime, LocalDateTime or OffsetDateTime)

lbruun avatar Aug 24 '23 06:08 lbruun

Another reason: A Range<Instant> implementation wouldn't suffer this nasty problem.

lbruun avatar Sep 20 '23 20:09 lbruun