Обсуждение: Timestamp changes committed to HEAD

I've committed my Timestamp/Date/Time changes to CVS HEAD; here's the
commit message. With these changes {get,set}{Timestamp,Date,Time} should
respect Calendars properly and not make any assumptions about server
timezone vs. JVM timezone.

Assuming this works as people expect (please test!), any thoughts on
whether this is suitable to backport to 8.0? It *is* a big set of
changes, but the stable driver is pretty broken in this area as it stands.

-O


Timestamp/Time/Date work per discussion on pgsql-jdbc.

Rewrote quite a bit of TimestampUtils so that timezones are correctly
handled both when parsing and when stringizing values.

TimestampUtils is now an instance attached to the Connection so we can
have different behaviour for 7.3 vs. 7.4 (7.3 does not allow timezone
specifications in time literals).

Added TimezoneTest to test all the various legal conversions when
Calendars are involved.

Server versions and what they support:

  8.0: everything works
  7.4: everything works
  7.3: setTime()->timetz, setTimestamp()->time fail
  7.2: setTime()->timetz, setTimestamp()->time, +1300 timezones fail

The problem with setTime() on timetz is that we can't pass the timezone
down in <=7.3 in case it's actually a time value that is expected, so
the timetz will always end up with the server's TimeZone regardless of
the Calendar passed.

A similar problem affects setTimestamp() as it must pass a timezone down
in case the expected type is timestamptz, which will fail if the
expected type is actually time.

We can't fix these cases by asking the server for the actual type
expected, as that requires protocol v3 which 7.2/7.3 don't support.

The +1300 timezone problem is a server bug in 7.2 (possibly fixed in
later 7.2 releases, haven't checked); +1300 is a legal timezone but 7.2
doesn't accept it.

Oid.UNKNOWN is now used for setTimestamp() and setDate(). It's
unfortunately necessary for setDate() to handle setDate(i,date,calendar)
when the underlying column is a timestamptz, for much the same reasons
as the timestamp/timestamptz issue -- we need to avoid an unwanted
rotation by the server's TimeZone setting.

Cleaned up some of the tests in TimestampTest so they don't break when
the JVM's timezone is different to the server's timezone.

Still broken: CallableStatement.get{Date,Time,Timestamp} with Calendar.
Fixing this will like require some rearrangement of when OUT parameters
are turned into Objects.

On Mon, 1 Aug 2005, Oliver Jowett wrote:

> I've committed my Timestamp/Date/Time changes to CVS HEAD; here's the
> commit message. With these changes {get,set}{Timestamp,Date,Time} should
> respect Calendars properly and not make any assumptions about server
> timezone vs. JVM timezone.
>

To aid in people's testing I've put up a prebuilt jar file here:

http://www.ejurka.com/pgsql/jars/ts/

Kris Jurka

On 8/1/05, Oliver Jowett <oliver@opencloud.com> wrote:
> I've committed my Timestamp/Date/Time changes to CVS HEAD; here's the
> commit message. With these changes {get,set}{Timestamp,Date,Time} should
> respect Calendars properly and not make any assumptions about server
> timezone vs. JVM timezone.

This driver version works better with {get,set}Timestamp than any
other versions I've tested before.

My tests use three 8.03 severs with server timezones UTC, CET and
America/St_Johns (Newfoundland).

Timestamps 1970-01-01 00:00:00+0000, 2000-01-01 00:00:00+0000 and
2000-07-01 00:00:00+0000 are written to TIMESTAMP WITH TIME ZONE and
TIMESTAMP WITHOUT TIME ZONE fields, read, and then compared to the
original values. Additionally, the values in the database were checked
with pgAdmin III as a client.

Everything seems to be OK, except that read operations from the
America/St_Johns server lead to a deviation of minus half an hour,
probably due to a rounding error in the driver's getTimestamp
implementation.

Regards,
Alex

emergency.shower@gmail.com wrote:

> Everything seems to be OK, except that read operations from the
> America/St_Johns server lead to a deviation of minus half an hour,
> probably due to a rounding error in the driver's getTimestamp
> implementation.

Ok, it looks like there is another longstanding bug in TimestampUtils
that I didn't find: it just doesn't parse timezones with non-zero minute
offsets at all (there's code there that tries to do it, but it's incorrect)

I'll commit a fix to CVS shortly, once I've sorted out a test to go with it.

-O

On Mon, 1 Aug 2005, Oliver Jowett wrote:

> I've committed my Timestamp/Date/Time changes to CVS HEAD.

The regression tests fail on 7.2 and 7.3 with the below stacktrace.  I
understand that we may not be able to achieve 100% correctness with
7.2/7.3, but aren't there at least certain cases that work?

     [junit] Testcase: testSetTimestamp(org.postgresql.test.jdbc2.TimezoneTest):    Caused an ERROR
     [junit] ERROR:  Bad time external representation '2005-01-01 13:00:00.000000 +0100'
     [junit] org.postgresql.util.PSQLException: ERROR:  Bad time external representation '2005-01-01 13:00:00.000000
+0100'
     [junit]     at org.postgresql.core.v2.QueryExecutorImpl.receiveErrorMessage(QueryExecutorImpl.java:542)
     [junit]     at org.postgresql.core.v2.QueryExecutorImpl.processResults(QueryExecutorImpl.java:466)
     [junit]     at org.postgresql.core.v2.QueryExecutorImpl.execute(QueryExecutorImpl.java:364)
     [junit]     at org.postgresql.core.v2.QueryExecutorImpl.execute(QueryExecutorImpl.java:258)
     [junit]     at org.postgresql.jdbc2.AbstractJdbc2Statement.execute(AbstractJdbc2Statement.java:430)
     [junit]     at org.postgresql.jdbc2.AbstractJdbc2Statement.executeWithFlags(AbstractJdbc2Statement.java:346)
     [junit]     at org.postgresql.jdbc2.AbstractJdbc2Statement.executeUpdate(AbstractJdbc2Statement.java:300)
     [junit]     at org.postgresql.test.jdbc2.TimezoneTest.testSetTimestamp(TimezoneTest.java:310)

Kris Jurka

Kris Jurka wrote:
>
>
> On Mon, 1 Aug 2005, Oliver Jowett wrote:
>
>> I've committed my Timestamp/Date/Time changes to CVS HEAD.
>
>
> The regression tests fail on 7.2 and 7.3 with the below stacktrace.  I
> understand that we may not be able to achieve 100% correctness with
> 7.2/7.3, but aren't there at least certain cases that work?

There certainly are cases that work, but that test isn't one of them.
Per my commit comments, this test is the case of setTimestamp() on a
'time without timezone' column. 7.4 was the first version to accept (and
discard) a timezone in this case. We don't know ahead of time that the
target parameter is a 'time without timezone' (and can't for pre-7.4
even if we wanted to, since there's no V3 protocol support), and if we
omitted the timezone information then using setTimestamp() against
timestamp fields would produce incorrect results.

If you're suggesting we skip that particular case so we can run the rest
of the testcase on 7.2/7.3, it's a good idea but more work than I had
time for -- feel free to update the test as necessary.

-O