Re: [HACKERS] move 0 behaviour
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > Tom Lane wrote: > >> Bruce Momjian <[EMAIL PROTECTED]> writes: > >>> I thought about that, but I need to fail if the cursor name is invalid. > >> > >> What has that got to do with it? > > > If I put the 'return' for 0 MOVE/FETCH in utility.c's FetchStmt code, I > > will not get the checks for invalid cursor names, and I will not get the > > proper return tag. > > Oh, I see. Yeah, you're probably right, we have to change the calling > convention for PerformPortalFetch. > > BTW, portalcmds.h also contains a comment that would need to be fixed. Updated. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [HACKERS] move 0 behaviour
Bruce Momjian <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Bruce Momjian <[EMAIL PROTECTED]> writes: >>> I thought about that, but I need to fail if the cursor name is invalid. >> >> What has that got to do with it? > If I put the 'return' for 0 MOVE/FETCH in utility.c's FetchStmt code, I > will not get the checks for invalid cursor names, and I will not get the > proper return tag. Oh, I see. Yeah, you're probably right, we have to change the calling convention for PerformPortalFetch. BTW, portalcmds.h also contains a comment that would need to be fixed. regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] move 0 behaviour
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > Tom Lane wrote: > >> Do not hack up PerformPortalFetch; put the special case for INT_MAX in > >> utility.c's FetchStmt code, instead. As-is, you probably broke other > >> callers of PerformPortalFetch. > > > I thought about that, but I need to fail if the cursor name is invalid. > > What has that got to do with it? If I put the 'return' for 0 MOVE/FETCH in utility.c's FetchStmt code, I will not get the checks for invalid cursor names, and I will not get the proper return tag. I don't see how to do anything in utility.c. I assume this is the code you want to move to utility.c: + /* If zero count, we are done */ + if (count == 0) + return; + + /* Internally, zero count processes all portal rows */ + if (count == INT_MAX) + count = 0; + -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [HACKERS] move 0 behaviour
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > The following patch makes FETCH/MOVE 0 do nothing, and FETCH LAST move
> > to the end.
>
> Do not hack up PerformPortalFetch; put the special case for INT_MAX in
> utility.c's FetchStmt code, instead. As-is, you probably broke other
> callers of PerformPortalFetch.
I thought about that, but I need to fail if the cursor name is invalid.
Those tests are done in PerformPortalFetch(). The good news is that no
one else call it. Other ideas?
> BTW, there's a comment in parsenodes.h that needs to be fixed too:
>
> inthowMany;/* amount to fetch ("ALL" --> 0) */
Done.
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [HACKERS] move 0 behaviour
Bruce Momjian <[EMAIL PROTECTED]> writes:
> The following patch makes FETCH/MOVE 0 do nothing, and FETCH LAST move
> to the end.
Do not hack up PerformPortalFetch; put the special case for INT_MAX in
utility.c's FetchStmt code, instead. As-is, you probably broke other
callers of PerformPortalFetch.
BTW, there's a comment in parsenodes.h that needs to be fixed too:
inthowMany;/* amount to fetch ("ALL" --> 0) */
regards, tom lane
---(end of broadcast)---
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
Re: [HACKERS] move 0 behaviour
Peter Eisentraut wrote:
> Bruce Momjian writes:
>
> > So, that is why MOVE 0 goes to the end of the cursor. One idea would be
> > for MOVE 0 to actually move nothing, but jdbc and others need the
> > ability to move the end of the cursor, perhaps to then back up a certain
> > amount and read from there. Seems MOVE 0 is the logical way to do that.
> > (I can't think of another reasonable value).
>
> It would seem more logical and reasonable for MOVE 0 to do nothing and
> have some special syntax such as MOVE LAST to move to the end. (MOVE LAST
> would actually be consistent with the standard syntax FETCH LAST.)
Yea, I started thinking and we need to get MOVE/FETCH to make sense.
The following patch makes FETCH/MOVE 0 do nothing, and FETCH LAST move
to the end. I was going to use the word END, but if LAST is more
standard, we will use that. It uses INT_MAX in the grammar for FETCH
ALL/MOVE LAST, but maps that to zero so it is consistent in the
/executor code.
I will keep this patch for 7.4.
JDBC folks, I realize you need this. Seems you will have to use MOVE 0
for 7,3 and MOVE LAST for 7.4.
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/ref/move.sgml
===
RCS file: /cvsroot/pgsql-server/doc/src/sgml/ref/move.sgml,v
retrieving revision 1.13
diff -c -c -r1.13 move.sgml
*** doc/src/sgml/ref/move.sgml 21 Apr 2002 19:02:39 - 1.13
--- doc/src/sgml/ref/move.sgml 31 Oct 2002 01:15:42 -
***
*** 21,27
1999-07-20
! MOVE [ direction ] [ count ]
{ IN | FROM } cursor
--- 21,28
1999-07-20
! MOVE [ direction ]
! {count | LAST }
{ IN | FROM } cursor
Index: src/backend/commands/portalcmds.c
===
RCS file: /cvsroot/pgsql-server/src/backend/commands/portalcmds.c,v
retrieving revision 1.3
diff -c -c -r1.3 portalcmds.c
*** src/backend/commands/portalcmds.c 4 Sep 2002 20:31:15 - 1.3
--- src/backend/commands/portalcmds.c 31 Oct 2002 01:15:44 -
***
*** 15,20
--- 15,22
#include "postgres.h"
+ #include
+
#include "commands/portalcmds.h"
#include "executor/executor.h"
***
*** 55,61
*
*name: name of portal
*forward: forward or backward fetch?
! *count: # of tuples to fetch (0 implies all)
*dest: where to send results
*completionTag: points to a buffer of size COMPLETION_TAG_BUFSIZE
*in which to store a command completion status string.
--- 57,63
*
*name: name of portal
*forward: forward or backward fetch?
! *count: # of tuples to fetch
*dest: where to send results
*completionTag: points to a buffer of size COMPLETION_TAG_BUFSIZE
*in which to store a command completion status string.
***
*** 100,105
--- 102,115
return;
}
+ /* If zero count, we are done */
+ if (count == 0)
+ return;
+
+ /* Internally, zero count processes all portal rows */
+ if (count == INT_MAX)
+ count = 0;
+
/*
* switch into the portal context
*/
Index: src/backend/executor/execMain.c
===
RCS file: /cvsroot/pgsql-server/src/backend/executor/execMain.c,v
retrieving revision 1.180
diff -c -c -r1.180 execMain.c
*** src/backend/executor/execMain.c 14 Oct 2002 16:51:30 - 1.180
--- src/backend/executor/execMain.c 31 Oct 2002 01:15:50 -
***
*** 1119,1125
/*
* check our tuple count.. if we've processed the proper number
!* then quit, else loop again and process more tuples..
*/
current_tuple_count++;
if (numberTuples == current_tuple_count)
--- 1119,1126
/*
* check our tuple count.. if we've processed the proper number
!* then quit, else loop again and process more tuples. Zero
!* number_tuples means no limit.
*/
current_tuple_count++;
if (numberTuples == current_tuple_count)
Index: src/backend/parser/gram.y
===
RCS file: /cvsroot/pgsql-server/src/backend/parser/gram.y,v
retrieving revision 2.370
diff -c -c -r2.370 gram.y
*** src/backend/parser/gram.y 22 Sep 2002 21:44:43 - 2.370
--- src/backend/parser/gram.y 31 Oct 2002 01:16:14 -
***
*** 49,54
--- 49,55
#include "postg
Re: [HACKERS] move 0 behaviour
Bruce Momjian writes: > So, that is why MOVE 0 goes to the end of the cursor. One idea would be > for MOVE 0 to actually move nothing, but jdbc and others need the > ability to move the end of the cursor, perhaps to then back up a certain > amount and read from there. Seems MOVE 0 is the logical way to do that. > (I can't think of another reasonable value). It would seem more logical and reasonable for MOVE 0 to do nothing and have some special syntax such as MOVE LAST to move to the end. (MOVE LAST would actually be consistent with the standard syntax FETCH LAST.) -- Peter Eisentraut [EMAIL PROTECTED] ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] move 0 behaviour
Bruce Momjian <[EMAIL PROTECTED]> writes: > I did some research on this. It turns out the parser uses 0 for ALL, so > when you do a FETCH ALL it is passing zero. Now, when you do MOVE 0, > you are really asking for FETCH ALL and all the tuples are thrown away > because of the MOVE. Yeah. I think this is a bug and "MOVE 0" ought to be a no-op ... but changing it requires a different parsetree representation for MOVE ALL, which is tedious enough that it hasn't gotten done yet. > I have the following patch which just documents the fact that MOVE 0 > goes to the end of the cursor. It does not change any behavior, just > document it. It should be documented as behavior that is likely to change. Also, I believe FETCH 0 has the same issue. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] move 0 behaviour
Dave Cramer wrote: > Currently there is a TODO list item to have move 0 not position to the > end of the cursor. > > Moving to the end of the cursor is useful, can we keep the behaviour and > change it to move end, or just leave it the way it is? I did some research on this. It turns out the parser uses 0 for ALL, so when you do a FETCH ALL it is passing zero. Now, when you do MOVE 0, you are really asking for FETCH ALL and all the tuples are thrown away because of the MOVE. So, that is why MOVE 0 goes to the end of the cursor. One idea would be for MOVE 0 to actually move nothing, but jdbc and others need the ability to move the end of the cursor, perhaps to then back up a certain amount and read from there. Seems MOVE 0 is the logical way to do that. (I can't think of another reasonable value). I have the following patch which just documents the fact that MOVE 0 goes to the end of the cursor. It does not change any behavior, just document it. If/when I apply the patch, I will remove the TODO item. Another idea would be to require MOVE END to move to the end. Comments? -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/ref/move.sgml === RCS file: /cvsroot/pgsql-server/doc/src/sgml/ref/move.sgml,v retrieving revision 1.13 diff -c -c -r1.13 move.sgml *** doc/src/sgml/ref/move.sgml 21 Apr 2002 19:02:39 - 1.13 --- doc/src/sgml/ref/move.sgml 26 Oct 2002 20:01:15 - *** *** 37,44 MOVE allows a user to move cursor position a specified number of rows. MOVE works like the FETCH command, !but only positions the cursor and does !not return rows. Refer to --- 37,44 MOVE allows a user to move cursor position a specified number of rows. MOVE works like the FETCH command, !but only positions the cursor and does not return rows. The special !direction 0 moves to the end of the cursor. Refer to Index: src/backend/executor/execMain.c === RCS file: /cvsroot/pgsql-server/src/backend/executor/execMain.c,v retrieving revision 1.180 diff -c -c -r1.180 execMain.c *** src/backend/executor/execMain.c 14 Oct 2002 16:51:30 - 1.180 --- src/backend/executor/execMain.c 26 Oct 2002 20:01:20 - *** *** 1119,1125 /* * check our tuple count.. if we've processed the proper number !* then quit, else loop again and process more tuples.. */ current_tuple_count++; if (numberTuples == current_tuple_count) --- 1119,1127 /* * check our tuple count.. if we've processed the proper number !* then quit, else loop again and process more tuples. !* If numberTuples is zero, it means we have done MOVE 0 !* or FETCH ALL and we want to go to the end of the portal. */ current_tuple_count++; if (numberTuples == current_tuple_count) Index: src/backend/tcop/utility.c === RCS file: /cvsroot/pgsql-server/src/backend/tcop/utility.c,v retrieving revision 1.180 diff -c -c -r1.180 utility.c *** src/backend/tcop/utility.c 21 Oct 2002 20:31:52 - 1.180 --- src/backend/tcop/utility.c 26 Oct 2002 20:01:29 - *** *** 263,270 /* * parser ensures that count is >= 0 and 'fetch ALL' -> 0 */ - count = stmt->howMany; PerformPortalFetch(portalName, forward, count, (stmt->ismove) ? None : dest, --- 263,270 /* * parser ensures that count is >= 0 and 'fetch ALL' -> 0 +* MOVE 0 is equivalent to fetch ALL with no returned +tuples. */ count = stmt->howMany; PerformPortalFetch(portalName, forward, count, (stmt->ismove) ? None : dest, ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
[HACKERS] move 0 behaviour
Currently there is a TODO list item to have move 0 not position to the end of the cursor. Moving to the end of the cursor is useful, can we keep the behaviour and change it to move end, or just leave it the way it is? Dave ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
