Thread: Patch for jdbc2 ResultSet.java
Just some minor fixes to avoid duplicate function calls that were bugging me.
If rows is garunteed not to change then we can store the size in a final class variable,
but I wasn't sure I could assume that.
*** ResultSet.java.orig Tue Sep 4 16:34:14 2001
--- ResultSet.java Tue Sep 4 16:45:20 2001
***************
*** 134,140 ****
{
//release resources held (memory for tuples)
if(rows!=null) {
- rows.setSize(0);
rows=null;
}
}
--- 134,139 ----
***************
*** 709,716 ****
public int findColumn(String columnName) throws SQLException
{
int i;
!
! for (i = 0 ; i < fields.length; ++i)
if (fields[i].getName().equalsIgnoreCase(columnName))
return (i+1);
throw new PSQLException ("postgresql.res.colname",columnName);
--- 708,715 ----
public int findColumn(String columnName) throws SQLException
{
int i;
! final int flen = fields.length;
! for (i = 0 ; i < flen; ++i)
if (fields[i].getName().equalsIgnoreCase(columnName))
return (i+1);
throw new PSQLException ("postgresql.res.colname",columnName);
***************
*** 726,736 ****
if (index==0)
throw new SQLException("Cannot move to index of 0");
//if index<0, count from the end of the result set, but check
//to be sure that it is not beyond the first index
if (index<0)
! if (index>=-rows.size())
! internalIndex=rows.size()+index;
else {
beforeFirst();
return false;
--- 725,737 ----
if (index==0)
throw new SQLException("Cannot move to index of 0");
+ final int rows_size = rows.size();
+
//if index<0, count from the end of the result set, but check
//to be sure that it is not beyond the first index
if (index<0)
! if (index > rows_size)
! internalIndex = rows_size+index;
else {
beforeFirst();
return false;
***************
*** 739,745 ****
//must be the case that index>0,
//find the correct place, assuming that
//the index is not too large
! if (index<=rows.size())
internalIndex = index-1;
else {
afterLast();
--- 740,746 ----
//must be the case that index>0,
//find the correct place, assuming that
//the index is not too large
! if (index <= rows_size)
internalIndex = index-1;
else {
afterLast();
***************
*** 753,760 ****
public void afterLast() throws SQLException
{
! if (rows.size() > 0)
! current_row = rows.size();
}
public void beforeFirst() throws SQLException
--- 754,762 ----
public void afterLast() throws SQLException
{
! final int rows_size = rows.size();
! if (rows_size > 0)
! current_row = rows_size;
}
public void beforeFirst() throws SQLException
***************
*** 967,973 ****
public boolean isAfterLast() throws SQLException
{
! return (current_row >= rows.size() && rows.size() > 0);
}
public boolean isBeforeFirst() throws SQLException
--- 969,976 ----
public boolean isAfterLast() throws SQLException
{
! final int rows_size = rows.size();
! return (current_row >= rows_size && rows_size > 0);
}
public boolean isBeforeFirst() throws SQLException
***************
*** 982,995 ****
public boolean isLast() throws SQLException
{
! return (current_row == rows.size() -1 && rows.size() > 0);
}
public boolean last() throws SQLException
{
! if (rows.size() <= 0)
return false;
! current_row = rows.size() - 1;
this_row = (byte [][])rows.elementAt(current_row);
return true;
}
--- 985,1000 ----
public boolean isLast() throws SQLException
{
! final int rows_size = rows.size();
! return (current_row == rows_size -1 && rows_size > 0);
}
public boolean last() throws SQLException
{
! final int rows_size = rows.size();
! if (rows_size <= 0)
return false;
! current_row = rows_size - 1;
this_row = (byte [][])rows.elementAt(current_row);
return true;
}
***************
*** 1480,1483 ****
}
}
}
-
--- 1485,1487 ----
--
Joseph Shraibman
jks@selectacast.net
Increase signal to noise ratio. http://www.targabot.com
Joseph,
In looking at this patch it looks OK, except for the following change:
> ! if (index>=-rows.size())
> --- 725,737 ----
> ! if (index > rows_size)
I haven't looked at the entire method, but the change you made seems
incorrect.
If you want this patch to be applied it should be sent to the
pgsql-patches mail list.
thanks,
--Barry
Joseph Shraibman wrote:
> Just some minor fixes to avoid duplicate function calls that were
> bugging me.
>
> If rows is garunteed not to change then we can store the size in a final
> class variable, but I wasn't sure I could assume that.
>
> *** ResultSet.java.orig Tue Sep 4 16:34:14 2001
> --- ResultSet.java Tue Sep 4 16:45:20 2001
> ***************
> *** 134,140 ****
> {
> //release resources held (memory for tuples)
> if(rows!=null) {
> - rows.setSize(0);
> rows=null;
> }
> }
> --- 134,139 ----
> ***************
> *** 709,716 ****
> public int findColumn(String columnName) throws SQLException
> {
> int i;
> !
> ! for (i = 0 ; i < fields.length; ++i)
> if (fields[i].getName().equalsIgnoreCase(columnName))
> return (i+1);
> throw new PSQLException ("postgresql.res.colname",columnName);
> --- 708,715 ----
> public int findColumn(String columnName) throws SQLException
> {
> int i;
> ! final int flen = fields.length;
> ! for (i = 0 ; i < flen; ++i)
> if (fields[i].getName().equalsIgnoreCase(columnName))
> return (i+1);
> throw new PSQLException ("postgresql.res.colname",columnName);
> ***************
> *** 726,736 ****
> if (index==0)
> throw new SQLException("Cannot move to index of 0");
>
> //if index<0, count from the end of the result set, but check
> //to be sure that it is not beyond the first index
> if (index<0)
> ! if (index>=-rows.size())
> ! internalIndex=rows.size()+index;
> else {
> beforeFirst();
> return false;
> --- 725,737 ----
> if (index==0)
> throw new SQLException("Cannot move to index of 0");
>
> + final int rows_size = rows.size();
> +
> //if index<0, count from the end of the result set, but check
> //to be sure that it is not beyond the first index
> if (index<0)
> ! if (index > rows_size)
> ! internalIndex = rows_size+index;
> else {
> beforeFirst();
> return false;
> ***************
> *** 739,745 ****
> //must be the case that index>0,
> //find the correct place, assuming that
> //the index is not too large
> ! if (index<=rows.size())
> internalIndex = index-1;
> else {
> afterLast();
> --- 740,746 ----
> //must be the case that index>0,
> //find the correct place, assuming that
> //the index is not too large
> ! if (index <= rows_size)
> internalIndex = index-1;
> else {
> afterLast();
> ***************
> *** 753,760 ****
>
> public void afterLast() throws SQLException
> {
> ! if (rows.size() > 0)
> ! current_row = rows.size();
> }
>
> public void beforeFirst() throws SQLException
> --- 754,762 ----
>
> public void afterLast() throws SQLException
> {
> ! final int rows_size = rows.size();
> ! if (rows_size > 0)
> ! current_row = rows_size;
> }
>
> public void beforeFirst() throws SQLException
> ***************
> *** 967,973 ****
>
> public boolean isAfterLast() throws SQLException
> {
> ! return (current_row >= rows.size() && rows.size() > 0);
> }
>
> public boolean isBeforeFirst() throws SQLException
> --- 969,976 ----
>
> public boolean isAfterLast() throws SQLException
> {
> ! final int rows_size = rows.size();
> ! return (current_row >= rows_size && rows_size > 0);
> }
>
> public boolean isBeforeFirst() throws SQLException
> ***************
> *** 982,995 ****
>
> public boolean isLast() throws SQLException
> {
> ! return (current_row == rows.size() -1 && rows.size() > 0);
> }
>
> public boolean last() throws SQLException
> {
> ! if (rows.size() <= 0)
> return false;
> ! current_row = rows.size() - 1;
> this_row = (byte [][])rows.elementAt(current_row);
> return true;
> }
> --- 985,1000 ----
>
> public boolean isLast() throws SQLException
> {
> ! final int rows_size = rows.size();
> ! return (current_row == rows_size -1 && rows_size > 0);
> }
>
> public boolean last() throws SQLException
> {
> ! final int rows_size = rows.size();
> ! if (rows_size <= 0)
> return false;
> ! current_row = rows_size - 1;
> this_row = (byte [][])rows.elementAt(current_row);
> return true;
> }
> ***************
> *** 1480,1483 ****
> }
> }
> }
> -
> --- 1485,1487 ----
>
>
Barry Lind wrote:
> Joseph,
>
> In looking at this patch it looks OK, except for the following change:
>
> > ! if (index>=-rows.size())
> > --- 725,737 ----
> > ! if (index > rows_size)
>
> I haven't looked at the entire method, but the change you made seems
> incorrect.
>
Oops! Thanks for catching that. Cut and paste error. I hate those.
> If you want this patch to be applied it should be sent to the
> pgsql-patches mail list.
I thought that jdbc stuff was preferred to be on the jdbc list. I guess not.
new patch:
*** ResultSet.java.orig Tue Sep 4 16:34:14 2001
--- ResultSet.java Wed Sep 5 15:35:59 2001
***************
*** 134,140 ****
{
//release resources held (memory for tuples)
if(rows!=null) {
- rows.setSize(0);
rows=null;
}
}
--- 134,139 ----
***************
*** 709,716 ****
public int findColumn(String columnName) throws SQLException
{
int i;
!
! for (i = 0 ; i < fields.length; ++i)
if (fields[i].getName().equalsIgnoreCase(columnName))
return (i+1);
throw new PSQLException ("postgresql.res.colname",columnName);
--- 708,715 ----
public int findColumn(String columnName) throws SQLException
{
int i;
! final int flen = fields.length;
! for (i = 0 ; i < flen; ++i)
if (fields[i].getName().equalsIgnoreCase(columnName))
return (i+1);
throw new PSQLException ("postgresql.res.colname",columnName);
***************
*** 726,736 ****
if (index==0)
throw new SQLException("Cannot move to index of 0");
//if index<0, count from the end of the result set, but check
//to be sure that it is not beyond the first index
if (index<0)
! if (index>=-rows.size())
! internalIndex=rows.size()+index;
else {
beforeFirst();
return false;
--- 725,737 ----
if (index==0)
throw new SQLException("Cannot move to index of 0");
+ final int rows_size = rows.size();
+
//if index<0, count from the end of the result set, but check
//to be sure that it is not beyond the first index
if (index<0)
! if (index > -rows_size)
! internalIndex = rows_size+index;
else {
beforeFirst();
return false;
***************
*** 739,745 ****
//must be the case that index>0,
//find the correct place, assuming that
//the index is not too large
! if (index<=rows.size())
internalIndex = index-1;
else {
afterLast();
--- 740,746 ----
//must be the case that index>0,
//find the correct place, assuming that
//the index is not too large
! if (index <= rows_size)
internalIndex = index-1;
else {
afterLast();
***************
*** 753,760 ****
public void afterLast() throws SQLException
{
! if (rows.size() > 0)
! current_row = rows.size();
}
public void beforeFirst() throws SQLException
--- 754,762 ----
public void afterLast() throws SQLException
{
! final int rows_size = rows.size();
! if (rows_size > 0)
! current_row = rows_size;
}
public void beforeFirst() throws SQLException
***************
*** 967,973 ****
public boolean isAfterLast() throws SQLException
{
! return (current_row >= rows.size() && rows.size() > 0);
}
public boolean isBeforeFirst() throws SQLException
--- 969,976 ----
public boolean isAfterLast() throws SQLException
{
! final int rows_size = rows.size();
! return (current_row >= rows_size && rows_size > 0);
}
public boolean isBeforeFirst() throws SQLException
***************
*** 982,995 ****
public boolean isLast() throws SQLException
{
! return (current_row == rows.size() -1 && rows.size() > 0);
}
public boolean last() throws SQLException
{
! if (rows.size() <= 0)
return false;
! current_row = rows.size() - 1;
this_row = (byte [][])rows.elementAt(current_row);
return true;
}
--- 985,1000 ----
public boolean isLast() throws SQLException
{
! final int rows_size = rows.size();
! return (current_row == rows_size -1 && rows_size > 0);
}
public boolean last() throws SQLException
{
! final int rows_size = rows.size();
! if (rows_size <= 0)
return false;
! current_row = rows_size - 1;
this_row = (byte [][])rows.elementAt(current_row);
return true;
}
***************
*** 1480,1483 ****
}
}
}
-
--- 1485,1487 ----
--
Joseph Shraibman
jks@selectacast.net
Increase signal to noise ratio. http://www.targabot.com
At 12:41 PM 9/5/2001, Joseph Shraibman wrote:
>new patch:
There still seems to be an error with the same if-block.
! if (index>=-rows.size())
! internalIndex=rows.size()+index;
becomes
! if (index > -rows_size)
! internalIndex = rows_size+index;
Note that the original used >=, not >.
Also, why is the first edit being done? Granted it's faster in that it
doesn't null out the array of rows, but won't that have other effects? I'm
not very familiar with the code, so pardon if I'm off base on that.
//release resources held (memory for tuples)
if(rows!=null) {
- rows.setSize(0);
rows=null;
}
Peace,
Dave
> > > Barry Lind wrote: > > Joseph, > > > > In looking at this patch it looks OK, except for the following change: > > > > > ! if (index>=-rows.size()) > > > --- 725,737 ---- > > > ! if (index > rows_size) > > > > I haven't looked at the entire method, but the change you made seems > > incorrect. > > > Oops! Thanks for catching that. Cut and paste error. I hate those. > > > If you want this patch to be applied it should be sent to the > > pgsql-patches mail list. > > > I thought that jdbc stuff was preferred to be on the jdbc list. I guess not. Actually, yes, I throw stuff to jdbc and patches for completeness. jdbc people, tell me what you want done in the future. -- 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
Joseph Shraibman <jks@selectacast.net> writes:
> Barry Lind wrote:
>> If you want this patch to be applied it should be sent to the
>> pgsql-patches mail list.
> I thought that jdbc stuff was preferred to be on the jdbc list.
> I guess not.
Well, patches are patches, and it's easier for the committers to spot
proposed patches that go by on pgsql-patches. However, the people who
are competent to review JDBC patches all seem to be hanging out on the
JDBC list. Seems like there are a couple of ways that we could handle
this:
1. First draft of a JDBC patch goes to JDBC list; if it passes muster
there then resend to pgsql-patches for application.
2. JDBC patches go to psql-patches only, and interested JDBC people
subscribe to pgsql-patches so they can kibitz.
3. We give commit privileges to one or two JDBC regulars, who take
responsibility for reviewing and applying JDBC-related patches after
discussion on pgsql-jdbc. (This was the old setup with Peter Mount,
but he seems not to have many spare cycles for Postgres anymore.)
Of these #3 seems like the solution that will emerge in the long term
anyway; but do we have candidate patch-meisters now?
Comments, better ideas, nominations, volunteers?
regards, tom lane
* Tom Lane <tgl@sss.pgh.pa.us> wrote: | | Of these #3 seems like the solution that will emerge in the long term | anyway; but do we have candidate patch-meisters now? | | Comments, better ideas, nominations, volunteers? I would like to nominate Barry Lind as he has been doing great work for a long time. Maybe 2-3 of the most active JDBC developers should be given commit access, so we're not dependent on one person to merge in patches. -- Gunnar Rønning - gunnar@polygnosis.com Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
On 06 Sep 2001 14:18:09 +0200, Gunnar Rønning wrote: >* Tom Lane <tgl@sss.pgh.pa.us> wrote: >| Of these #3 seems like the solution that will emerge in the long term >| anyway; but do we have candidate patch-meisters now? >| >| Comments, better ideas, nominations, volunteers? >I would like to nominate Barry Lind as he has been doing great work for a >long time. I second that, if Barry volunteers of course. >Maybe 2-3 of the most active JDBC developers should be given >commit access, so we're not dependent on one person to merge in patches. I'm not sure if I qualify, since I haven't been around very long, but I'll be glad to help out if volunteers are needed. I'm under the impression that JDBC receives more ad hoc patches from relative newcomers than the backend does. Therefore, I propose to follow a peer review procedure for applying patches to JDBC: 1) Every patch must be reviewable. It should be a clean diff and it must contain a clear description of the problem that's being solved, the reason for certain changes, JDBC compliance etc. 2) Every non-trivial patch should receive a positive recommendation from at least one person of a team of certified reviewers before it is applied. The review process (e.g. Q&A between reviewer and developer, approve/reject) occurs on the pgsql-jdbc list. This is already happening with a lot of patches, but I think it would be good to turn this practice into an official and documented procedure. Regards, René Pijlman <rene@lab.applinet.nl>
Rene Pijlman <rene@lab.applinet.nl> writes:
> I'm under the impression that JDBC receives more ad hoc patches
> from relative newcomers than the backend does. Therefore, I
> propose to follow a peer review procedure for applying patches
> to JDBC:
> 1) Every patch must be reviewable. It should be a clean diff and
> it must contain a clear description of the problem that's being
> solved, the reason for certain changes, JDBC compliance etc.
> 2) Every non-trivial patch should receive a positive
> recommendation from at least one person of a team of certified
> reviewers before it is applied. The review process (e.g. Q&A
> between reviewer and developer, approve/reject) occurs on the
> pgsql-jdbc list.
> This is already happening with a lot of patches, but I think it
> would be good to turn this practice into an official and
> documented procedure.
I would caution against getting overly bureaucratic. The Postgres
project has done quite nicely for the last five years with only informal
procedures, and I don't think we should change that dynamic.
Peer review is a good thing, no doubt about it, but don't get too
rigorous about it. Ultimately it's the committer's responsibility
to have confidence that the patch he applies is good; if he doesn't
feel competent to check it himself, he should call for more reviewers.
If he does feel sure about it, there's no need for procedural overhead.
Speaking of committers --- Barry, you've been nominated twice now.
Are you willing to accept that responsibility? I'll bring it up
with the core committee if you want to do it.
regards, tom lane
On Thu, 06 Sep 2001 14:30:49 -0400, you wrote: >Ultimately it's the committer's responsibility >to have confidence that the patch he applies is good; if he doesn't >feel competent to check it himself, he should call for more reviewers. >If he does feel sure about it, there's no need for procedural overhead. Agreed. If the committer is in the "team of certified reviewers" and is willing to review patches, our procedures are basically the same. Regards, René Pijlman <rene@lab.applinet.nl>
> On Thu, 06 Sep 2001 14:30:49 -0400, you wrote: > >Ultimately it's the committer's responsibility > >to have confidence that the patch he applies is good; if he doesn't > >feel competent to check it himself, he should call for more reviewers. > >If he does feel sure about it, there's no need for procedural overhead. > > Agreed. If the committer is in the "team of certified reviewers" > and is willing to review patches, our procedures are basically > the same. Any committing assistance would be greatly appreciated, especially for jdbc. -- 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
Dave Harkness wrote: > At 12:41 PM 9/5/2001, Joseph Shraibman wrote: > >> new patch: > > > There still seems to be an error with the same if-block. > > ! if (index>=-rows.size()) > ! internalIndex=rows.size()+index; > > becomes > > ! if (index > -rows_size) > ! internalIndex = rows_size+index; > > Note that the original used >=, not >. Argh! I need a better screen. :( > > Also, why is the first edit being done? Granted it's faster in that it > doesn't null out the array of rows, but won't that have other effects? No. Nulling out the array is pointless. -- Joseph Shraibman jks@selectacast.net Increase signal to noise ratio. http://www.targabot.com
I am willing to lend a hand if required Dave -----Original Message----- From: pgsql-jdbc-owner@postgresql.org [mailto:pgsql-jdbc-owner@postgresql.org] On Behalf Of Bruce Momjian Sent: September 6, 2001 4:08 PM To: Rene Pijlman Cc: Tom Lane; pgsql-jdbc@postgresql.org; pgsql-patches@postgresql.org Subject: Re: [JDBC] JDBC patch procedures (Re: [PATCHES] Patch for jdbc2 ResultSet.java) > On Thu, 06 Sep 2001 14:30:49 -0400, you wrote: > >Ultimately it's the committer's responsibility > >to have confidence that the patch he applies is good; if he doesn't > >feel competent to check it himself, he should call for more > >reviewers. If he does feel sure about it, there's no need for > >procedural overhead. > > Agreed. If the committer is in the "team of certified reviewers" and > is willing to review patches, our procedures are basically the same. Any committing assistance would be greatly appreciated, especially for jdbc. -- 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 ---------------------------(end of broadcast)--------------------------- TIP 6: Have you searched our list archives? http://www.postgresql.org/search.mpl
Thanks for the votes of confidence. I am willing to take on the responsibility if the core committee agrees. thanks, --Barry Rene Pijlman wrote: > On 06 Sep 2001 14:18:09 +0200, Gunnar Rønning wrote: > >>* Tom Lane <tgl@sss.pgh.pa.us> wrote: >>| Of these #3 seems like the solution that will emerge in the long term >>| anyway; but do we have candidate patch-meisters now? >>| >>| Comments, better ideas, nominations, volunteers? >> > >>I would like to nominate Barry Lind as he has been doing great work for a >>long time. >> > > I second that, if Barry volunteers of course. > > >>Maybe 2-3 of the most active JDBC developers should be given >>commit access, so we're not dependent on one person to merge in patches. >> > > I'm not sure if I qualify, since I haven't been around very > long, but I'll be glad to help out if volunteers are needed. > > I'm under the impression that JDBC receives more ad hoc patches > from relative newcomers than the backend does. Therefore, I > propose to follow a peer review procedure for applying patches > to JDBC: > > 1) Every patch must be reviewable. It should be a clean diff and > it must contain a clear description of the problem that's being > solved, the reason for certain changes, JDBC compliance etc. > > 2) Every non-trivial patch should receive a positive > recommendation from at least one person of a team of certified > reviewers before it is applied. The review process (e.g. Q&A > between reviewer and developer, approve/reject) occurs on the > pgsql-jdbc list. > > This is already happening with a lot of patches, but I think it > would be good to turn this practice into an official and > documented procedure. > > Regards, > René Pijlman <rene@lab.applinet.nl> >
> Thanks for the votes of confidence. I am willing to take on the > responsibility if the core committee agrees. > Yes, we do. We don't know Java very well and are struggling. It would be nice to have someone who actually know the language applying the patches. :-) -- 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
> Thanks for the votes of confidence. I am willing to take on the > responsibility if the core committee agrees. Well, actually, I was only speaking for myself, not for all of core, but I hope they will agree this is a good idea. -- 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
Can I have a new version of this for application? > > > Dave Harkness wrote: > > At 12:41 PM 9/5/2001, Joseph Shraibman wrote: > > > >> new patch: > > > > > > There still seems to be an error with the same if-block. > > > > ! if (index>=-rows.size()) > > ! internalIndex=rows.size()+index; > > > > becomes > > > > ! if (index > -rows_size) > > ! internalIndex = rows_size+index; > > > > Note that the original used >=, not >. > > Argh! I need a better screen. :( > > > > Also, why is the first edit being done? Granted it's faster in that it > > doesn't null out the array of rows, but won't that have other effects? > > No. Nulling out the array is pointless. > > > > > -- > Joseph Shraibman > jks@selectacast.net > Increase signal to noise ratio. http://www.targabot.com > > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go 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
Sorry, I see it now. > > > Dave Harkness wrote: > > At 12:41 PM 9/5/2001, Joseph Shraibman wrote: > > > >> new patch: > > > > > > There still seems to be an error with the same if-block. > > > > ! if (index>=-rows.size()) > > ! internalIndex=rows.size()+index; > > > > becomes > > > > ! if (index > -rows_size) > > ! internalIndex = rows_size+index; > > > > Note that the original used >=, not >. > > Argh! I need a better screen. :( > > > > Also, why is the first edit being done? Granted it's faster in that it > > doesn't null out the array of rows, but won't that have other effects? > > No. Nulling out the array is pointless. > > > > > -- > Joseph Shraibman > jks@selectacast.net > Increase signal to noise ratio. http://www.targabot.com > > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go 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
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 within the next 48 hours.
>
>
> Barry Lind wrote:
> > Joseph,
> >
> > In looking at this patch it looks OK, except for the following change:
> >
> > > ! if (index>=-rows.size())
> > > --- 725,737 ----
> > > ! if (index > rows_size)
> >
> > I haven't looked at the entire method, but the change you made seems
> > incorrect.
> >
> Oops! Thanks for catching that. Cut and paste error. I hate those.
>
> > If you want this patch to be applied it should be sent to the
> > pgsql-patches mail list.
>
>
> I thought that jdbc stuff was preferred to be on the jdbc list. I guess not.
>
> new patch:
>
>
> *** ResultSet.java.orig Tue Sep 4 16:34:14 2001
> --- ResultSet.java Wed Sep 5 15:35:59 2001
> ***************
> *** 134,140 ****
> {
> //release resources held (memory for tuples)
> if(rows!=null) {
> - rows.setSize(0);
> rows=null;
> }
> }
> --- 134,139 ----
> ***************
> *** 709,716 ****
> public int findColumn(String columnName) throws SQLException
> {
> int i;
> !
> ! for (i = 0 ; i < fields.length; ++i)
> if (fields[i].getName().equalsIgnoreCase(columnName))
> return (i+1);
> throw new PSQLException ("postgresql.res.colname",columnName);
> --- 708,715 ----
> public int findColumn(String columnName) throws SQLException
> {
> int i;
> ! final int flen = fields.length;
> ! for (i = 0 ; i < flen; ++i)
> if (fields[i].getName().equalsIgnoreCase(columnName))
> return (i+1);
> throw new PSQLException ("postgresql.res.colname",columnName);
> ***************
> *** 726,736 ****
> if (index==0)
> throw new SQLException("Cannot move to index of 0");
>
> //if index<0, count from the end of the result set, but check
> //to be sure that it is not beyond the first index
> if (index<0)
> ! if (index>=-rows.size())
> ! internalIndex=rows.size()+index;
> else {
> beforeFirst();
> return false;
> --- 725,737 ----
> if (index==0)
> throw new SQLException("Cannot move to index of 0");
>
> + final int rows_size = rows.size();
> +
> //if index<0, count from the end of the result set, but check
> //to be sure that it is not beyond the first index
> if (index<0)
> ! if (index > -rows_size)
> ! internalIndex = rows_size+index;
> else {
> beforeFirst();
> return false;
> ***************
> *** 739,745 ****
> //must be the case that index>0,
> //find the correct place, assuming that
> //the index is not too large
> ! if (index<=rows.size())
> internalIndex = index-1;
> else {
> afterLast();
> --- 740,746 ----
> //must be the case that index>0,
> //find the correct place, assuming that
> //the index is not too large
> ! if (index <= rows_size)
> internalIndex = index-1;
> else {
> afterLast();
> ***************
> *** 753,760 ****
>
> public void afterLast() throws SQLException
> {
> ! if (rows.size() > 0)
> ! current_row = rows.size();
> }
>
> public void beforeFirst() throws SQLException
> --- 754,762 ----
>
> public void afterLast() throws SQLException
> {
> ! final int rows_size = rows.size();
> ! if (rows_size > 0)
> ! current_row = rows_size;
> }
>
> public void beforeFirst() throws SQLException
> ***************
> *** 967,973 ****
>
> public boolean isAfterLast() throws SQLException
> {
> ! return (current_row >= rows.size() && rows.size() > 0);
> }
>
> public boolean isBeforeFirst() throws SQLException
> --- 969,976 ----
>
> public boolean isAfterLast() throws SQLException
> {
> ! final int rows_size = rows.size();
> ! return (current_row >= rows_size && rows_size > 0);
> }
>
> public boolean isBeforeFirst() throws SQLException
> ***************
> *** 982,995 ****
>
> public boolean isLast() throws SQLException
> {
> ! return (current_row == rows.size() -1 && rows.size() > 0);
> }
>
> public boolean last() throws SQLException
> {
> ! if (rows.size() <= 0)
> return false;
> ! current_row = rows.size() - 1;
> this_row = (byte [][])rows.elementAt(current_row);
> return true;
> }
> --- 985,1000 ----
>
> public boolean isLast() throws SQLException
> {
> ! final int rows_size = rows.size();
> ! return (current_row == rows_size -1 && rows_size > 0);
> }
>
> public boolean last() throws SQLException
> {
> ! final int rows_size = rows.size();
> ! if (rows_size <= 0)
> return false;
> ! current_row = rows_size - 1;
> this_row = (byte [][])rows.elementAt(current_row);
> return true;
> }
> ***************
> *** 1480,1483 ****
> }
> }
> }
> -
> --- 1485,1487 ----
>
>
>
>
>
>
>
> --
> Joseph Shraibman
> jks@selectacast.net
> Increase signal to noise ratio. http://www.targabot.com
>
>
> ---------------------------(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. Thanks.
The patch did not apply so I had to merge it in manually. Patch
attached. I have review the diff I made vs. your.
>
>
> Barry Lind wrote:
> > Joseph,
> >
> > In looking at this patch it looks OK, except for the following change:
> >
> > > ! if (index>=-rows.size())
> > > --- 725,737 ----
> > > ! if (index > rows_size)
> >
> > I haven't looked at the entire method, but the change you made seems
> > incorrect.
> >
> Oops! Thanks for catching that. Cut and paste error. I hate those.
>
> > If you want this patch to be applied it should be sent to the
> > pgsql-patches mail list.
>
>
> I thought that jdbc stuff was preferred to be on the jdbc list. I guess not.
>
> new patch:
>
>
> *** ResultSet.java.orig Tue Sep 4 16:34:14 2001
> --- ResultSet.java Wed Sep 5 15:35:59 2001
> ***************
> *** 134,140 ****
> {
> //release resources held (memory for tuples)
> if(rows!=null) {
> - rows.setSize(0);
> rows=null;
> }
> }
> --- 134,139 ----
> ***************
> *** 709,716 ****
> public int findColumn(String columnName) throws SQLException
> {
> int i;
> !
> ! for (i = 0 ; i < fields.length; ++i)
> if (fields[i].getName().equalsIgnoreCase(columnName))
> return (i+1);
> throw new PSQLException ("postgresql.res.colname",columnName);
> --- 708,715 ----
> public int findColumn(String columnName) throws SQLException
> {
> int i;
> ! final int flen = fields.length;
> ! for (i = 0 ; i < flen; ++i)
> if (fields[i].getName().equalsIgnoreCase(columnName))
> return (i+1);
> throw new PSQLException ("postgresql.res.colname",columnName);
> ***************
> *** 726,736 ****
> if (index==0)
> throw new SQLException("Cannot move to index of 0");
>
> //if index<0, count from the end of the result set, but check
> //to be sure that it is not beyond the first index
> if (index<0)
> ! if (index>=-rows.size())
> ! internalIndex=rows.size()+index;
> else {
> beforeFirst();
> return false;
> --- 725,737 ----
> if (index==0)
> throw new SQLException("Cannot move to index of 0");
>
> + final int rows_size = rows.size();
> +
> //if index<0, count from the end of the result set, but check
> //to be sure that it is not beyond the first index
> if (index<0)
> ! if (index > -rows_size)
> ! internalIndex = rows_size+index;
> else {
> beforeFirst();
> return false;
> ***************
> *** 739,745 ****
> //must be the case that index>0,
> //find the correct place, assuming that
> //the index is not too large
> ! if (index<=rows.size())
> internalIndex = index-1;
> else {
> afterLast();
> --- 740,746 ----
> //must be the case that index>0,
> //find the correct place, assuming that
> //the index is not too large
> ! if (index <= rows_size)
> internalIndex = index-1;
> else {
> afterLast();
> ***************
> *** 753,760 ****
>
> public void afterLast() throws SQLException
> {
> ! if (rows.size() > 0)
> ! current_row = rows.size();
> }
>
> public void beforeFirst() throws SQLException
> --- 754,762 ----
>
> public void afterLast() throws SQLException
> {
> ! final int rows_size = rows.size();
> ! if (rows_size > 0)
> ! current_row = rows_size;
> }
>
> public void beforeFirst() throws SQLException
> ***************
> *** 967,973 ****
>
> public boolean isAfterLast() throws SQLException
> {
> ! return (current_row >= rows.size() && rows.size() > 0);
> }
>
> public boolean isBeforeFirst() throws SQLException
> --- 969,976 ----
>
> public boolean isAfterLast() throws SQLException
> {
> ! final int rows_size = rows.size();
> ! return (current_row >= rows_size && rows_size > 0);
> }
>
> public boolean isBeforeFirst() throws SQLException
> ***************
> *** 982,995 ****
>
> public boolean isLast() throws SQLException
> {
> ! return (current_row == rows.size() -1 && rows.size() > 0);
> }
>
> public boolean last() throws SQLException
> {
> ! if (rows.size() <= 0)
> return false;
> ! current_row = rows.size() - 1;
> this_row = (byte [][])rows.elementAt(current_row);
> return true;
> }
> --- 985,1000 ----
>
> public boolean isLast() throws SQLException
> {
> ! final int rows_size = rows.size();
> ! return (current_row == rows_size -1 && rows_size > 0);
> }
>
> public boolean last() throws SQLException
> {
> ! final int rows_size = rows.size();
> ! if (rows_size <= 0)
> return false;
> ! current_row = rows_size - 1;
> this_row = (byte [][])rows.elementAt(current_row);
> return true;
> }
> ***************
> *** 1480,1483 ****
> }
> }
> }
> -
> --- 1485,1487 ----
>
>
>
>
>
>
>
> --
> Joseph Shraibman
> jks@selectacast.net
> Increase signal to noise ratio. http://www.targabot.com
>
>
> ---------------------------(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
Index: src/interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java,v
retrieving revision 1.31
diff -c -r1.31 ResultSet.java
*** src/interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java 2001/08/24 16:50:18 1.31
--- src/interfaces/jdbc/org/postgresql/jdbc2/ResultSet.java 2001/09/07 22:12:18
***************
*** 134,140 ****
{
//release resources held (memory for tuples)
if(rows!=null) {
- rows.setSize(0);
rows=null;
}
}
--- 134,139 ----
***************
*** 710,716 ****
{
int i;
! for (i = 0 ; i < fields.length; ++i)
if (fields[i].getName().equalsIgnoreCase(columnName))
return (i+1);
throw new PSQLException ("postgresql.res.colname",columnName);
--- 709,716 ----
{
int i;
! final int flen = fields.length;
! for (i = 0 ; i < flen; ++i)
if (fields[i].getName().equalsIgnoreCase(columnName))
return (i+1);
throw new PSQLException ("postgresql.res.colname",columnName);
***************
*** 726,736 ****
if (index==0)
throw new SQLException("Cannot move to index of 0");
//if index<0, count from the end of the result set, but check
//to be sure that it is not beyond the first index
if (index<0)
! if (index>=-rows.size())
! internalIndex=rows.size()+index;
else {
beforeFirst();
return false;
--- 726,738 ----
if (index==0)
throw new SQLException("Cannot move to index of 0");
+ final int rows_size = rows.size();
+
//if index<0, count from the end of the result set, but check
//to be sure that it is not beyond the first index
if (index<0)
! if (index > -rows_size)
! internalIndex = rows_size+index;
else {
beforeFirst();
return false;
***************
*** 739,745 ****
//must be the case that index>0,
//find the correct place, assuming that
//the index is not too large
! if (index<=rows.size())
internalIndex = index-1;
else {
afterLast();
--- 741,747 ----
//must be the case that index>0,
//find the correct place, assuming that
//the index is not too large
! if (index <= rows_size)
internalIndex = index-1;
else {
afterLast();
***************
*** 753,760 ****
public void afterLast() throws SQLException
{
! if (rows.size() > 0)
! current_row = rows.size();
}
public void beforeFirst() throws SQLException
--- 755,763 ----
public void afterLast() throws SQLException
{
! final int rows_size = rows.size();
! if (rows_size > 0)
! current_row = rows_size;
}
public void beforeFirst() throws SQLException
***************
*** 967,973 ****
public boolean isAfterLast() throws SQLException
{
! return (current_row >= rows.size() && rows.size() > 0);
}
public boolean isBeforeFirst() throws SQLException
--- 970,977 ----
public boolean isAfterLast() throws SQLException
{
! final int rows_size = rows.size();
! return (current_row >= rows_size && rows_size > 0);
}
public boolean isBeforeFirst() throws SQLException
***************
*** 982,997 ****
public boolean isLast() throws SQLException
{
! return (current_row == rows.size() -1 && rows.size() > 0);
}
public boolean last() throws SQLException
{
! if (rows.size() <= 0)
! return false;
! current_row = rows.size() - 1;
! this_row = (byte [][])rows.elementAt(current_row);
! return true;
}
public void moveToCurrentRow() throws SQLException
--- 986,1003 ----
public boolean isLast() throws SQLException
{
! final int rows_size = rows.size();
! return (current_row == rows_size -1 && rows_size > 0);
}
public boolean last() throws SQLException
{
! final int rows_size = rows.size();
! if (rows_size <= 0)
! return false;
! current_row = rows_size - 1;
! this_row = (byte [][])rows.elementAt(current_row);
! return true;
}
public void moveToCurrentRow() throws SQLException
***************
*** 1480,1483 ****
}
}
}
-
--- 1486,1488 ----
I am willing to lend a hand, if required Dave -----Original Message----- From: pgsql-jdbc-owner@postgresql.org [mailto:pgsql-jdbc-owner@postgresql.org] On Behalf Of Bruce Momjian Sent: September 6, 2001 4:08 PM To: Rene Pijlman Cc: Tom Lane; pgsql-jdbc@postgresql.org; pgsql-patches@postgresql.org Subject: Re: [JDBC] JDBC patch procedures (Re: [PATCHES] Patch for jdbc2 ResultSet.java) > On Thu, 06 Sep 2001 14:30:49 -0400, you wrote: > >Ultimately it's the committer's responsibility > >to have confidence that the patch he applies is good; if he doesn't > >feel competent to check it himself, he should call for more > >reviewers. If he does feel sure about it, there's no need for > >procedural overhead. > > Agreed. If the committer is in the "team of certified reviewers" and > is willing to review patches, our procedures are basically the same. Any committing assistance would be greatly appreciated, especially for jdbc. -- 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 ---------------------------(end of broadcast)--------------------------- TIP 6: Have you searched our list archives? http://www.postgresql.org/search.mpl