Обсуждение: Any reason not to return row_count in cursor of plpgsql?
hi all, I read the code that it seems easy for the cursor in plpgsql to return ROW_COUNT afterMOVE LAST etc. The SPI_processed variable already there, but didn't put it into estatestructure, any reason for that? thanks and best regards -laser
laser wrote: > hi all, > > I read the code that it seems easy for the cursor in plpgsql to return > ROW_COUNT after > MOVE LAST etc. The SPI_processed variable already there, but didn't put > it into estate > structure, any reason for that? [ Sorry for the delay.] Would some tests how Oracle behaves in this case? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
laser wrote: > hi all, > > I read the code that it seems easy for the cursor in plpgsql to return > ROW_COUNT after > MOVE LAST etc. The SPI_processed variable already there, but didn't put > it into estate > structure, any reason for that? > > thanks and best regards Sorry, we have decided against this change because it might break existing applications. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
>>>>> "Bruce" == Bruce Momjian <bruce@momjian.us> writes: >> hi all,>> >> I read the code that it seems easy for the cursor in plpgsql to>> return ROW_COUNT after MOVE LAST etc. TheSPI_processed variable>> already there, but didn't put it into estate structure, any reason>> for that?>> >> thanks andbest regards Bruce> Sorry, we have decided against this change because it mightBruce> break existing applications. As they say on wikipedia, [citation needed] GET DIAGNOSTICS ROW_COUNT is documented as working for all commands; if it doesn't work for MOVE (and FETCH), that's a bug. It might be one that's not appropriate to backpatch, but that's no excuse for not fixing it in a new release. It's especially egregious in that MOVE _does_ set FOUND. diff -c -r1.235 pl_exec.c *** pl_exec.c 23 Feb 2009 10:03:22 -0000 1.235 --- pl_exec.c 27 Mar 2009 10:44:08 -0000 *************** *** 3368,3373 **** --- 3368,3375 ---- exec_set_found(estate, n != 0); } + estate->eval_processed = n; + return PLPGSQL_RC_OK; } -- Andrew (irc:RhodiumToad)
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> GET DIAGNOSTICS ROW_COUNT is documented as working for all commands;
> if it doesn't work for MOVE (and FETCH), that's a bug.
Or a documentation problem. I don't see any claim that it works for
"all commands" anyway.
regards, tom lane
>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: > Andrew Gierth <andrew@tao11.riddles.org.uk> writes:>> GET DIAGNOSTICS ROW_COUNT is documented as working for all commands;>>if it doesn't work for MOVE (and FETCH), that's a bug. Tom> Or a documentation problem. I don't see any claim that it works forTom> "all commands" anyway. "This command allows retrieval of system status indicators. Each item is a key word identifying a state value to be assigned to the specified variable (which should be of the right data type to receive it). The currently available status items are ROW_COUNT, the number of rows processed by the last SQL command sent down to the SQL engine, and RESULT_OID, the OID of the last row inserted by the most recent SQL command. Note that RESULT_OID is only useful after an INSERT command into a table containing OIDs." The idea that fetch/move should _intentionally_ not set ROW_COUNT is beyond ludicrous. -- Andrew.
On Fri, Mar 27, 2009 at 08:59:29PM +0000, Andrew Gierth wrote: > >>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes: > > > Andrew Gierth <andrew@tao11.riddles.org.uk> writes: > >> GET DIAGNOSTICS ROW_COUNT is documented as working for all commands; > >> if it doesn't work for MOVE (and FETCH), that's a bug. > > Tom> Or a documentation problem. I don't see any claim that it works for > Tom> "all commands" anyway. > > "This command allows retrieval of system status indicators. Each item > is a key word identifying a state value to be assigned to the > specified variable (which should be of the right data type to receive > it). The currently available status items are ROW_COUNT, the number of > rows processed by the last SQL command sent down to the SQL engine, > and RESULT_OID, the OID of the last row inserted by the most recent > SQL command. Note that RESULT_OID is only useful after an INSERT > command into a table containing OIDs." > > The idea that fetch/move should _intentionally_ not set ROW_COUNT is > beyond ludicrous. It's a flat-out bug not to have FETCH/MOVE set this. Cheers, David. -- David Fetter <david@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Thanks, patch applied. --------------------------------------------------------------------------- Andrew Gierth wrote: > >>>>> "Bruce" == Bruce Momjian <bruce@momjian.us> writes: > > >> hi all, > >> > >> I read the code that it seems easy for the cursor in plpgsql to > >> return ROW_COUNT after MOVE LAST etc. The SPI_processed variable > >> already there, but didn't put it into estate structure, any reason > >> for that? > >> > >> thanks and best regards > > Bruce> Sorry, we have decided against this change because it might > Bruce> break existing applications. > > As they say on wikipedia, [citation needed] > > GET DIAGNOSTICS ROW_COUNT is documented as working for all commands; > if it doesn't work for MOVE (and FETCH), that's a bug. It might be one > that's not appropriate to backpatch, but that's no excuse for not > fixing it in a new release. > > It's especially egregious in that MOVE _does_ set FOUND. > > diff -c -r1.235 pl_exec.c > *** pl_exec.c 23 Feb 2009 10:03:22 -0000 1.235 > --- pl_exec.c 27 Mar 2009 10:44:08 -0000 > *************** > *** 3368,3373 **** > --- 3368,3375 ---- > exec_set_found(estate, n != 0); > } > > + estate->eval_processed = n; > + > return PLPGSQL_RC_OK; > } > > -- > Andrew (irc:RhodiumToad) -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +