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.

Sign up for GitHub

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

simplify handling of datetimes and fix conversion to JDBC timezone #9284

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

simplify handling of datetimes and fix conversion to JDBC timezone #9284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
849f23c
simplify handling of datetimes and fix conversion to JDBC timezone
gavinking Nov 19, 2024
9c7ec90
temporarily disable test
gavinking Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions hibernate-core/src/main/java/org/hibernate/type/descriptor/WrapperOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.engine.spi.SharedSessionContractImplementor;

import java.time.Instant;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.util.TimeZone;

/**
Expand Down Expand Up @@ -91,4 +94,53 @@ default Dialect getDialect() {
* @see org.hibernate.cfg.AvailableSettings#JDBC_TIME_ZONE
*/
TimeZone getJdbcTimeZone();

/**
* Get the {@link ZoneId} representing the
* {@linkplain #getJdbcTimeZone JDBC time zone}, or the
* {@linkplain ZoneId#systemDefault JVM default time zone id}
* if no JDBC time zone was set via
* {@value org.hibernate.cfg.AvailableSettings#JDBC_TIME_ZONE}.
*
* @apiNote Use for converting datetimes to and from the JDBC
* time zone.
*
* @see org.hibernate.cfg.AvailableSettings#JDBC_TIME_ZONE
*/
default ZoneId getJdbcZoneId() {
final TimeZone jdbcTimeZone = getJdbcTimeZone();
return jdbcTimeZone == null
? ZoneId.systemDefault()
: jdbcTimeZone.toZoneId();
}

/**
* Get the current {@link ZoneOffset} for the {@link ZoneId}
* representing the {@linkplain #getJdbcTimeZone JDBC time zone},
* or the {@linkplain ZoneId#systemDefault JVM default time zone id}
* if no JDBC time zone was set via
* {@value org.hibernate.cfg.AvailableSettings#JDBC_TIME_ZONE}.
*
* @apiNote Use for converting <em>times</em> to and from the JDBC
* time zone. For datetimes, prefer {@link #getJdbcZoneId()}
* to apply the rules in force at the given datetime.
*
* @see org.hibernate.cfg.AvailableSettings#JDBC_TIME_ZONE
*/
default ZoneOffset getJdbcZoneOffset() {
return getJdbcZoneId().getRules().getOffset( Instant.now() );
}

/**
* Get the current {@link ZoneOffset} for the
* {@linkplain ZoneId#systemDefault JVM default time zone id}.
*
* @apiNote Use for converting <em>times</em> to and from the
* system time zone. For datetimes, prefer
* {@link ZoneId#systemDefault()}, to apply the rules
* in force at the given datetime.
*/
default ZoneOffset getSystemZoneOffset() {
return ZoneId.systemDefault().getRules().getOffset( Instant.now() );
}
}
42 changes: 8 additions & 34 deletions hibernate-core/src/main/java/org/hibernate/type/descriptor/java/InstantJavaType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import java.sql.Timestamp;
import java.time.Instant;
import java.time.OffsetDateTime;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
Expand Down Expand Up @@ -102,23 +101,10 @@ public <X> X unwrap(Instant instant, Class<X> type, WrapperOptions options) {
}

if ( Timestamp.class.isAssignableFrom( type ) ) {
/*
* This works around two bugs:
* - HHH-13266 (JDK-8061577): around and before 1900,
* the number of milliseconds since the epoch does not mean the same thing
* for java.util and java.time, so conversion must be done using the year, month, day, hour, etc.
* - HHH-13379 (JDK-4312621): after 1908 (approximately),
* Daylight Saving Time introduces ambiguity in the year/month/day/hour/etc representation once a year
* (on DST end), so conversion must be done using the number of milliseconds since the epoch.
* - around 1905, both methods are equally valid, so we don't really care which one is used.
*/
ZonedDateTime zonedDateTime = instant.atZone( ZoneId.systemDefault() );
if ( zonedDateTime.getYear() < 1905 ) {
return (X) Timestamp.valueOf( zonedDateTime.toLocalDateTime() );
}
else {
return (X) Timestamp.from( instant );
}
// NOTE: do not use Timestamp.from(Instant) because that returns a Timestamp in UTC
final ZonedDateTime dateTime =
instant.atZone( options.getJdbcZoneId() ); // convert to the JDBC timezone
return (X) Timestamp.valueOf( dateTime.toLocalDateTime() );
}

if ( java.sql.Date.class.isAssignableFrom( type ) ) {
Expand Down Expand Up @@ -155,22 +141,10 @@ public <X> Instant wrap(X value, WrapperOptions options) {
}

if ( value instanceof Timestamp timestamp ) {
/*
* This works around two bugs:
* - HHH-13266 (JDK-8061577): around and before 1900,
* the number of milliseconds since the epoch does not mean the same thing
* for java.util and java.time, so conversion must be done using the year, month, day, hour, etc.
* - HHH-13379 (JDK-4312621): after 1908 (approximately),
* Daylight Saving Time introduces ambiguity in the year/month/day/hour/etc representation once a year
* (on DST end), so conversion must be done using the number of milliseconds since the epoch.
* - around 1905, both methods are equally valid, so we don't really care which one is used.
*/
if ( timestamp.getYear() < 5 ) { // Timestamp year 0 is 1900
return timestamp.toLocalDateTime().atZone( ZoneId.systemDefault() ).toInstant();
}
else {
return timestamp.toInstant();
}
// NOTE: do not use Timestamp.getInstant() because that assumes the Timestamp is in UTC
return timestamp.toLocalDateTime()
.atZone( options.getJdbcZoneId() ) // the Timestamp is in the JDBC timezone
.toInstant(); // return the instant (always in UTC)
}

if ( value instanceof Long longValue ) {
Expand Down
41 changes: 7 additions & 34 deletions hibernate-core/src/main/java/org/hibernate/type/descriptor/java/OffsetDateTimeJavaType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,24 +137,9 @@ public <X> X unwrap(OffsetDateTime offsetDateTime, Class<X> type, WrapperOptions
}

if ( Timestamp.class.isAssignableFrom( type ) ) {
/*
* This works around two bugs:
* - HHH-13266 (JDK-8061577): around and before 1900,
* the number of milliseconds since the epoch does not mean the same thing
* for java.util and java.time, so conversion must be done using the year, month, day, hour, etc.
* - HHH-13379 (JDK-4312621): after 1908 (approximately),
* Daylight Saving Time introduces ambiguity in the year/month/day/hour/etc representation once a year
* (on DST end), so conversion must be done using the number of milliseconds since the epoch.
* - around 1905, both methods are equally valid, so we don't really care which one is used.
*/
if ( offsetDateTime.getYear() < 1905 ) {
return (X) Timestamp.valueOf(
offsetDateTime.atZoneSameInstant( ZoneId.systemDefault() ).toLocalDateTime()
);
}
else {
return (X) Timestamp.from( offsetDateTime.toInstant() );
}
final ZonedDateTime dateTime =
offsetDateTime.atZoneSameInstant( options.getJdbcZoneId() ); // convert to the JDBC timezone
return (X) Timestamp.valueOf( dateTime.toLocalDateTime() );
}

if ( java.sql.Date.class.isAssignableFrom( type ) ) {
Expand Down Expand Up @@ -195,22 +180,10 @@ public <X> OffsetDateTime wrap(X value, WrapperOptions options) {
}

if (value instanceof Timestamp timestamp) {
/*
* This works around two bugs:
* - HHH-13266 (JDK-8061577): around and before 1900,
* the number of milliseconds since the epoch does not mean the same thing
* for java.util and java.time, so conversion must be done using the year, month, day, hour, etc.
* - HHH-13379 (JDK-4312621): after 1908 (approximately),
* Daylight Saving Time introduces ambiguity in the year/month/day/hour/etc representation once a year
* (on DST end), so conversion must be done using the number of milliseconds since the epoch.
* - around 1905, both methods are equally valid, so we don't really care which one is used.
*/
if ( timestamp.getYear() < 5 ) { // Timestamp year 0 is 1900
return timestamp.toLocalDateTime().atZone( ZoneId.systemDefault() ).toOffsetDateTime();
}
else {
return OffsetDateTime.ofInstant( timestamp.toInstant(), ZoneId.systemDefault() );
}
return timestamp.toLocalDateTime()
.atZone( options.getJdbcZoneId() ) // the Timestamp is in the JDBC timezone
.withZoneSameInstant( ZoneId.systemDefault() ) // convert back to the VM timezone
.toOffsetDateTime(); // return the corresponding OffsetDateTime
}

if (value instanceof Date date) {
Expand Down
138 changes: 63 additions & 75 deletions hibernate-core/src/main/java/org/hibernate/type/descriptor/java/OffsetTimeJavaType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import java.time.LocalTime;
import java.time.OffsetDateTime;
import java.time.OffsetTime;
import java.time.ZoneOffset;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;
import java.time.temporal.ChronoField;
import java.util.Calendar;
Expand Down Expand Up @@ -92,59 +92,71 @@ public <X> X unwrap(OffsetTime offsetTime, Class<X> type, WrapperOptions options
}

if ( LocalTime.class.isAssignableFrom( type ) ) {
return (X) offsetTime.withOffsetSameInstant( getCurrentSystemOffset() ).toLocalTime();
return (X) offsetTime.withOffsetSameInstant( options.getSystemZoneOffset() ).toLocalTime();
}

if ( OffsetDateTime.class.isAssignableFrom( type ) ) {
return (X) offsetTime.atDate( LocalDate.EPOCH );
return (X) offsetDateTimeAtEpoch( offsetTime );
}

// for legacy types, we assume that the JDBC timezone is passed to JDBC
// (since PS.setTime() and friends do accept a timezone passed as a Calendar)

final OffsetTime jdbcOffsetTime = offsetTime.withOffsetSameInstant( getCurrentJdbcOffset(options) );

if ( Time.class.isAssignableFrom( type ) ) {
// Use ZoneOffset rather than ZoneId in the conversion,
// since the offset for a zone varies over time, but
// a Time does not have an attached Date.
final OffsetTime jdbcOffsetTime =
offsetTime.withOffsetSameInstant( options.getJdbcZoneOffset() ); // convert to the JDBC timezone
final Time time = Time.valueOf( jdbcOffsetTime.toLocalTime() );
if ( jdbcOffsetTime.getNano() == 0 ) {
return (X) time;
}
// Preserve milliseconds, which java.sql.Time supports
return (X) new Time( time.getTime() + DateTimeUtils.roundToPrecision( jdbcOffsetTime.getNano(), 3 ) / 1000000 );
// Time.valueOf() throws away milliseconds
return (X) withMillis( jdbcOffsetTime, time );
}

final OffsetDateTime jdbcOffsetDateTime = jdbcOffsetTime.atDate( LocalDate.EPOCH );

if ( Timestamp.class.isAssignableFrom( type ) ) {
/*
* Workaround for HHH-13266 (JDK-8061577).
* Ideally we'd want to use Timestamp.from( jdbcOffsetDateTime.toInstant() ),
* but this won't always work since Timestamp.from() assumes the number of
* milliseconds since the epoch means the same thing in Timestamp and Instant,
* but it doesn't, in particular before 1900.
*/
return (X) Timestamp.valueOf( jdbcOffsetDateTime.toLocalDateTime() );
final OffsetTime jdbcOffsetTime =
offsetTime.withOffsetSameInstant( options.getJdbcZoneOffset() ); // convert to the JDBC timezone
return (X) Timestamp.valueOf( offsetDateTimeAtEpoch( jdbcOffsetTime ).toLocalDateTime() );
}

if ( Calendar.class.isAssignableFrom( type ) ) {
return (X) GregorianCalendar.from( jdbcOffsetDateTime.toZonedDateTime() );
return (X) GregorianCalendar.from( offsetDateTimeAtEpoch( offsetTime ).toZonedDateTime() );
}

// for instants, we assume that the JDBC timezone, if any, is ignored

final Instant instant = offsetTime.atDate( LocalDate.EPOCH ).toInstant();

if ( Long.class.isAssignableFrom( type ) ) {
return (X) Long.valueOf( instant.toEpochMilli() );
return (X) Long.valueOf( instantAtEpoch( offsetTime ).toEpochMilli() );
}

if ( Date.class.isAssignableFrom( type ) ) {
return (X) Date.from( instant );
return (X) Date.from( instantAtEpoch( offsetTime ) );
}

throw unknownUnwrap( type );
}

private static Time withMillis(OffsetTime jdbcOffsetTime, Time time) {
final int nanos = jdbcOffsetTime.getNano();
if ( nanos == 0 ) {
return time;
}
else {
// Preserve milliseconds, which java.sql.Time supports
final long millis = DateTimeUtils.roundToPrecision( nanos, 3 ) / 1_000_000;
return new Time( time.getTime() + millis );
}
}

private static OffsetDateTime offsetDateTimeAtEpoch(OffsetTime jdbcOffsetTime) {
return jdbcOffsetTime.atDate( LocalDate.EPOCH );
}

private static Instant instantAtEpoch(OffsetTime offsetTime) {
return offsetDateTimeAtEpoch( offsetTime ).toInstant();
}

@Override
public <X> OffsetTime wrap(X value, WrapperOptions options) {
if ( value == null ) {
Expand All @@ -159,67 +171,39 @@ public <X> OffsetTime wrap(X value, WrapperOptions options) {
}

if (value instanceof LocalTime localTime) {
return localTime.atOffset( getCurrentSystemOffset() );
return localTime.atOffset( options.getSystemZoneOffset() );
}

if ( value instanceof OffsetDateTime offsetDateTime) {
return offsetDateTime.toOffsetTime();
}

/*
* Also, in order to fix HHH-13357, and to be consistent with the conversion to Time (see above),
* we set the offset to the current offset of the JVM (OffsetDateTime.now().getOffset()).
* This is different from setting the *zone* to the current *zone* of the JVM (ZoneId.systemDefault()),
* since a zone has a varying offset over time,
* thus the zone might have a different offset for the given timezone than it has for the current date/time.
* For example, if the timestamp represents 1970-01-01TXX:YY,
* and the JVM is set to use Europe/Paris as a timezone, and the current time is 2019-04-16-08:53,
* then applying the JVM timezone to the timestamp would result in the offset +01:00,
* but applying the JVM offset would result in the offset +02:00, since DST is in effect at 2019-04-16-08:53.
*
* Of course none of this would be a problem if we just stored the offset in the database,
* but I guess there are historical reasons that explain why we don't.
*/

// for legacy types, we assume that the JDBC timezone is passed to JDBC
// (since PS.setTime() and friends do accept a timezone passed as a Calendar)

if (value instanceof Time time) {
final OffsetTime offsetTime = time.toLocalTime()
.atOffset( getCurrentJdbcOffset( options) )
.withOffsetSameInstant( getCurrentSystemOffset() );
long millis = time.getTime() % 1000;
if ( millis == 0 ) {
return offsetTime;
}
if ( millis < 0 ) {
// The milliseconds for a Time could be negative,
// which usually means the time is in a different time zone
millis += 1_000L;
}
return offsetTime.with( ChronoField.NANO_OF_SECOND, millis * 1_000_000L );
// Use ZoneOffset rather than ZoneId in the conversion,
// since the offset for a zone varies over time, but
// a Time does not have an attached Date.
final OffsetTime offsetTime =
time.toLocalTime().atOffset( options.getJdbcZoneOffset() ) // the Timestamp is in the current JDBC timezone offset
.withOffsetSameInstant( options.getSystemZoneOffset() ); // convert back to the VM timezone
// Time.toLocalTime() strips off nanos
return withNanos( time, offsetTime );
}

if (value instanceof Timestamp timestamp) {
/*
* Workaround for HHH-13266 (JDK-8061577).
* Ideally we'd want to use OffsetDateTime.ofInstant( ts.toInstant(), ... ),
* but this won't always work since ts.toInstant() assumes the number of
* milliseconds since the epoch means the same thing in Timestamp and Instant,
* but it doesn't, in particular before 1900.
*/
return timestamp.toLocalDateTime().toLocalTime().atOffset( getCurrentJdbcOffset(options) )
.withOffsetSameInstant( getCurrentSystemOffset() );
return timestamp.toLocalDateTime()
.atZone( options.getJdbcZoneId() ) // the Timestamp is in the JDBC timezone
.withZoneSameInstant( ZoneId.systemDefault() ) // convert back to the VM timezone
.toOffsetDateTime().toOffsetTime(); // return the time part
}

if (value instanceof Date date) {
return OffsetTime.ofInstant( date.toInstant(), getCurrentSystemOffset() );
return OffsetTime.ofInstant( date.toInstant(), options.getSystemZoneOffset() );
}

// for instants, we assume that the JDBC timezone, if any, is ignored

if (value instanceof Long millis) {
return OffsetTime.ofInstant( Instant.ofEpochMilli(millis), getCurrentSystemOffset() );
return OffsetTime.ofInstant( Instant.ofEpochMilli(millis), options.getSystemZoneOffset() );
}

if (value instanceof Calendar calendar) {
Expand All @@ -229,17 +213,21 @@ public <X> OffsetTime wrap(X value, WrapperOptions options) {
throw unknownWrap( value.getClass() );
}

private static ZoneOffset getCurrentJdbcOffset(WrapperOptions options) {
if ( options.getJdbcTimeZone() != null ) {
return OffsetDateTime.now().atZoneSameInstant( options.getJdbcTimeZone().toZoneId() ).getOffset();
private static OffsetTime withNanos(Time time, OffsetTime offsetTime) {
final long millis = time.getTime() % 1000;
final long nanos;
if ( millis == 0 ) {
return offsetTime;
}
else if ( millis < 0 ) {
// The milliseconds for a Time could be negative,
// which usually means the time is in a different time zone
nanos = (millis + 1_000L) * 1_000_000L;
}
else {
return getCurrentSystemOffset();
nanos = millis * 1_000_000L;
}
}

private static ZoneOffset getCurrentSystemOffset() {
return OffsetDateTime.now().getOffset();
return offsetTime.with( ChronoField.NANO_OF_SECOND, nanos );
}

@Override
Expand Down
Loading
Loading