Thread: work in progress: timestamp patch
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/AbstractJdbc2ResultSet.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/AbstractJdbc2ResultSet.java,v
retrieving revision 1.75
diff -c -r1.75 AbstractJdbc2ResultSet.java
*** org/postgresql/jdbc2/AbstractJdbc2ResultSet.java 8 Jun 2005 01:44:02 -0000 1.75
--- org/postgresql/jdbc2/AbstractJdbc2ResultSet.java 26 Jul 2005 00:00:23 -0000
***************
*** 139,145 ****
case Types.TIME:
return getTime(columnIndex);
case Types.TIMESTAMP:
! return getTimestamp(columnIndex);
case Types.BINARY:
case Types.VARBINARY:
case Types.LONGVARBINARY:
--- 139,145 ----
case Types.TIME:
return getTime(columnIndex);
case Types.TIMESTAMP:
! return getTimestamp(columnIndex, 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.AbstractJdbc2Statement.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(columnIndex);
! 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(columnIndex, null);
}
public InputStream getAsciiStream(int columnIndex) throws SQLException
***************
*** 2261,2267 ****
public Timestamp getTimestamp(String columnName) throws SQLException
{
! return getTimestamp(findColumn(columnName));
}
public InputStream getAsciiStream(String columnName) throws SQLException
--- 2261,2267 ----
public Timestamp getTimestamp(String columnName) throws SQLException
{
! return getTimestamp(findColumn(columnName), null);
}
public InputStream getAsciiStream(String columnName) throws SQLException
Index: org/postgresql/jdbc2/AbstractJdbc2Statement.java
===================================================================
RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/AbstractJdbc2Statement.java,v
retrieving revision 1.79
diff -c -r1.79 AbstractJdbc2Statement.java
*** org/postgresql/jdbc2/AbstractJdbc2Statement.java 8 Jul 2005 17:38:29 -0000 1.79
--- org/postgresql/jdbc2/AbstractJdbc2Statement.java 26 Jul 2005 00:00:31 -0000
***************
*** 1301,1317 ****
*/
public void setTimestamp(int parameterIndex, Timestamp x) throws SQLException
{
! checkClosed();
! if (null == x)
! {
! setNull(parameterIndex, Types.TIMESTAMP);
! }
! else
! {
! if (calendar == null)
! calendar = new GregorianCalendar();
! bindString(parameterIndex, TimestampUtils.toString(sbuf, calendar, x), Oid.TIMESTAMPTZ);
! }
}
private void setCharacterStreamPost71(int parameterIndex, InputStream x, int length, String encoding) throws
SQLException
--- 1301,1307 ----
*/
public void setTimestamp(int parameterIndex, Timestamp x) throws SQLException
{
! setTimestamp(parameterIndex, x, null);
}
private void setCharacterStreamPost71(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(GregorianCalendar 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(Calendar 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(GregorianCalendar 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(StringBuffer sbuf, GregorianCalendar cal, Timestamp x) {
synchronized(sbuf) {
synchronized(cal) {
cal.setTime(x);
--- 253,259 ----
}
}
! public static String toString(StringBuffer sbuf, Calendar cal, Timestamp x) {
synchronized(sbuf) {
synchronized(cal) {
cal.setTime(x);
***************
*** 276,282 ****
}
}
! public static String toString(StringBuffer sbuf, GregorianCalendar cal, Date x) {
synchronized(sbuf) {
synchronized(cal) {
cal.setTime(x);
--- 275,281 ----
}
}
! public static String toString(StringBuffer sbuf, Calendar cal, Date x) {
synchronized(sbuf) {
synchronized(cal) {
cal.setTime(x);
***************
*** 295,301 ****
}
}
! public static String toString(StringBuffer sbuf, GregorianCalendar cal, Time x) {
synchronized(sbuf) {
synchronized(cal) {
cal.setTime(x);
--- 294,300 ----
}
}
! public static String toString(StringBuffer sbuf, Calendar cal, Time x) {
synchronized(sbuf) {
synchronized(cal) {
cal.setTime(x);
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='newtimezone'") 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/AbstractJdbc2ResultSet.java
> ===================================================================
> RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/
> AbstractJdbc2ResultSet.java,v
> retrieving revision 1.75
> diff -c -r1.75 AbstractJdbc2ResultSet.java
> *** org/postgresql/jdbc2/AbstractJdbc2ResultSet.java 8 Jun 2005
> 01:44:02 -0000 1.75
> --- org/postgresql/jdbc2/AbstractJdbc2ResultSet.java 26 Jul 2005
> 00:00:23 -0000
> ***************
> *** 139,145 ****
> case Types.TIME:
> return getTime(columnIndex);
> case Types.TIMESTAMP:
> ! return getTimestamp(columnIndex);
> case Types.BINARY:
> case Types.VARBINARY:
> case Types.LONGVARBINARY:
> --- 139,145 ----
> case Types.TIME:
> return getTime(columnIndex);
> case Types.TIMESTAMP:
> ! return getTimestamp(columnIndex, 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.AbstractJdbc2Statement.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(columnIndex);
> ! 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(columnIndex, null);
> }
>
> public InputStream getAsciiStream(int columnIndex) throws
> SQLException
> ***************
> *** 2261,2267 ****
>
> public Timestamp getTimestamp(String columnName) throws
> SQLException
> {
> ! return getTimestamp(findColumn(columnName));
> }
>
> public InputStream getAsciiStream(String columnName) throws
> SQLException
> --- 2261,2267 ----
>
> public Timestamp getTimestamp(String columnName) throws
> SQLException
> {
> ! return getTimestamp(findColumn(columnName), null);
> }
>
> public InputStream getAsciiStream(String columnName) throws
> SQLException
> Index: org/postgresql/jdbc2/AbstractJdbc2Statement.java
> ===================================================================
> RCS file: /usr/local/cvsroot/pgjdbc/pgjdbc/org/postgresql/jdbc2/
> AbstractJdbc2Statement.java,v
> retrieving revision 1.79
> diff -c -r1.79 AbstractJdbc2Statement.java
> *** org/postgresql/jdbc2/AbstractJdbc2Statement.java 8 Jul 2005
> 17:38:29 -0000 1.79
> --- org/postgresql/jdbc2/AbstractJdbc2Statement.java 26 Jul 2005
> 00:00:31 -0000
> ***************
> *** 1301,1317 ****
> */
> public void setTimestamp(int parameterIndex, Timestamp x)
> throws SQLException
> {
> ! checkClosed();
> ! if (null == x)
> ! {
> ! setNull(parameterIndex, Types.TIMESTAMP);
> ! }
> ! else
> ! {
> ! if (calendar == null)
> ! calendar = new GregorianCalendar();
> ! bindString(parameterIndex, TimestampUtils.toString
> (sbuf, calendar, x), Oid.TIMESTAMPTZ);
> ! }
> }
>
> private void setCharacterStreamPost71(int parameterIndex,
> InputStream x, int length, String encoding) throws SQLException
> --- 1301,1307 ----
> */
> public void setTimestamp(int parameterIndex, Timestamp x)
> throws SQLException
> {
> ! setTimestamp(parameterIndex, x, null);
> }
>
> private void setCharacterStreamPost71(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(GregorianCalendar 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(Calendar 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(GregorianCalendar 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(StringBuffer sbuf,
> GregorianCalendar cal, Timestamp x) {
> synchronized(sbuf) {
> synchronized(cal) {
> cal.setTime(x);
> --- 253,259 ----
> }
> }
>
> ! public static String toString(StringBuffer sbuf, Calendar
> cal, Timestamp x) {
> synchronized(sbuf) {
> synchronized(cal) {
> cal.setTime(x);
> ***************
> *** 276,282 ****
> }
> }
>
> ! public static String toString(StringBuffer sbuf,
> GregorianCalendar cal, Date x) {
> synchronized(sbuf) {
> synchronized(cal) {
> cal.setTime(x);
> --- 275,281 ----
> }
> }
>
> ! public static String toString(StringBuffer sbuf, Calendar
> cal, Date x) {
> synchronized(sbuf) {
> synchronized(cal) {
> cal.setTime(x);
> ***************
> *** 295,301 ****
> }
> }
>
> ! public static String toString(StringBuffer sbuf,
> GregorianCalendar cal, Time x) {
> synchronized(sbuf) {
> synchronized(cal) {
> cal.setTime(x);
> --- 294,300 ----
> }
> }
>
> ! public static String toString(StringBuffer sbuf, Calendar
> cal, Time x) {
> synchronized(sbuf) {
> synchronized(cal) {
> cal.setTime(x);
>
> ---------------------------(end of
> broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
>
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='newtimezone'") 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
Oliver,
Dave
On 25-Jul-05, at 11:19 PM, Oliver Jowett wrote:
> 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='newtimezone'") 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?
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
>
>
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
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
On 26-Jul-05, at 1:23 AM, Oliver Jowett wrote: > 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.. 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 > >
Dave Cramer wrote: > > On 26-Jul-05, at 1:23 AM, Oliver Jowett 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. 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
On 26-Jul-05, at 6:23 AM, Oliver Jowett wrote:
No, I think this may be the bug that causes it to fail currently. Not that it shouldn't be overhauled. But this one isDave Cramer wrote:On 26-Jul-05, at 1:23 AM, Oliver Jowett wrote:I looked at this and the current code is certainly wrong. The timezoneoffset of a Timestamp (deprecated method!) returns the offset of theJVM's default timezone always. We should indeed be passing the targetcalendar and using that.I've added that change to my patch. Interestingly none of the regressiontests fail with it changed; we're very short on tests that actually testthe with-Calendar code..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?
probably the most significant "bug"
OK.
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?
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