PostgreSQL JDBC - work in progress: timestamp patch

Author

work in progress: timestamp patch

Oliver Jowett

2005-07-26, 3:24 am

Here's the current state of my changes to use Oid.INVALID to make the
server decide between timestamp and timestamptz. Warning: work in progress.

I haven't fixed CallableStatement.getTimestamp() (for OUT parameters)
yet as it appears to be turning the return value into a Timestamp too
early at the moment .. seems a bit messy to fix.

Can the various protagonists try this patch out and see if it fixes
their problems?

-O

Dave Cramer

2005-07-26, 3:24 am

Oliver,

I've been messing with it a bit myself, and noticed that
TimeStampUtils.toString used the timezone of the incoming
timestamp, not the calendar. So the call to appendTimeZone, passes in
the timestamp, not the new calendar.

Also I've added functionality to track the servers's timezone so that
stmt.execute("set timezone='newtimezon
e'") allows you to
programatically set the server timezone to UTC. Which would be
necessary to get Christian's problem to work even using Oid.INVALID
( I think )

I'll try your patch in the morning though.

Dave

On 25-Jul-05, at 8:02 PM, Oliver Jowett wrote:

> Here's the current state of my changes to use Oid.INVALID to make the
> server decide between timestamp and timestamptz. Warning: work in
> progress.
>
> I haven't fixed CallableStatement.getTimestamp() (for OUT parameters)
> yet as it appears to be turning the return value into a Timestamp too
> early at the moment .. seems a bit messy to fix.
>
> Can the various protagonists try this patch out and see if it fixes
> their problems?
>
> -O
> ? build.local.properties
> Index: org/postgresql/jdbc2/ AbstractJdbc2ResultS
et.java
> ====================
====================
====================
=======
> RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/
> AbstractJdbc2ResultS
et.java,v
> retrieving revision 1.75
> diff -c -r1.75 AbstractJdbc2ResultS
et.java
> *** org/postgresql/jdbc2/ AbstractJdbc2ResultS
et.java 8 Jun 2005
> 01:44:02 -0000 1.75
> --- org/postgresql/jdbc2/ AbstractJdbc2ResultS
et.java 26 Jul 2005
> 00:00:23 -0000
> ***************
> *** 139,145 ****
> case Types.TIME:
> return getTime(columnIndex)
;
> case Types.TIMESTAMP:
> ! return getTimestamp(columnI
ndex);
> case Types.BINARY:
> case Types.VARBINARY:
> case Types.LONGVARBINARY:
> --- 139,145 ----
> case Types.TIME:
> return getTime(columnIndex)
;
> case Types.TIMESTAMP:
> ! return getTimestamp(columnI
ndex, null);
> case Types.BINARY:
> case Types.VARBINARY:
> case Types.LONGVARBINARY:
> ***************
> *** 415,429 ****
>
> public Timestamp getTimestamp(int i, java.util.Calendar cal)
> throws SQLException
> {
> ! // apply available calendar if there is no timezone
> information
> ! if (cal == null || getPGType(i).endsWith("tz") )
> ! return getTimestamp(i);
> ! java.util.Date tmp = getTimestamp(i);
> ! if (tmp == null)
> ! return null;
> ! Calendar _cal = (Calendar)cal.clone();
> ! _cal =
> org.postgresql.jdbc2. AbstractJdbc2Stateme
nt.changeTime(tmp, _cal,
> false);
> ! return new java.sql.Timestamp(_cal.getTime().getTime());
> }
>
>
> --- 415,432 ----
>
> public Timestamp getTimestamp(int i, java.util.Calendar cal)
> throws SQLException
> {
> ! this.checkResultSet(i);
> !
> ! if (cal == null) {
> ! cal = new GregorianCalendar();

> ! } else {
> ! cal = (Calendar)cal.clone();
> ! }
> !
> ! // If this is actually a timestamptz, the server-provided
> timezone will override
> ! // the one we pass in, which is the desired behaviour.
> Otherwise, we'll
> ! // interpret the timezone-less value in the provided
> timezone.
> ! return TimestampUtils.toTimestamp(cal, getString(i));
> }
>
>
> ***************
> *** 2098,2107 ****
>
> public Timestamp getTimestamp(int columnIndex) throws
> SQLException
> {
> ! this. checkResultSet(colum
nIndex);
> ! if (calendar == null)
> ! calendar = new GregorianCalendar();

> ! return TimestampUtils. toTimestamp(calendar
, getString
> (columnIndex));
> }
>
> public InputStream getAsciiStream(int columnIndex) throws
> SQLException
> --- 2101,2107 ----
>
> public Timestamp getTimestamp(int columnIndex) throws
> SQLException
> {
> ! return getTimestamp(columnI
ndex, null);
> }
>
> public InputStream getAsciiStream(int columnIndex) throws
> SQLException
> ***************
> *** 2261,2267 ****
>
> public Timestamp getTimestamp(String columnName) throws
> SQLException
> {
> ! return getTimestamp(findCol
umn(columnName));
> }
>
> public InputStream getAsciiStream(Strin
g columnName) throws
> SQLException
> --- 2261,2267 ----
>
> public Timestamp getTimestamp(String columnName) throws
> SQLException
> {
> ! return getTimestamp(findCol
umn(columnName), null);
> }
>
> public InputStream getAsciiStream(Strin
g columnName) throws
> SQLException
> Index: org/postgresql/jdbc2/ AbstractJdbc2Stateme
nt.java
> ====================
====================
====================
=======
> RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/
> AbstractJdbc2Stateme
nt.java,v
> retrieving revision 1.79
> diff -c -r1.79 AbstractJdbc2Stateme
nt.java
> *** org/postgresql/jdbc2/ AbstractJdbc2Stateme
nt.java 8 Jul 2005
> 17:38:29 -0000 1.79
> --- org/postgresql/jdbc2/ AbstractJdbc2Stateme
nt.java 26 Jul 2005
> 00:00:31 -0000
> ***************
> *** 1301,1317 ****
> */
> public void setTimestamp(int parameterIndex, Timestamp x)
> throws SQLException
> {
> ! checkClosed();
> ! if (null == x)
> ! {
> ! setNull(parameterInd
ex, Types.TIMESTAMP);
> ! }
> ! else
> ! {
> ! if (calendar == null)
> ! calendar = new GregorianCalendar();

> ! bindString(parameter
Index, TimestampUtils.toString
> (sbuf, calendar, x), Oid.TIMESTAMPTZ);
> ! }
> }
>
> private void setCharacterStreamPo
st71(int parameterIndex,
> InputStream x, int length, String encoding) throws SQLException
> --- 1301,1307 ----
> */
> public void setTimestamp(int parameterIndex, Timestamp x)
> throws SQLException
> {
> ! setTimestamp(paramet
erIndex, x, null);
> }
>
> private void setCharacterStreamPo
st71(int parameterIndex,
> InputStream x, int length, String encoding) throws SQLException
> ***************
> *** 2886,2899 ****
> public void setTimestamp(int i, Timestamp t,
> java.util.Calendar cal) throws SQLException
> {
> checkClosed();
> ! if (cal == null)
> ! setTimestamp(i, t);
> ! else
> ! {
> ! Calendar _cal = (Calendar)cal.clone();
> ! _cal = changeTime(t, _cal, true);
> ! setTimestamp(i, new java.sql.Timestamp(_cal.getTime
> ().getTime()));
> }
> }
>
> // ** JDBC 2 Extensions for CallableStatement**
> --- 2876,2926 ----
> public void setTimestamp(int i, Timestamp t,
> java.util.Calendar cal) throws SQLException
> {
> checkClosed();
> !
> ! if (t == null) {
> ! setNull(i, Types.TIMESTAMP);
> ! return;
> }
> +
> + if (cal == null) {
> + if (calendar == null)
> + calendar = new GregorianCalendar();

> + cal = calendar;
> + } else {
> + cal = (Calendar)cal.clone();
> + }
> +
> + // Use INVALID as a compromise to get both TIMESTAMP and
> TIMESTAMPTZ working.
> + // This is because you get this in a +1300 timezone:
> + //
> + // template1=# select '2005-01-01 15:00:00
> +1000'::timestamptz;

> + // timestamptz
> + // ------------------------
> + // 2005-01-01 18:00:00+13
> + // (1 row)
> +
> + // template1=# select '2005-01-01 15:00:00
> +1000'::timestamp;
> + // timestamp
> + // ---------------------
> + // 2005-01-01 15:00:00
> + // (1 row)
> +
> + // template1=# select '2005-01-01 15:00:00
> +1000'::timestamptz:
:timestamp;
> + // timestamp
> + // ---------------------
> + // 2005-01-01 18:00:00
> + // (1 row)
> +
> + // So we want to avoid doing a timestamptz -> timestamp
> conversion, as that
> + // will first convert the timestamptz to an equivalent
> time in the server's
> + // timezone (+1300, above), then turn it into a timestamp
> with the "wrong"
> + // time compared to the string we originally provided.
> But going straight
> + // to timestamp is OK as the input parser for timestamp
> just throws away
> + // the timezone part entirely. Since we don't know ahead
> of time what type
> + // we're actually dealing with, INVALID seems the lesser
> evil, even if it
> + // does give more scope for type-mismatch errors being
> silently hidden.
> +
> + bindString(i, TimestampUtils.toString(sbuf, cal, t),
> Oid.INVALID); // Let the server infer the right type.
> }
>
> // ** JDBC 2 Extensions for CallableStatement**
> Index: org/postgresql/jdbc2/TimestampUtils.java
> ====================
====================
====================
=======
> RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/
> TimestampUtils.java,v
> retrieving revision 1.13
> diff -c -r1.13 TimestampUtils.java
> *** org/postgresql/jdbc2/TimestampUtils.java 15 Feb 2005
> 08:31:47 -0000 1.13
> --- org/postgresql/jdbc2/TimestampUtils.java 26 Jul 2005
> 00:00:32 -0000
> ***************
> *** 31,37 ****
> * Load date/time information into the provided calendar
> * returning the fractional seconds.
> */
> ! private static int loadCalendar(Gregori
anCalendar cal, String
> s, String type) throws SQLException {
> int slen = s.length();
>
> // Zero out all the fields.
> --- 31,37 ----
> * Load date/time information into the provided calendar
> * returning the fractional seconds.
> */
> ! private static int loadCalendar(Calenda
r cal, String s,
> String type) throws SQLException {
> int slen = s.length();
>
> // Zero out all the fields.
> ***************
> *** 167,173 ****
> *
> * @throws SQLException if there is a problem parsing s.
> **/
> ! public static Timestamp toTimestamp(Gregoria
nCalendar cal,
> String s) throws SQLException
> {
> if (s == null)
> return null;
> --- 167,173 ----
> *
> * @throws SQLException if there is a problem parsing s.
> **/
> ! public static Timestamp toTimestamp(Calendar
cal, String s)
> throws SQLException
> {
> if (s == null)
> return null;
> ***************
> *** 184,195 ****
> }
>
> synchronized(cal) {
> - cal.set(Calendar.ZONE_OFFSET, 0);
> - cal.set(Calendar.DST_OFFSET, 0);
> int nanos = loadCalendar(cal, s, "timestamp");
>
> Timestamp result = new Timestamp(cal.getTime().getTime
> ());
> result.setNanos(nanos);
> return result;
> }
> }
> --- 184,194 ----
> }
>
> synchronized(cal) {
> int nanos = loadCalendar(cal, s, "timestamp");
>
> Timestamp result = new Timestamp(cal.getTime().getTime
> ());
> result.setNanos(nanos);
> +
> return result;
> }
> }
> ***************
> *** 254,260 ****
> }
> }
>
> ! public static String toString(StringBuffe
r sbuf,
> GregorianCalendar cal, Timestamp x) {
> synchronized(sbuf) {
> synchronized(cal) {
> cal.setTime(x);
> --- 253,259 ----
> }
> }
>
> ! public static String toString(StringBuffe
r sbuf, Calendar
> cal, Timestamp x) {
> synchronized(sbuf) {
> synchronized(cal) {
> cal.setTime(x);
> ***************
> *** 276,282 ****
> }
> }
>
> ! public static String toString(StringBuffe
r sbuf,
> GregorianCalendar cal, Date x) {
> synchronized(sbuf) {
> synchronized(cal) {
> cal.setTime(x);
> --- 275,281 ----
> }
> }
>
> ! public static String toString(StringBuffe
r sbuf, Calendar
> cal, Date x) {
> synchronized(sbuf) {
> synchronized(cal) {
> cal.setTime(x);
> ***************
> *** 295,301 ****
> }
> }
>
> ! public static String toString(StringBuffe
r sbuf,
> GregorianCalendar cal, Time x) {
> synchronized(sbuf) {
> synchronized(cal) {
> cal.setTime(x);
> --- 294,300 ----
> }
> }
>
> ! public static String toString(StringBuffe
r sbuf, Calendar
> cal, Time x) {
> synchronized(sbuf) {
> synchronized(cal) {
> cal.setTime(x);
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
>

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Oliver Jowett

2005-07-26, 3:24 am

Dave Cramer wrote:

> I've been messing with it a bit myself, and noticed that
> TimeStampUtils.toString used the timezone of the incoming
> timestamp, not the calendar. So the call to appendTimeZone, passes in
> the timestamp, not the new calendar.

Yeah, looking at the stringizing TimestampUtils code is my next step.
I've just overhauled the parsing side of things to correctly use passed
Calendars (for getTimestamp() and friends)

> Also I've added functionality to track the servers's timezone so that
> stmt.execute("set timezone='newtimezon
e'") allows you to
> programatically set the server timezone to UTC. Which would be
> necessary to get Christian's problem to work even using Oid.INVALID ( I
> think )

I don't think it's necessary to track the server timezone at all..
what's the case where this goes wrong?

I'll send you an updated version of my patch offlist (it's changing
fairly fast at the moment so I'll avoid spamming the list with every
revision)

-O

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Dave Cramer

2005-07-26, 3:24 am

Oliver,

Dave
On 25-Jul-05, at 11:19 PM, Oliver Jowett wrote:

> Dave Cramer wrote:
>
>
>
> Yeah, looking at the stringizing TimestampUtils code is my next step.
> I've just overhauled the parsing side of things to correctly use
> passed
> Calendars (for getTimestamp() and friends)
>
>
>
> I don't think it's necessary to track the server timezone at all..
> what's the case where this goes wrong?

One of the times he is trying to stick in the db is a non-existant
time if it is associated with a
time zone

stmt.execute("INSERT INTO Foo (t1) VALUES ('2005-04-03 02:29:43.0')");

in any US timezone this time does not exist. In US/Mountain timezone
this
inserts as 2005-04-03 03:29:43.0' note the hour has incremented from
2 to 3

The only way to insert this time correctly is to use UTC

>
> I'll send you an updated version of my patch offlist (it's changing
> fairly fast at the moment so I'll avoid spamming the list with every
> revision)
Ok, I'll test it in the morning... Thanks
>
> -O
>
>

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Oliver Jowett

2005-07-26, 3:24 am

Dave Cramer wrote:

> One of the times he is trying to stick in the db is a non-existant time
> if it is associated with a
> time zone
>
> stmt.execute("INSERT INTO Foo (t1) VALUES ('2005-04-03 02:29:43.0')");
>
> in any US timezone this time does not exist. In US/Mountain timezone this
> inserts as 2005-04-03 03:29:43.0' note the hour has incremented from 2
> to 3

What's the underlying column, timestamp or timestamptz?

How is the value represented on the Java side? Seems like if you used
Timestamp + a UTC calendar on both the set and get paths, it should work
fine with no need to mess with the server timezone (assuming untyped
parameters)?

-O

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Oliver Jowett

2005-07-26, 3:24 am

Dave Cramer wrote:

> I've been messing with it a bit myself, and noticed that
> TimeStampUtils.toString used the timezone of the incoming
> timestamp, not the calendar. So the call to appendTimeZone, passes in
> the timestamp, not the new calendar.

I looked at this and the current code is certainly wrong. The timezone
offset of a Timestamp (deprecated method!) returns the offset of the
JVM's default timezone always. We should indeed be passing the target
calendar and using that.

I've added that change to my patch. Interestingly none of the regression
tests fail with it changed; we're very short on tests that actually test
the with-Calendar code..

-O

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Dave Cramer

2005-07-26, 7:23 am

On 26-Jul-05, at 1:23 AM, Oliver Jowett wrote:

> Dave Cramer wrote:
>
>
>
> I looked at this and the current code is certainly wrong. The timezone
> offset of a Timestamp (deprecated method!) returns the offset of the
> JVM's default timezone always. We should indeed be passing the target
> calendar and using that.
>
> I've added that change to my patch. Interestingly none of the
> regression
> tests fail with it changed; we're very short on tests that actually
> test
> the with-Calendar code..
Well, I actually think this little change is more of the problem than
anything else.
The reason it doesn't fail is because the timezone of the server and
the client are the same.

Did you manage to cobble together a patch for me to test ?

>
> -O
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
>
>

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Oliver Jowett

2005-07-26, 7:23 am

Dave Cramer wrote:
>
> On 26-Jul-05, at 1:23 AM, Oliver Jowett wrote:
>
>
> Well, I actually think this little change is more of the problem than
> anything else.

I don't understand what you mean -- are you saying we shouldn't change this?

> Did you manage to cobble together a patch for me to test ?

I'll send you a current snapshot in an hour or so. I got sidetracked
into trying to work out the semantics of getDate() and getTime() on a
timestamptz, still haven't found an entirely satisfactory solution..

-O

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Dave Cramer

2005-07-26, 7:23 am

On 26-Jul-05, at 6:23 AM, Oliver Jowett wrote:

> Dave Cramer wrote:
>
>
> I don't understand what you mean -- are you saying we shouldn't
> change this?
No, I think this may be the bug that causes it to fail currently. Not
that it shouldn't be overhauled. But this one is
probably the most significant "bug"
>
>
>
> I'll send you a current snapshot in an hour or so. I got
> sidetracked into trying to work out the semantics of getDate() and
> getTime() on a timestamptz, still haven't found an entirely
> satisfactory solution..

OK.
> -O
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.postgresql.org
>
>

Christian Cryder

2005-07-26, 8:24 pm

On 7/25/05, Dave Cramer <pg@fastcrypt.com> wrote:
> One of the times he is trying to stick in the db is a non-existant
> time if it is associated with a time zone
>
> stmt.execute("INSERT INTO Foo (t1) VALUES ('2005-04-03 02:29:43.0')");
>
> in any US timezone this time does not exist. In US/Mountain timezone
> this inserts as 2005-04-03 03:29:43.0' note the hour has incremented from
> 2 to 3

Just a clarification guys.
a) this time is only invalid in a US timezone that uses Daylight Savings
b) if you insert it via Statement like this, it inserts into the db
just fine; if you insert it using PreparedStatement the value will get
munged
c) once you have the value in the DB, the question is: "How can I read
the value and then rewrite it without munging it"

The only answer to c right now is
a) reconfigure both your client and your server to run in UTC (or some
other non-DST zone)
b) reconfigure your client to turn DST off, and then adjust the server
zone programatically just for the scope of the connection (that was
what my little patch was about)

Neither of these solutions is particularly appealing. I'm surprised
the basic data-integrity aspects of this issue don't bother people
more...

Christian

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql
.org so that your
message can get through to the mailing list cleanly

Sponsored Links