Thread: Re: [HACKERS] Outstanding patches
[ Charset ISO-8859-15 unsupported, converting... ] > Bruce Momjian wrote: > > > > OK, now that we have started 7.2 development, I am going to go through > > the outstanding patches and start to apply them or reject them. They > > are at: > > > > http://candle.pha.pa.us/cgi-bin/pgpatches > > > > Has the patch that makes MOVE return number of rows actually moved > (analoguous > to UPDATE and DELETE) been properly submitted to patches ? I know MOVE had fixes in 7.1. I don't know of any outstanding MOVE bugs. -- 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:
>> Has the patch that makes MOVE return number of rows actually moved
>> (analoguous to UPDATE and DELETE) been properly submitted to patches ?
> I know MOVE had fixes in 7.1. I don't know of any outstanding MOVE
> bugs.
It wasn't a bug, it was a feature ;-)
Bruce did not have that patch on his list of things-to-apply, so either
it was never properly submitted or it slipped through the cracks.
Anyone want to dig it up and verify it against 7.1?
regards, tom lane
> + /* I use CMD_UPDATE, because no CMD_MOVE or the like
> + exists, and I would like to provide the same
> + kind of info as CMD_UPDATE */
> + UpdateCommandInfo(CMD_UPDATE, 0, -1*estate->es_processed);
I do not think it is a good idea to return a negative count for a
backwards move; that is too likely to break client code that parses
command result strings and isn't expecting minus signs. The client
should know whether he issued MOVE FORWARD or MOVE BACKWARDS anyway,
so just returning es_processed ought to be sufficient.
Otherwise I think the patch is probably OK.
regards, tom lane
Tom Lane wrote:
>
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> Has the patch that makes MOVE return number of rows actually moved
> >> (analoguous to UPDATE and DELETE) been properly submitted to patches ?
>
> > I know MOVE had fixes in 7.1. I don't know of any outstanding MOVE
> > bugs.
>
> It wasn't a bug, it was a feature ;-)
>
> Bruce did not have that patch on his list of things-to-apply, so either
> it was never properly submitted or it slipped through the cracks.
> Anyone want to dig it up and verify it against 7.1?
I forward it here so you don't have to dig it up:
-----------------------------------------------------------------------
Hi.
A few weeks (months?) ago I made a patch to the postgres
backend to get back the number of realized moves after
a MOVE command. So if I issue a "MOVE 100 IN cusrorname",
but there was only 66 rows left, I get back not only "MOVE",
but "MOVE 66". If the 100 steps could be realized, then
"MOVE 100" would come back.
I send this info to you, because I would like to ask you if
it could be OK to include in future versions. I think you
are in a beta testing phase now, so it is trivially not the
time to include it now...
The solution is 2 code lines into command.c, and then the
message of move cames with the number into for example psql.
1 other word into the jdbc driver, and this number is
available at update_count.
I made the patch to the latest (one day old) CVS version.
Here are the patches. Please look at them, and if you think
it's a good idea, then please let me know where and how should
I post them, and approximately when will you finish with the
beta testing, so it can be really considered seriously.
I included them also as an attachment, because my silly pine
insists to break the lines...
--- ./src/backend/commands/command.c.orig Fri Mar 23 05:49:52 2001
+++ ./src/backend/commands/command.c Sat Apr 7 10:24:27 2001
@@ -174,6 +174,12 @@
if (!portal->atEnd)
{
ExecutorRun(queryDesc, estate, EXEC_FOR, (long) count);
+
+ /* I use CMD_UPDATE, because no CMD_MOVE or the like
+ exists, and I would like to provide the same
+ kind of info as CMD_UPDATE */
+ UpdateCommandInfo(CMD_UPDATE, 0, estate->es_processed);
+
if (estate->es_processed > 0)
portal->atStart = false; /* OK to back up now */
if (count <= 0 || (int) estate->es_processed < count)
@@ -185,6 +191,12 @@
if (!portal->atStart)
{
ExecutorRun(queryDesc, estate, EXEC_BACK, (long) count);
+
+ /* I use CMD_UPDATE, because no CMD_MOVE or the like
+ exists, and I would like to provide the same
+ kind of info as CMD_UPDATE */
+ UpdateCommandInfo(CMD_UPDATE, 0, -1*estate->es_processed);
+
if (estate->es_processed > 0)
portal->atEnd = false; /* OK to go forward now */
if (count <= 0 || (int) estate->es_processed < count)
Here is the patch for the jdbc driver. >! I couldn't test it
with the current version, because it needs ant, and I didn't
have time and money today to download it... !< However, it
is a trivial change, and if Peter T. Mount reads it, I ask
him to check if he likes it... Thanks for any kind of answer.
--- ./src/interfaces/jdbc/org/postgresql/Connection.java.orig Wed Jan 31
09:26:01 2001
+++ ./src/interfaces/jdbc/org/postgresql/Connection.java Sat Apr 7
16:42:04 2001
@@ -490,7 +490,7 @@
recv_status =
pg_stream.ReceiveString(receive_sbuf,8192,getEncoding());
// Now handle the update count correctly.
- if(recv_status.startsWith("INSERT") ||
recv_status.startsWith("UPDATE") || recv_status.startsWith("DELETE")) {
+ if(recv_status.startsWith("INSERT") ||
recv_status.startsWith("UPDATE") || recv_status.startsWith("DELETE") ||
recv_status.startsWith("MOVE")) {
try {
update_count =
Integer.parseInt(recv_status.substring(1+recv_status.lastIndexOf(' ')));
} catch(NumberFormatException nfe) {
-------------------
(This last looks a bit complex, but the change is really a new
"|| recv_status.startsWith("MOVE")" only...)
Thank you for having read this,
Baldvin
p.s.: I read a page on your homepage, called "unapplied patches".
I would like to know if it means "still unapplied patches", or
it means "bad, and not accepted ideas".
Can this patch be resubmitted with a postive-only return value? > > + /* I use CMD_UPDATE, because no CMD_MOVE or the like > > + exists, and I would like to provide the same > > + kind of info as CMD_UPDATE */ > > + UpdateCommandInfo(CMD_UPDATE, 0, -1*estate->es_processed); > > I do not think it is a good idea to return a negative count for a > backwards move; that is too likely to break client code that parses > command result strings and isn't expecting minus signs. The client > should know whether he issued MOVE FORWARD or MOVE BACKWARDS anyway, > so just returning es_processed ought to be sufficient. > > Otherwise I think the patch is probably OK. > > regards, tom lane > > ---------------------------(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 use CMD_UPDATE, because no CMD_MOVE or the like > > + exists, and I would like to provide the same > > + kind of info as CMD_UPDATE */ > > + UpdateCommandInfo(CMD_UPDATE, 0, -1*estate->es_processed); > > I do not think it is a good idea to return a negative count for a > backwards move; that is too likely to break client code that parses > command result strings and isn't expecting minus signs. The client > should know whether he issued MOVE FORWARD or MOVE BACKWARDS anyway, > so just returning es_processed ought to be sufficient. > > Otherwise I think the patch is probably OK. I have applied this patch with does MOVE output for both the backend and jdbc. I tested the JDBC patch by compiling, and changed the backend to only output postitive values. Thanks. -- 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/backend/commands/command.c =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/commands/command.c,v retrieving revision 1.131 diff -c -r1.131 command.c *** src/backend/commands/command.c 2001/05/30 13:00:03 1.131 --- src/backend/commands/command.c 2001/06/07 00:03:44 *************** *** 176,181 **** --- 176,187 ---- if (!portal->atEnd) { ExecutorRun(queryDesc, estate, EXEC_FOR, (long) count); + /* + * I use CMD_UPDATE, because no CMD_MOVE or the like + * exists, and I would like to provide the same + * kind of info as CMD_UPDATE + */ + UpdateCommandInfo(CMD_UPDATE, 0, estate->es_processed); if (estate->es_processed > 0) portal->atStart = false; /* OK to back up now */ if (count <= 0 || (int) estate->es_processed < count) *************** *** 187,192 **** --- 193,204 ---- if (!portal->atStart) { ExecutorRun(queryDesc, estate, EXEC_BACK, (long) count); + /* + * I use CMD_UPDATE, because no CMD_MOVE or the like + * exists, and I would like to provide the same + * kind of info as CMD_UPDATE + */ + UpdateCommandInfo(CMD_UPDATE, 0, estate->es_processed); if (estate->es_processed > 0) portal->atEnd = false; /* OK to go forward now */ if (count <= 0 || (int) estate->es_processed < count) Index: src/interfaces/jdbc/org/postgresql/Connection.java =================================================================== RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/Connection.java,v retrieving revision 1.16 diff -c -r1.16 Connection.java *** src/interfaces/jdbc/org/postgresql/Connection.java 2001/06/01 20:57:58 1.16 --- src/interfaces/jdbc/org/postgresql/Connection.java 2001/06/07 00:03:56 *************** *** 505,511 **** recv_status = pg_stream.ReceiveString(receive_sbuf,8192,getEncoding()); // Now handle the update count correctly. ! if(recv_status.startsWith("INSERT") || recv_status.startsWith("UPDATE") || recv_status.startsWith("DELETE")){ try { update_count = Integer.parseInt(recv_status.substring(1+recv_status.lastIndexOf(' '))); } catch(NumberFormatException nfe) { --- 505,511 ---- recv_status = pg_stream.ReceiveString(receive_sbuf,8192,getEncoding()); // Now handle the update count correctly. ! if(recv_status.startsWith("INSERT") || recv_status.startsWith("UPDATE") || recv_status.startsWith("DELETE")|| recv_status.startsWith("MOVE")) { try { update_count = Integer.parseInt(recv_status.substring(1+recv_status.lastIndexOf(' '))); } catch(NumberFormatException nfe) {