vertx-sql-client icon indicating copy to clipboard operation
vertx-sql-client copied to clipboard

[Postgres] Accept LocalDateTime (and similar types without a timezone) as parameter for a column of type timestamptz

Open DavideD opened this issue 3 years ago • 9 comments

This is started as an issue for Quarkus.

The problem is that when passing a LocalDateTime (or some similar type without a timezone) as parameter of an insert query we get the following error:

io.vertx.core.impl.NoStackTraceThrowable: Parameter at position[0] with class = [java.time.LocalDateTime] and value = [2020-07-16T10:10:24.378] can not be coerced to the expected class = [java.time.OffsetDateTime] for encoding.

Timestamptz doesn't really store the timezone and it's just state that everything is converted to UTC. So I'm not sure why it would be a problem to save the LocalDateTime by assigning it an UTC timezone and convert it to an OffsetDateTime.

This is an issue for Hibernate Reactive because we don't know the type of the column on the database and we just assume that a timestamp will work.

DavideD avatar Jan 28 '22 10:01 DavideD

Any update with this issue ?

AlexandreGuidin avatar Apr 24 '22 17:04 AlexandreGuidin

Hi 🙋🏻‍♂️ I'm one of the contributor for temporal type.

That's correct the time zone is in user space and in the server is saved as UTC that's how postgres works even if the host time zone is different.

I don't think it makes sense to allow local date and time.

In addition in the early development for the reactive client we decided to take the maximum advantage of new temporal api in jdk 1.8

I'm not sure if there is a workaround to solve the issue.

EmadAlblueshi avatar Jul 04 '22 22:07 EmadAlblueshi

I don't think it makes sense to allow local date and time.

Why not?

DavideD avatar Jul 05 '22 09:07 DavideD

I don't think it makes sense to allow local date and time.

Why not?

Yah, good question. I'm not seeing that at all.

gavinking avatar Jul 05 '22 09:07 gavinking

LocalDate, LocalDateTime, OffsetDate, OffsetDateTime, ZonedDate, ZonedDateTime are important classes in the java.time package. PostgreSql's timestampz (timestamp with timezone) handles OffsetDateTime and ZonedDateTime very well.

I was surprised recently when trying to port a project from quarkus-hibernate-orm to quarkus-hibernate-reactive to learn that the vertex didn't support persisting these classes into the pg timestampz data type.

Now, a custom hibernate UserType couple be implemented BUT implementing a UserType for foundational items like "time" isn't attractive especially when the types are native to Java time package, Postgresql has a type that can handle these classes and the Java class and Pg type have been mappable in Hibernate without issue since the Java classes appeared in the JDK in version 8.

Even if postgresql by default stores the user space data as UTC in a distributed and cloud environment where developers can't always depend on who and how database systems are administered having that extra time and offset/zone information cuts down on noise and doubt when coding around a process that one does not fully control. Some of us have to handle the data model we are given. In our case it is timestampz which is why I added my $0.02 to https://github.com/quarkusio/quarkus/issues/10768.

To be more concise, if only for that need to support existing production systems handling these types seems reasonable and makes sense.

In the above use case scenario OffsetDateTime / ZonedDateTime in relation to timestampz is pretty important. In the business application process no assumptions are needed with regard to system operations on the "when" part of a coding requirement.

That the jdbc driver that pre-dates the non-blocking efforts in vertx has handled this java class and type mapping for 9+ years would seem to justify it making sense to also support these classes. Meaning it should make sense that JDK classes are being used along with the corresponding pg datatype in production systems.

In any event, it would be a plus if the driver could be depended upon to handle the date and time datatypes that are part of the JDK gracefully. That would also seem to make sense.

I think the "Why not?" question raised by @DavideD above is a fair question.

The classes are in the JDK and there is a datatype in the database that handles the data from these classes correctly. So, why not support?

Support would be a good thing.

gesker avatar Jul 17 '22 09:07 gesker

As an implementor of a fully compliant JDBC driver for PostgreSQL (http://github.com/impossibl/pgjdbc-ng)... please let me suggest allowing local times here is the wrong path.

JDBC did this in its first couple versions; assuming a lot with about timezones and how the driver should be expected to convert them. It's impossible to get correct. One person's "common sense" doesn't work for everybody. This was a nightmare to implement and I really don't want to see this mistake repeated.

The wisdom of this is seen with JDBC 4.2's addition of java.time types. They explicitly disallow "local" date/times in explicit fields. E.g. you cannot insert a LocalDateTime into a timestamp with timezone. As far as I remember, even ZonedDateTime isn't supported by the specification.

If you want to store a LocalDateTime into timestamp with timezone convert it to your preferred timezone first and trust that the PostgreSQL will faithfully reproduce it, regardless of how it's stored.

kdubb avatar Sep 26 '22 18:09 kdubb

I still don't get what the problem is, though. As far as I understand it, there is no convertion to do and the type is always UTC.

If you want to store a LocalDateTime into timestamp with timezone convert it to your preferred timezone first and trust that the PostgreSQL will faithfully reproduce it, regardless of how it's stored.

Hibernate doesn't know the type of the column on the database, so we don't know when to add the timezone.

If I understand correctly, what you are asking is to add a timezone to a value so that the driver can ignore it because ... checking notes ... the timezone is always stored as UTC.

I'm not asking to add it to a standard because different databases might behave in different ways, but I still don't get it why it cannot work for Postgres.

DavideD avatar Sep 27 '22 08:09 DavideD

I was checking the PostgreSQL documentation about timezones and I've learned that passing a timestamp without a time zone is a valid use case:

For timestamp with time zone, the internally stored value is always in UTC (Universal Coordinated Time, traditionally known as Greenwich Mean Time, GMT). An input value that has an explicit time zone specified is converted to UTC using the appropriate offset for that time zone.

If no time zone is stated in the input string, then it is assumed to be in the time zone indicated by the system's TimeZone parameter, and is converted to UTC using the offset for the timezone zone.

So, it's still not clear to me why we cannot accept a LocalDateTime as parameter (it doesn't require any conversion). Note that we could throw an exception for other databases if this is not allowed.

DavideD avatar Oct 31 '22 09:10 DavideD

because that https://github.com/eclipse-vertx/vertx-sql-client/issues/1283 was linked here, I really don't care if they are all supported in on go or each in a separate move, but Instant in Java is actually the true equivalent of OffsetDateTime in Postgres as it is how Postgres stores the timestamp internally (in UTC without timezone or offset). so this is different story than supporting an additional type.

mohamnag avatar Jun 21 '23 08:06 mohamnag