Thread: Cursornames
This fixes the behaviour that Felipe Schnack noticed. The logic goes, if the statement name is null (which happens when you do not use cursors or server prepared statements), then we have already fetched all the rows, and there are no more. Cheers, Kim
Attachment
I can't understand what you mean... You shouldn't fetch all the rows if you specify a fetchsize and aren't using server-sidepreparedstatements... On 04 Aug 2003 17:38:21 -0400 Kim Ho <kho@redhat.com> wrote: > This fixes the behaviour that Felipe Schnack noticed. > > The logic goes, if the statement name is null (which happens when you do > not use cursors or server prepared statements), then we have already > fetched all the rows, and there are no more. > > Cheers, > > Kim > -- /~\ The ASCII Felipe Schnack (felipes@ritterdosreis.br) \ / Ribbon Campaign Analista de Sistemas X Against HTML Cel.: 51-91287530 / \ Email! Linux Counter #281893 Centro Universitário Ritter dos Reis http://www.ritterdosreis.br ritter@ritterdosreis.br Fone: 51-32303341
I looked a little bit more closely into this matter and Kim's patch seems correct to me. We do need better comments or to refactor the code to make this logic clearer. What _I think_ happens is that we attempt to create a cursor for implementing the fetch size but sometimes this is not possible. For instance, the query is itself creating a cursor (a DECLARE statement) or there are multiple queries (statements separated by ';'). As setFetchSize() is just a hint for the driver (it does not _need_ to consider it), the driver uses the original statement and fetches the whole lot (the complete result set). In this case the statement name is null. I would just move the test to right after the test to fetchsize and explain the situation in a comment. Regards to all, Fernando P.S.: Which means that we indeed should not generate an exception if we cannot create the cursor as I originally thought. Kim Ho wrote: > This fixes the behaviour that Felipe Schnack noticed. > > The logic goes, if the statement name is null (which happens when you do > not use cursors or server prepared statements), then we have already > fetched all the rows, and there are no more. > > Cheers, > > Kim > > > ------------------------------------------------------------------------ > > Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java > =================================================================== > RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v > retrieving revision 1.13 > diff -c -p -r1.13 AbstractJdbc1ResultSet.java > *** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 30 Jun 2003 21:10:55 -0000 1.13 > --- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 4 Aug 2003 21:32:11 -0000 > *************** public abstract class AbstractJdbc1Resul > *** 131,136 **** > --- 131,138 ---- > String[] binds = new String[0]; > // Is this the correct query??? > String cursorName = statement.getStatementName(); > + if (cursorName == null) > + return false; > sql[0] = "FETCH FORWARD " + fetchSize + " FROM " + cursorName; > QueryExecutor.execute(sql, > binds, > > > ------------------------------------------------------------------------ > > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly -- Fernando Nasser Red Hat Canada Ltd. E-Mail: fnasser@redhat.com 2323 Yonge Street, Suite #300 Toronto, Ontario M4P 2C9
Do you still want me to write a test case for the bug I found before Kim sent his patch? I can write it today... Anyway, I know in the spec setFetchSize() is just a hint for the driver, but the current pgsql's driver behavior isn'tto create a cursor? I'm pretty sure the query that generated my error was very simple select clause. I don't use DECLAREor multiple queries anywhere in the app. Btw: the idea of using cursors isn't to prevent the backend from loading all the query results into memory? On Tue, 05 Aug 2003 11:13:18 -0400 Fernando Nasser <fnasser@redhat.com> wrote: > I looked a little bit more closely into this matter and Kim's patch > seems correct to me. We do need better comments or to refactor the code > to make this logic clearer. > > What _I think_ happens is that we attempt to create a cursor for > implementing the fetch size but sometimes this is not possible. For > instance, the query is itself creating a cursor (a DECLARE statement) or > there are multiple queries (statements separated by ';'). > > As setFetchSize() is just a hint for the driver (it does not _need_ to > consider it), the driver uses the original statement and fetches the > whole lot (the complete result set). In this case the statement name is > null. > > I would just move the test to right after the test to fetchsize and > explain the situation in a comment. > > Regards to all, > Fernando > > P.S.: Which means that we indeed should not generate an exception if we > cannot create the cursor as I originally thought. > > > > Kim Ho wrote: > > This fixes the behaviour that Felipe Schnack noticed. > > > > The logic goes, if the statement name is null (which happens when you do > > not use cursors or server prepared statements), then we have already > > fetched all the rows, and there are no more. > > > > Cheers, > > > > Kim > > > > > > ------------------------------------------------------------------------ > > > > Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java > > =================================================================== > > RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v > > retrieving revision 1.13 > > diff -c -p -r1.13 AbstractJdbc1ResultSet.java > > *** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 30 Jun 2003 21:10:55 -0000 1.13 > > --- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 4 Aug 2003 21:32:11 -0000 > > *************** public abstract class AbstractJdbc1Resul > > *** 131,136 **** > > --- 131,138 ---- > > String[] binds = new String[0]; > > // Is this the correct query??? > > String cursorName = statement.getStatementName(); > > + if (cursorName == null) > > + return false; > > sql[0] = "FETCH FORWARD " + fetchSize + " FROM " + cursorName; > > QueryExecutor.execute(sql, > > binds, > > > > > > ------------------------------------------------------------------------ > > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 3: if posting/reading through Usenet, please send an appropriate > > subscribe-nomail command to majordomo@postgresql.org so that your > > message can get through to the mailing list cleanly > > > -- > Fernando Nasser > Red Hat Canada Ltd. E-Mail: fnasser@redhat.com > 2323 Yonge Street, Suite #300 > Toronto, Ontario M4P 2C9 > > > ---------------------------(end of broadcast)--------------------------- > TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) -- /~\ The ASCII Felipe Schnack (felipes@ritterdosreis.br) \ / Ribbon Campaign Analista de Sistemas X Against HTML Cel.: 51-91287530 / \ Email! Linux Counter #281893 Centro Universitário Ritter dos Reis http://www.ritterdosreis.br ritter@ritterdosreis.br Fone: 51-32303341
Felipe Schnack wrote: > Do you still want me to write a test case for the bug I found before Kim sent his patch? Yes, Dave wants to add a test case for each bug we find, so that it does not come back. > I can write it today... I would appreciate if you could take the time to do it. > Anyway, I know in the spec setFetchSize() is just a hint for the driver, but the current pgsql's driver behavior isn'tto create a cursor? Only if it can, if it cannot it just sends the query as is and gets the full result set. > I'm pretty sure the query that generated my error was very simple select clause. I don't use DECLARE or multiple queriesanywhere in the app. One more reason for us to have a test case. As Kim pointed out, if you have autocommit off you should get a cursor to implement your fetch size even if you are not using server prepare. Unless the driver deemed your query unsuitable for a cursor. Maybe there is an error in the small parsing that is done there. We do need a way to reproduce this. > Btw: the idea of using cursors isn't to prevent the backend from loading all the query results into memory? > It is meant as a way to match the "impedance" between host languages which usually specify scalar variables to receive the data retrieved and SQL (which can return sets). Regards, Fernando > On Tue, 05 Aug 2003 11:13:18 -0400 > Fernando Nasser <fnasser@redhat.com> wrote: > > >>I looked a little bit more closely into this matter and Kim's patch >>seems correct to me. We do need better comments or to refactor the code >>to make this logic clearer. >> >>What _I think_ happens is that we attempt to create a cursor for >>implementing the fetch size but sometimes this is not possible. For >>instance, the query is itself creating a cursor (a DECLARE statement) or >>there are multiple queries (statements separated by ';'). >> >>As setFetchSize() is just a hint for the driver (it does not _need_ to >>consider it), the driver uses the original statement and fetches the >>whole lot (the complete result set). In this case the statement name is >>null. >> >>I would just move the test to right after the test to fetchsize and >>explain the situation in a comment. >> >>Regards to all, >>Fernando >> >>P.S.: Which means that we indeed should not generate an exception if we >>cannot create the cursor as I originally thought. >> >> >> >>Kim Ho wrote: >> >>>This fixes the behaviour that Felipe Schnack noticed. >>> >>>The logic goes, if the statement name is null (which happens when you do >>>not use cursors or server prepared statements), then we have already >>>fetched all the rows, and there are no more. >>> >>>Cheers, >>> >>>Kim >>> >>> >>>------------------------------------------------------------------------ >>> >>>Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java >>>=================================================================== >>>RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v >>>retrieving revision 1.13 >>>diff -c -p -r1.13 AbstractJdbc1ResultSet.java >>>*** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 30 Jun 2003 21:10:55 -0000 1.13 >>>--- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 4 Aug 2003 21:32:11 -0000 >>>*************** public abstract class AbstractJdbc1Resul >>>*** 131,136 **** >>>--- 131,138 ---- >>> String[] binds = new String[0]; >>> // Is this the correct query??? >>> String cursorName = statement.getStatementName(); >>>+ if (cursorName == null) >>>+ return false; >>> sql[0] = "FETCH FORWARD " + fetchSize + " FROM " + cursorName; >>> QueryExecutor.execute(sql, >>> binds, >>> >>> >>>------------------------------------------------------------------------ >>> >>> >>>---------------------------(end of broadcast)--------------------------- >>>TIP 3: if posting/reading through Usenet, please send an appropriate >>> subscribe-nomail command to majordomo@postgresql.org so that your >>> message can get through to the mailing list cleanly >> >> >>-- >>Fernando Nasser >>Red Hat Canada Ltd. E-Mail: fnasser@redhat.com >>2323 Yonge Street, Suite #300 >>Toronto, Ontario M4P 2C9 >> >> >>---------------------------(end of broadcast)--------------------------- >>TIP 2: you can get off all lists at once with the unregister command >> (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > > > -- Fernando Nasser Red Hat Canada Ltd. E-Mail: fnasser@redhat.com 2323 Yonge Street, Suite #300 Toronto, Ontario M4P 2C9
> > I can write it today... > I would appreciate if you could take the time to do it. Ok, I will > > Anyway, I know in the spec setFetchSize() is just a hint for the > > driver, but the current pgsql's driver behavior isn't to create a > >cursor? > Only if it can, if it cannot it just sends the query as is and gets > the full result set. When it can't? When there are multiple queries in the SQL or when you're using DECLARE I can understand... there are moresituations? It can use a cursor when I'm not using servir-side prepared statements? > It is meant as a way to match the "impedance" between host languages > which usually specify scalar variables to receive the data retrieved > and SQL (which can return sets). Hm... impedance? I don't think I understand you. Maybe you can send me a private message in portuguese, as far as I rememberyou're also a Brazilian :-) But which are the situations that pgsql loads all data from the query to server's memory? I had this once, can't rememberhow I fixed it. -- /~\ The ASCII Felipe Schnack (felipes@ritterdosreis.br) \ / Ribbon Campaign Analista de Sistemas X Against HTML Cel.: 51-91287530 / \ Email! Linux Counter #281893 Centro Universitário Ritter dos Reis http://www.ritterdosreis.br ritter@ritterdosreis.br Fone: 51-32303341
Felipe Schnack wrote: >>>I can write it today... >> >>I would appreciate if you could take the time to do it. > > Ok, I will > Thanks. > >>> Anyway, I know in the spec setFetchSize() is just a hint for the >>>driver, but the current pgsql's driver behavior isn't to create a >>>cursor? >> >>Only if it can, if it cannot it just sends the query as is and gets >>the full result set. > > When it can't? When there are multiple queries in the SQL or when you're using DECLARE I can understand... there aremore situations? Theorectically these are the two situations tested for. Maybe there is a parsing problem.. It can use a cursor when I'm not using servir-side prepared statements? > Yes, it makes a DECLARE CURSOR with your statement in it, but only if you have autocommit off (you must be in a transaction to use cursors...). > >>It is meant as a way to match the "impedance" between host languages >>which usually specify scalar variables to receive the data retrieved >>and SQL (which can return sets). > > Hm... impedance? I don't think I understand you. Maybe you can send me a private message in portuguese, as far as I rememberyou're also a Brazilian :-) Yes I could, but it would translate to "impedancia" so it would not help us much :-) What I am tryint to say is that host variables are scalar so thay can hold just one value (from one rwo), so if your query returns more than one row you must get one at a time. > But which are the situations that pgsql loads all data from the query to server's memory? I had this once, can't rememberhow I fixed it. Not sure. You would have to ask the backend folks. -- Fernando Nasser Red Hat Canada Ltd. E-Mail: fnasser@redhat.com 2323 Yonge Street, Suite #300 Toronto, Ontario M4P 2C9
Patch applied. thanks, --Barry PS. I would still like the mentioned addition to the test suite to check for this case. Kim Ho wrote: > This fixes the behaviour that Felipe Schnack noticed. > > The logic goes, if the statement name is null (which happens when you do > not use cursors or server prepared statements), then we have already > fetched all the rows, and there are no more. > > Cheers, > > Kim > > > ------------------------------------------------------------------------ > > Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java > =================================================================== > RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v > retrieving revision 1.13 > diff -c -p -r1.13 AbstractJdbc1ResultSet.java > *** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 30 Jun 2003 21:10:55 -0000 1.13 > --- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 4 Aug 2003 21:32:11 -0000 > *************** public abstract class AbstractJdbc1Resul > *** 131,136 **** > --- 131,138 ---- > String[] binds = new String[0]; > // Is this the correct query??? > String cursorName = statement.getStatementName(); > + if (cursorName == null) > + return false; > sql[0] = "FETCH FORWARD " + fetchSize + " FROM " + cursorName; > QueryExecutor.execute(sql, > binds, > > > ------------------------------------------------------------------------ > > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly