Simplify handling of Instant and OffsetDateTime and remove incorrect/dubious comments about JDBC 4.2

[jonenst]

In this PR I simplify the code based on the guarantees of the JDBC 4.2 standard.

There is a lot of confusion (EDIT: in the general discussions on the internet, not in the hibernate project) around JDBC 4.2 support for Instant and OffsetDateTime. I think it stems from subtle differences in precise words between "Support in JDBC" vs "Support in the JDBC driver" vs "Support in getObject/SetObject". see https://stackoverflow.com/a/76984102

TLDR:

• jdbc 4.2 does guarantee that getObject/setObject work for OffsetDateTime, there is absolutely no doubt about it.
• jdbc 4.2 can rightfully claim "safe" Instant support, albeit through weird APIs (not getObject/setObject, not based on driver support): jdbc 4.2 adds toInstant() and from(Instant) to java.sql.Timestamp which is part of JDBC and builds upon the existing getTimestamp/setTimestamp methods with a Calendar argument. There is absolutely no doubt that the JDBC 4.2. standard does not support Instant in getObject/setObject (but some drivers may).

EDIT: For context, this PR currently tries to remove as much code as possible as a proposal to see the actual effects in tests, but it's not meant to be merged as is necessarily.

NOTE: For OffsetDateTime, another strategy might be to keep support for non compliant JDBC 4.2 drivers, but do they really exist ? Are they worth the confusion ? I will let you decide.
NOTE2: For Instant, unless the jdbc standard adds support for Instant in getObject/setObject in a next version, I don't see any value in maintaining duplicated code (additionally the standard code is not that bad and required anyway) and it imposes a performance penalty (exception try catch) for compliant drivers.
NOTE3: the current code in the main branch for TimestampUtcAsOffsetDateTimeJdbcType already doesn't support non compliant drivers (no try catch), so this PR makes everything more homogeneous (unless the difference is on purpose because TimestampUtcAsOffsetDateTimeJdbcType is known to be used only with compliant drivers ? in which case maybe it's better to add a comment saying why TimestampWithTimeZoneJdbcType needs non compliant driver support but TimestampUtcAsOffsetDateTimeJdbcType doesn't ?)

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.

---

[hibernate-github-bot]

Thanks for your pull request!

This pull request does not follow the contribution rules. Could you have a look?

❌ All commit messages should start with a JIRA issue key matching pattern HHH-\d+
    ↳ Offending commits: [93f41f4]

› This message was automatically generated.

[jonenst]

If you want to proceed with this PR, please let me know how. For example I can create the HH-* jira issue if you want or let you do it and reference it in the commit message. Otherwise feel free to close this or take any inspiration you want from it and do as you want.

[gavinking]

There is a lot of confusion around JDBC 4.2 support for Instant and OffsetDateTime.
There is absolutely no doubt that the JDBC 4.2. standard does not support Instant in getObject/setObject (but some drivers may).

Yes, sure, the comment is incorrect.

The explanation (I checked the git history) is that when I first wrote that code back in 2020, it looked like this:

try {
	final OffsetDateTime dateTime = javaTypeDescriptor.unwrap( value, OffsetDateTime.class, wrapperOptions.getSession() );
	// supposed to be supported in JDBC 4.2
	st.setObject( name, dateTime, Types.TIMESTAMP_WITH_TIMEZONE );
}
catch (SQLException|AbstractMethodError e) {
	// fall back to treating it as a JDBC Timestamp
	final Timestamp timestamp = javaTypeDescriptor.unwrap( value, Timestamp.class, wrapperOptions.getSession() );
	st.setTimestamp( name, timestamp );
}

And when at some stage Christian changed it to use Instant, he forgot to update the comment.

This is the sort of thing that happens incredibly often in software development, and doesn't reflect any deep "confusion" or "dubious" beliefs about JDBC. I'm sure something like this has happened to you at some point, and if it hasn't yet, it will. Sometimes comments get out of sync with the core. It's not uncommon.

In this PR I simplify the code based on the guarantees of the JDBC 4.2 standard.

What Hibernate does with respect to date/time handling is rarely determined by what the JDBC spec says, but rather by what we found actually works when testing across a large range of actual JDBC drivers. However complicated you believe that supporting datetime types and operations uniformly across 14 database (not counting community-dialects), I can promise you it's way more complicated than that.

• JDCB drivers are of generally bad quality,
• databases have extremely idiosyncratic semantics around dates and times, and
• the JVM itself has multiple known bugs in conversions between java.sql and java.time stuff, which we don't expect to ever be fixed.

So I can absolutely 100% guarantee you that I had a perfectly good motivation for adding those catch blocks. I didn't just randomly add them due to some "confusion"; I added them because I absolutely needed to do so to get shit to work on some database we were testing against at the time. Now, perhaps they're no longer needed; that's definitely not impossible, since the code has changed in various ways since then, but it's very risky to make that determination without testing the community dialects.

And as you can see from the failing tests, naive simplifications of this code are incredibly likely to break things.

NOTE: For OffsetDateTime, another strategy might be to keep support for non compliant JDBC 4.2 drivers, but do they really exist ? Are they worth the confusion ? I will let you decide.

Of course they exist.

The "confusion" is what doesn't exist.

NOTE2: For Instant, unless the jdbc standard adds support for Instant in getObject/setObject in a next version, I don't see any value in maintaining duplicated code (additionally the standard code is not that bad and required anyway) and

If some drivers natively support Instant (which they do, or the code would not have been written like that), it's a good idea to make use of that support, because, in principle, the driver author has a much better understanding of the semantics of timestamps on the given database than we or you do.

it imposes a performance penalty (exception try catch) for compliant drivers.

Any trivial performance penalty is irrelevant because these particular JdbcTypes are extremely uncommon compared to, say, VarcharJdbcType.

Look, I appreciate that you're trying to help, but if you want to fuck with how Hibernate handles dates and times, let me promise you that you're going to have to adopt a much humbler attitude, since this stuff is an absolute nightmare and everyone who has ever touched it is suffering extreme PTSD from the experience. We do our best to make things behave sensibly and uniformly, but we're building on a foundation which is completely broken.

[gavinking]

So the error on db2 was:

       Caused by:
        com.ibm.db2.jcc.am.SqlSyntaxErrorException: [jcc][1091][10417][4.33.31] Invalid data conversion: Parameter instance 2025-05-02T10:15:51.269587778Z is invalid for the requested conversion. ERRORCODE=-4461, SQLSTATE=42815
            at app//com.ibm.db2.jcc.am.b4.a(b4.java:810)
            at app//com.ibm.db2.jcc.am.b4.a(b4.java:66)
            at app//com.ibm.db2.jcc.am.b4.a(b4.java:116)
            at app//com.ibm.db2.jcc.am.bf.a(bf.java:1550)
            at app//com.ibm.db2.jcc.am.bf.a(bf.java:1437)
            at app//com.ibm.db2.jcc.am.ko.a(ko.java:2714)
            at app//com.ibm.db2.jcc.am.ko.setObject(ko.java:2604)
            at app//org.hibernate.type.descriptor.jdbc.TimestampWithTimeZoneJdbcType$1.doBind(TimestampWithTimeZoneJdbcType.java:76)
            at app//org.hibernate.type.descriptor.jdbc.BasicBinder.bind(BasicBinder.java:59)
            at app//org.hibernate.engine.jdbc.mutation.internal.JdbcValueBindingsImpl.lambda$beforeStatement$0(JdbcValueBindingsImpl.java:85)
            ... 30 more

It was similar on MySQL:

        Caused by:
        java.sql.SQLFeatureNotSupportedException: Unsupported SQL type: TIMESTAMP_WITH_TIMEZONE
            at com.mysql.cj.jdbc.exceptions.SQLError.createSQLFeatureNotSupportedException(SQLError.java:242)
            at com.mysql.cj.jdbc.ClientPreparedStatement.setObject(ClientPreparedStatement.java:1924)
            at org.hibernate.type.descriptor.jdbc.TimestampWithTimeZoneJdbcType$1.doBind(TimestampWithTimeZoneJdbcType.java:76)
            at org.hibernate.type.descriptor.jdbc.BasicBinder.bind(BasicBinder.java:59)

On SQL Server it was:

       Caused by:
        com.microsoft.sqlserver.jdbc.SQLServerException: The conversion from datetime2 to DATETIMEOFFSET is unsupported.
            at app//com.microsoft.sqlserver.jdbc.SQLServerException.makeFromDriverError(SQLServerException.java:242)
            at app//com.microsoft.sqlserver.jdbc.DataTypes.throwConversionError(DataTypes.java:1118)
            at app//com.microsoft.sqlserver.jdbc.ServerDTVImpl.getValue(dtv.java:3696)
            at app//com.microsoft.sqlserver.jdbc.DTV.getValue(dtv.java:251)
            at app//com.microsoft.sqlserver.jdbc.Column.getValue(Column.java:190)
            at app//com.microsoft.sqlserver.jdbc.SQLServerResultSet.getValue(SQLServerResultSet.java:2105)
            at app//com.microsoft.sqlserver.jdbc.SQLServerResultSet.getValue(SQLServerResultSet.java:2091)
            at app//com.microsoft.sqlserver.jdbc.SQLServerResultSet.getDateTimeOffset(SQLServerResultSet.java:2796)
            at app//com.microsoft.sqlserver.jdbc.SQLServerResultSet.getObject(SQLServerResultSet.java:2479)
            at app//org.hibernate.type.descriptor.jdbc.TimestampWithTimeZoneJdbcType$2.doExtract(TimestampWithTimeZoneJdbcType.java:97)
            at app//org.hibernate.type.descriptor.jdbc.BasicExtractor.extract(BasicExtractor.java:42)

On h2 we had many failures like:

InstantTest > [2,017-11-6T19:19:1.0Z [JVM TZ: UTC-08:00, JDBC TZ: GMT, remapping dialect: null]] > writeThenRead[2,017-11-6T19:19:1.0Z [JVM TZ: UTC-08:00, JDBC TZ: GMT, remapping dialect: null]] FAILED
    java.lang.AssertionError: Writing then reading a value should return the original value expected:<2017-11-06T19:19:01Z> but was:<2017-11-07T03:19:01Z>
        at org.hibernate.orm.test.type.AbstractJavaTimeTypeTest.lambda$writeThenRead$2(AbstractJavaTimeTypeTest.java:126)

So this changed caused at least three different kinds of failures on four databases.

Welcome to the reality of dealing with JDBC as it actually exists!

[jonenst]

Hi, I'm sorry if my message came as not humble, that's not what I meant at all. I was just trying to be succinct to make it easier for reviewers to get to the point more rapidly. I also never expected this PR to be merged as is, but to be a basis for CI tests and open questions. Sorry for not explicitly stating that.

For what it's worth, when I said "There is a lot of confusion around JDBC 4.2 support for Instant and OffsetDateTime." I didn't mean specific confusion of hibernate developpers or source code, I meant in the general discussions about this on the internet. I'll edit my original message so as not cast doubt on the quality of the hibernate project and the work by all the people behind it, for which I have very high respect.

I also completely agree with you that you know better, hence why I wrote "I will let you decide." and "feel free to close". After rereading that sentence I now see that it may look ironic but it was truly not, sorry about that.

So I guess the little value in this PR is showing where comments can be improved:

• for Instant, this information could be a good comment ? "If some drivers natively support Instant (which they do, or the code would not have been written like that), it's a good idea to make use of that support, because, in principle, the driver author has a much better understanding of the semantics of timestamps on the given database than we or you do."
• for OffsetDateTime, maybe adding a comment explaining why TimestampWithTimeZoneJdbcType needs non compliant driver support but TimestampUtcAsOffsetDateTimeJdbcType doesn't ? Or maybe I just misunderstood something

Again, just as you said, I was just trying to help. I'm sorry it forced you to take some time to explain and prove that hibernate is a high quality project, I never intented to insinuate the contrary.

Have a good day

(ps: you are also right in saying that my work has problems !)

[gavinking]

No worries, I'm not yelling at you so much as just expressing my general frustration at the whole setup where we have how many trillions of dollars moving through JDBC and nobody actually seems to give a fuck about maintaining it.

you are also right in saying that my work has problems

It's not that your work or our work has problems; it's that the whole underlying infrastructure for handling database dates and times is just a completely broken mess and nobody cares enough to fix it.