Обсуждение: Patch for jdbc ResultSet.getTimestamp()
Included is a patch that fixes a bug introduced in the lastest version
(1.22) of interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java. That
change removed a line that set the variable s to the value of the
stringbuffer. This fix changes the following if checks to check the
length of the stringbuffer instead of s, since s no longer contains the
string the if conditions are expecting.
The bug manifests itself in getTimestamp() loosing the timezone
information of timestamps selected from the database, thereby causing
the time to be incorrect.
If possible this patch should be patched into 7.1 for the upcoming 7.1.2
patch.
thanks,
--Barry
*** ./interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java.orig Tue May 15 21:59:46 2001
--- ./interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java Tue May 15 22:06:43 2001
***************
*** 499,511 ****
// could optimize this a tad to remove too many object creations...
SimpleDateFormat df = null;
! if (s.length()>23 && subsecond) {
df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSSzzzzzzzzz");
! } else if (s.length()>23 && !subsecond) {
df = new SimpleDateFormat("yyyy-MM-dd HH:mm:sszzzzzzzzz");
! } else if (s.length()>10 && subsecond) {
df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
! } else if (s.length()>10 && !subsecond) {
df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
} else {
df = new SimpleDateFormat("yyyy-MM-dd");
--- 499,511 ----
// could optimize this a tad to remove too many object creations...
SimpleDateFormat df = null;
! if (sbuf.length()>23 && subsecond) {
df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSSzzzzzzzzz");
! } else if (sbuf.length()>23 && !subsecond) {
df = new SimpleDateFormat("yyyy-MM-dd HH:mm:sszzzzzzzzz");
! } else if (sbuf.length()>10 && subsecond) {
df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
! } else if (sbuf.length()>10 && !subsecond) {
df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
} else {
df = new SimpleDateFormat("yyyy-MM-dd");
Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it withing the next 48 hours.
I can not backpatch this to 7.1.X because we are only adding major bug
fixes into that release.
>
> Included is a patch that fixes a bug introduced in the lastest version
> (1.22) of interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java. That
> change removed a line that set the variable s to the value of the
> stringbuffer. This fix changes the following if checks to check the
> length of the stringbuffer instead of s, since s no longer contains the
> string the if conditions are expecting.
>
> The bug manifests itself in getTimestamp() loosing the timezone
> information of timestamps selected from the database, thereby causing
> the time to be incorrect.
>
> If possible this patch should be patched into 7.1 for the upcoming 7.1.2
> patch.
>
> thanks,
> --Barry
> *** ./interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java.orig Tue May 15 21:59:46 2001
> --- ./interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java Tue May 15 22:06:43 2001
> ***************
> *** 499,511 ****
> // could optimize this a tad to remove too many object creations...
> SimpleDateFormat df = null;
>
> ! if (s.length()>23 && subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSSzzzzzzzzz");
> ! } else if (s.length()>23 && !subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:sszzzzzzzzz");
> ! } else if (s.length()>10 && subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
> ! } else if (s.length()>10 && !subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
> } else {
> df = new SimpleDateFormat("yyyy-MM-dd");
> --- 499,511 ----
> // could optimize this a tad to remove too many object creations...
> SimpleDateFormat df = null;
>
> ! if (sbuf.length()>23 && subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSSzzzzzzzzz");
> ! } else if (sbuf.length()>23 && !subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:sszzzzzzzzz");
> ! } else if (sbuf.length()>10 && subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
> ! } else if (sbuf.length()>10 && !subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
> } else {
> df = new SimpleDateFormat("yyyy-MM-dd");
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Patch applied, and backpatched to jdbc1. Can't apply to 7.1.X. That
jdbc code is pretty much frozen. We have had too much breakage of jdbc
code to apply this to 7.1.X.
However, we are working on patching the CVS and should have jar files
available for people to use soon.
>
> Included is a patch that fixes a bug introduced in the lastest version
> (1.22) of interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java. That
> change removed a line that set the variable s to the value of the
> stringbuffer. This fix changes the following if checks to check the
> length of the stringbuffer instead of s, since s no longer contains the
> string the if conditions are expecting.
>
> The bug manifests itself in getTimestamp() loosing the timezone
> information of timestamps selected from the database, thereby causing
> the time to be incorrect.
>
> If possible this patch should be patched into 7.1 for the upcoming 7.1.2
> patch.
>
> thanks,
> --Barry
> *** ./interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java.orig Tue May 15 21:59:46 2001
> --- ./interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java Tue May 15 22:06:43 2001
> ***************
> *** 499,511 ****
> // could optimize this a tad to remove too many object creations...
> SimpleDateFormat df = null;
>
> ! if (s.length()>23 && subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSSzzzzzzzzz");
> ! } else if (s.length()>23 && !subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:sszzzzzzzzz");
> ! } else if (s.length()>10 && subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
> ! } else if (s.length()>10 && !subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
> } else {
> df = new SimpleDateFormat("yyyy-MM-dd");
> --- 499,511 ----
> // could optimize this a tad to remove too many object creations...
> SimpleDateFormat df = null;
>
> ! if (sbuf.length()>23 && subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSSzzzzzzzzz");
> ! } else if (sbuf.length()>23 && !subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:sszzzzzzzzz");
> ! } else if (sbuf.length()>10 && subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
> ! } else if (sbuf.length()>10 && !subsecond) {
> df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
> } else {
> df = new SimpleDateFormat("yyyy-MM-dd");
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026
> I can not backpatch this to 7.1.X because we are only adding major bug
> fixes into that release.
Why is that? afaik any bug fix is a candidate for the stable tree, as
long as someone is willing to take responsibility for its level of risk
and appropriateness. Something which fixes a clearly incorrect behavior
would seem appropriate, especially if it is as concise and specific as
is the case for this patch.
Would you like someone else to vet the patch before we apply it? Or
better yet have someone else who is compiling and using JDBC do so with
this applied?
Volunteers?
- Thomas
> > I can not backpatch this to 7.1.X because we are only adding major bug > > fixes into that release. > > Why is that? afaik any bug fix is a candidate for the stable tree, as > long as someone is willing to take responsibility for its level of risk > and appropriateness. Something which fixes a clearly incorrect behavior > would seem appropriate, especially if it is as concise and specific as > is the case for this patch. > > Would you like someone else to vet the patch before we apply it? Or > better yet have someone else who is compiling and using JDBC do so with > this applied? I have JDBC2 here, and can test the compile. I am focusing on the CVS because I have major jdbc patches back to February already applied. There are so many fixes in CVS now, I am not sure how valuable 7.1.2 jdbc will be even with this one patch. I certainly will not be applying any jdbc stuff to 7.1.X. I was told not to apply stuff during beta. I am sure not going to apply anything to a minor release. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
> I have JDBC2 here, and can test the compile. I am focusing on the CVS > because I have major jdbc patches back to February already applied. > There are so many fixes in CVS now, I am not sure how valuable 7.1.2 > jdbc will be even with this one patch. > > I certainly will not be applying any jdbc stuff to 7.1.X. I was told > not to apply stuff during beta. I am sure not going to apply anything > to a minor release. One more issue is that I am thinking JDBC will be similar to ODBC in that it will start releasing more frequently, independent of our major releases, Both interfaces need more frequent feedback to users. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
More frequent jdbc releases are probably a good idea, but I still would expect that they are being made from patches applied to the 7.1 branch, not being pulled from the 7.2 main trunk. What happens when changes to the backend for 7.2 start requiring changes to jdbc that may not be backwardly compatible with 7.1? Isn't the whole point of having a 7.1 branch meant to have a place for patches/bug fixes that are isolated from new feature development? thanks, --Barry Bruce Momjian wrote: >> I have JDBC2 here, and can test the compile. I am focusing on the CVS >> because I have major jdbc patches back to February already applied. >> There are so many fixes in CVS now, I am not sure how valuable 7.1.2 >> jdbc will be even with this one patch. >> >> I certainly will not be applying any jdbc stuff to 7.1.X. I was told >> not to apply stuff during beta. I am sure not going to apply anything >> to a minor release. > > > One more issue is that I am thinking JDBC will be similar to ODBC in > that it will start releasing more frequently, independent of our major > releases, Both interfaces need more frequent feedback to users. >
> I certainly will not be applying any jdbc stuff to 7.1.X. I was told
> not to apply stuff during beta. I am sure not going to apply anything
> to a minor release.
OK, but if someone takes responsibility for the accuracy and
effectiveness of a patch, then it should be considered for application.
We had trouble during beta applying larger patches or patches which were
not correct; some of which could be vetted by simple inspection. This
one would actually be in the "simple inspection" category, but if
someone (a third person) wants to test it first that would be better and
that step might be considered essential.
Without any patches, we would not have any minor releases so I'm not
sure that this is violating any standing rules for branch management ;)
Re: the code itself...
I see that this section of code assumes that time zones are on an even
hour boundary (that is, that there are only two digits in the ISO time
zone). That is not true for Canada/Newfoundland and Asia/Calcutta time
zones (perhaps others too), which are on half-hour boundaries. Since the
code is looking for a sign character exactly three positions away from
the end of the timestamp string, it is not likely to catch those cases
istm.
Anyone want to take a stab at handling both 3 ("-08") and 6 ("-02:30")
character time zone fields too? Or am I misreading the code??
- Thomas
Thomas Lockhart <lockhart@alumni.caltech.edu> writes:
>> I certainly will not be applying any jdbc stuff to 7.1.X. I was told
>> not to apply stuff during beta. I am sure not going to apply anything
>> to a minor release.
> OK, but if someone takes responsibility for the accuracy and
> effectiveness of a patch, then it should be considered for application.
> We had trouble during beta applying larger patches or patches which were
> not correct; some of which could be vetted by simple inspection.
Bruce got pounded (quite properly I thought) for applying patches
during late beta that he had not verified and indeed was not in a
position to verify. The rules need to be at least as stringent for
patches that go into a stable-release branch, since we don't normally
do much beta testing for dot-releases. But as long as someone has
adequately checked the patch, I don't think there should be a rule
against putting bug fixes into 7.1.* ...
regards, tom lane
> Bruce got pounded (quite properly I thought) for applying patches > during late beta that he had not verified and indeed was not in a > position to verify. The rules need to be at least as stringent for > patches that go into a stable-release branch, since we don't normally > do much beta testing for dot-releases. But as long as someone has > adequately checked the patch, I don't think there should be a rule > against putting bug fixes into 7.1.* ... Toward the end, I actually got approval from other JDBC developers before applying patches and people were still not happy. Also, this was during feature freeze in February, not even in beta yet. I know it was February because I just applied all my outstanding JDBC patches since February 1, and many of those should probably have been in 7.1.X too. So, I am not going to apply it. If someone else wants to, go ahead. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Toward the end, I actually got approval from other JDBC developers
> before applying patches and people were still not happy. Also, this was
> during feature freeze in February, not even in beta yet.
What? beta1 came out the first week of December. February would have
been beta4 or later.
regards, tom lane
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Toward the end, I actually got approval from other JDBC developers > > before applying patches and people were still not happy. Also, this was > > during feature freeze in February, not even in beta yet. > > What? beta1 came out the first week of December. February would have > been beta4 or later. Wow, we were in beta that long. Sorry. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Since I consider this patch very important (since selecting any
timestamp column from the database currently returns the wrong time),
what do I need to do to get a patch like this applied to the 7.1 branch?
thanks,
--Barry
Thomas Lockhart wrote:
>> I certainly will not be applying any jdbc stuff to 7.1.X. I was told
>> not to apply stuff during beta. I am sure not going to apply anything
>> to a minor release.
>
>
> OK, but if someone takes responsibility for the accuracy and
> effectiveness of a patch, then it should be considered for application.
> We had trouble during beta applying larger patches or patches which were
> not correct; some of which could be vetted by simple inspection. This
> one would actually be in the "simple inspection" category, but if
> someone (a third person) wants to test it first that would be better and
> that step might be considered essential.
>
> Without any patches, we would not have any minor releases so I'm not
> sure that this is violating any standing rules for branch management ;)
>
> Re: the code itself...
>
> I see that this section of code assumes that time zones are on an even
> hour boundary (that is, that there are only two digits in the ISO time
> zone). That is not true for Canada/Newfoundland and Asia/Calcutta time
> zones (perhaps others too), which are on half-hour boundaries. Since the
> code is looking for a sign character exactly three positions away from
> the end of the timestamp string, it is not likely to catch those cases
> istm.
>
> Anyone want to take a stab at handling both 3 ("-08") and 6 ("-02:30")
> character time zone fields too? Or am I misreading the code??
>
> - Thomas
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>
>