Analysis should catch mismatch for types with or without timezone in PostgreSQL instances

[nigredo-tori]

• Writing or reading OffsetDateTime or Instant as TIMESTAMP is generally an error.
• Writing or reading LocalDateTime as TIMESTAMPTZ is generally an error.

This was highlighted for me by a recent-ish change, where the implementation for corresponding instances for PostgreSQL has changed. For example, where previously Instant was passed to the database as java.util.Date, with the system timezone, it is now passed as OffsetDateTime with zero offset. This means that any code that was writing Instant to TIMESTAMP columns was silently broken by this. Reading this data has its own set of problems.

The most annoying part for me is that the analysis checks in place don't catch that, because (AFAICT) pgjdbc doesn't correctly label output columns types as TIMESTAMP_WITH_TIMEZONE to preserve compatibility with Hibernate. 🤦 Maybe there is some other piece of metadata we can use here? If, say, getColumnTypeName produces different values for TIMESTAMP and TIMESTAMPTZ, we can use that to disambiguate the two. Although I imagine that would require an overhaul of the Get and Meta machinery.

[nigredo-tori]

I've done some testing, and getColumnTypeName seems to produce "timestamp" and "timestamptz" depending on the type. So we can definitely use it to disambiguate the two.

[jatcwang]

Yeah I did try doing this when adding first-class PostgreSQL java.time support but observed the same thing you did. I think we could switch to use getColumnTypeName, but I think that'll require a rewrite of the analyzer to be DB-specific as well as the instances (e.g. Put[Instant]) now needs to specify their database type. Worth exploring, but probably a lot of work with breaking changes.

[nigredo-tori]

a rewrite of the analyzer to be DB-specific

Or we can move some of the analysis logic to the instances, since they are already, effectively, DB-specific. I.e. Get can check a column type and column type name from the metadata, and tell whether those match the expected type. I imagine this can even be done without breaking binary compatibility, if we preserve all the case class machinery for Get and Put - although that will certainly be noisy and ugly.