Re: Fix setObject() with Time/Timestamp/Date + Timezones

From: Barry Lind <blind(at)xythos(dot)com>
To: Kim Ho <kho(at)redhat(dot)com>
Cc: Dave Cramer <Dave(at)micro-automation(dot)net>, "'pgsql-jdbc(at)postgresql(dot)org'" <pgsql-jdbc(at)postgresql(dot)org>
Subject: Re: Fix setObject() with Time/Timestamp/Date + Timezones
Date: 2003-07-09 05:22:32
Message-ID: 3F0BA698.1000101@xythos.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-jdbc

Patch applied.

thanks,
--Barry

Kim Ho wrote:
> On Mon, 2003-07-07 at 21:01, Barry Lind wrote:
>
>>Kim,
>>
>>I have three issues with this patch. Although overall I think it is
>>very good work.
>>
>>1) The methods DateFromString(), TimeFromString() and
>>TimestampFromString() should be named dateFromString(), timeFromString()
>>and timestampFromString() to follow standard java method naming
>>conventions. If you really want the Date, Time and Timestamp part to
>>remain init caps, might I suggest the following names: toDate(),
>>toTime() and toTimestamp().
>>
>
> Done.
>
>
>>2) In the TIME and TIMESTAMP cases in the switch, if the object is
>>already of type Time or Timestamp you go ahead and toString() it and
>>then reconvert it back to a Time or Timestamp object. You should just
>>be using the value as passed. While I don't know if this will do
>>anything incorrectly, it certainly is a lot more work than necessary.
>>
>
> Done.
>
>
>>3) I would really like to see some additional test cases added to the
>>test suite for this logic. Experience has taught me that when it comes
>>to dates and the jdbc driver a good set of tests and expected results is
>>very important. What I have seen before is patches submitted that
>>change the behavior because someone thinks that the driver is handling
>>timezones offsets incorrectly where as it was really their code that was
>>doing things wrong. Without a set of tests and expected results it is
>>difficult to know whether such patches are correct or not.
>>
>
> I'll look into this.
>
>
>>thanks,
>>--Barry
>>
>
>
> Cheers,
>
> Kim
>
>
> ------------------------------------------------------------------------
>
> Index: org/postgresql/errors.properties
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/errors.properties,v
> retrieving revision 1.21
> diff -c -p -r1.21 errors.properties
> *** org/postgresql/errors.properties 30 Jun 2003 16:38:30 -0000 1.21
> --- org/postgresql/errors.properties 8 Jul 2003 14:07:24 -0000
> *************** postgresql.call.wrongrtntype:A CallableS
> *** 100,102 ****
> --- 100,105 ----
> postgresql.input.fetch.gt0:Fetch size must be a value greater than or equal to 0.
> postgresql.input.query.gt0:Query Timeout must be a value greater than or equal to 0.
> postgresql.input.rows.gt0:Maximum number of rows must be a value greater than or equal to 0.
> + postgresql.format.baddate:The date given: {0} does not match the format required: {1}.
> + postgresql.format.badtime:The time given: {0} does not match the format required: {1}.
> + postgresql.format.badtimestamp:The timestamp given {0} does not match the format required: {1}.
> Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
> retrieving revision 1.26
> diff -c -p -r1.26 AbstractJdbc1Statement.java
> *** org/postgresql/jdbc1/AbstractJdbc1Statement.java 30 Jun 2003 21:10:55 -0000 1.26
> --- org/postgresql/jdbc1/AbstractJdbc1Statement.java 8 Jul 2003 14:07:26 -0000
> *************** public abstract class AbstractJdbc1State
> *** 1488,1500 ****
> setString(parameterIndex, x.toString());
> break;
> case Types.DATE:
> ! setDate(parameterIndex, (java.sql.Date)x);
> break;
> case Types.TIME:
> ! setTime(parameterIndex, (Time)x);
> break;
> case Types.TIMESTAMP:
> ! setTimestamp(parameterIndex, (Timestamp)x);
> break;
> case Types.BIT:
> if (x instanceof Boolean)
> --- 1488,1518 ----
> setString(parameterIndex, x.toString());
> break;
> case Types.DATE:
> ! if (x instanceof java.sql.Date)
> ! setDate(parameterIndex, (java.sql.Date)x);
> ! else
> ! {
> ! java.sql.Date tmpd = (x instanceof java.util.Date) ? new java.sql.Date(((java.util.Date)x).getTime()) : dateFromString(x.toString());
> ! setDate(parameterIndex, tmpd);
> ! }
> break;
> case Types.TIME:
> ! if (x instanceof java.sql.Time)
> ! setTime(parameterIndex, (java.sql.Time)x);
> ! else
> ! {
> ! java.sql.Time tmpt = (x instanceof java.util.Date) ? new java.sql.Time(((java.util.Date)x).getTime()) : timeFromString(x.toString());
> ! setTime(parameterIndex, tmpt);
> ! }
> break;
> case Types.TIMESTAMP:
> ! if (x instanceof java.sql.Timestamp)
> ! setTimestamp(parameterIndex ,(java.sql.Timestamp)x);
> ! else
> ! {
> ! java.sql.Timestamp tmpts = (x instanceof java.util.Date) ? new java.sql.Timestamp(((java.util.Date)x).getTime()) : timestampFromString(x.toString());
> ! setTimestamp(parameterIndex, tmpts);
> ! }
> break;
> case Types.BIT:
> if (x instanceof Boolean)
> *************** public abstract class AbstractJdbc1State
> *** 2032,2038 ****
> return m_useServerPrepare;
> }
>
> !
> private static final String PG_TEXT = "text";
> private static final String PG_INTEGER = "integer";
> private static final String PG_INT2 = "int2";
> --- 2050,2162 ----
> return m_useServerPrepare;
> }
>
> ! private java.sql.Date dateFromString (String s) throws SQLException
> ! {
> ! int timezone = 0;
> ! long millis = 0;
> ! long localoffset = 0;
> ! int timezoneLocation = (s.indexOf('+') == -1) ? s.lastIndexOf("-") : s.indexOf('+');
> ! //if the last index of '-' or '+' is past 8. we are guaranteed that it is a timezone marker
> ! //shortest = yyyy-m-d
> ! //longest = yyyy-mm-dd
> ! try
> ! {
> ! timezone = (timezoneLocation>7) ? timezoneLocation : s.length();
> ! millis = java.sql.Date.valueOf(s.substring(0,timezone)).getTime();
> ! }
> ! catch (Exception e)
> ! {
> ! throw new PSQLException("postgresql.format.baddate",s , "yyyy-MM-dd[-tz]");
> ! }
> ! timezone = 0;
> ! if (timezoneLocation>7 && timezoneLocation+3 == s.length())
> ! {
> ! timezone = Integer.parseInt(s.substring(timezoneLocation+1,s.length()));
> ! localoffset = java.util.Calendar.getInstance().getTimeZone().getOffset(millis);
> ! if (s.charAt(timezoneLocation)=='+')
> ! timezone*=-1;
> ! }
> ! millis = millis + timezone*60*60*1000 + localoffset;
> ! return new java.sql.Date(millis);
> ! }
> !
> ! private java.sql.Time timeFromString (String s) throws SQLException
> ! {
> ! int timezone = 0;
> ! long millis = 0;
> ! long localoffset = 0;
> ! int timezoneLocation = (s.indexOf('+') == -1) ? s.lastIndexOf("-") : s.indexOf('+');
> ! //if the index of the last '-' or '+' is greater than 0 that means this time has a timezone.
> ! //everything earlier than that position, we treat as the time and parse it as such.
> ! try
> ! {
> ! timezone = (timezoneLocation==-1) ? s.length() : timezoneLocation;
> ! millis = java.sql.Time.valueOf(s.substring(0,timezone)).getTime();
> ! }
> ! catch (Exception e)
> ! {
> ! throw new PSQLException("postgresql.format.badtime",s, "HH:mm:ss[-tz]");
> ! }
> ! timezone = 0;
> ! if (timezoneLocation != -1 && timezoneLocation+3 == s.length())
> ! {
> ! timezone = Integer.parseInt(s.substring(timezoneLocation+1,s.length()));
> ! localoffset = java.util.Calendar.getInstance().getTimeZone().getOffset(millis);
> ! if (s.charAt(timezoneLocation)=='+')
> ! timezone*=-1;
> ! }
> ! millis = millis + timezone*60*60*1000 + localoffset;
> ! return new java.sql.Time(millis);
> ! }
> !
> ! private java.sql.Timestamp timestampFromString (String s) throws SQLException
> ! {
> ! int timezone = 0;
> ! long millis = 0;
> ! long localoffset = 0;
> ! int nanosVal = 0;
> ! int timezoneLocation = (s.indexOf('+') == -1) ? s.lastIndexOf("-") : s.indexOf('+');
> ! int nanospos = s.indexOf(".");
> ! //if there is a '.', that means there are nanos info, and we take the timestamp up to that point
> ! //if not, then we check to see if the last +/- (to indicate a timezone) is greater than 8
> ! //8 is because the shortest date, will have last '-' at position 7. e.g yyyy-x-x
> ! try
> ! {
> ! if (nanospos != -1)
> ! timezone = nanospos;
> ! else if (timezoneLocation > 8)
> ! timezone = timezoneLocation;
> ! else
> ! timezone = s.length();
> ! millis = java.sql.Timestamp.valueOf(s.substring(0,timezone)).getTime();
> ! }
> ! catch (Exception e)
> ! {
> ! throw new PSQLException("postgresql.format.badtimestamp", s, "yyyy-MM-dd HH:mm:ss[.xxxxxx][-tz]");
> ! }
> ! timezone = 0;
> ! if (nanospos != -1)
> ! {
> ! int tmploc = (timezoneLocation > 8) ? timezoneLocation : s.length();
> ! nanosVal = Integer.parseInt(s.substring(nanospos+1,tmploc));
> ! int diff = 8-((tmploc-1)-(nanospos+1));
> ! for (int i=0;i<diff;i++)
> ! nanosVal*=10;
> ! }
> ! if (timezoneLocation>8 && timezoneLocation+3 == s.length())
> ! {
> ! timezone = Integer.parseInt(s.substring(timezoneLocation+1,s.length()));
> ! localoffset = java.util.Calendar.getInstance().getTimeZone().getOffset(millis);
> ! if (s.charAt(timezoneLocation)=='+')
> ! timezone*=-1;
> ! }
> ! millis = millis + timezone*60*60*1000 + localoffset;
> ! java.sql.Timestamp tmpts = new java.sql.Timestamp(millis);
> ! tmpts.setNanos(nanosVal);
> ! return tmpts;
> ! }
> !
> !
> private static final String PG_TEXT = "text";
> private static final String PG_INTEGER = "integer";
> private static final String PG_INT2 = "int2";

In response to

Browse pgsql-jdbc by date

  From Date Subject
Next Message Arun Desai 2003-07-09 10:27:09 Postgresql JDBC question
Previous Message Paul Thomas 2003-07-08 20:06:22 Re: Strange exception opening JDBC connection