Re: 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 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

-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(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

>>>>> "Bruce" == Bruce Momjian <bruce(at)momjian(dot)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)

Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)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(at)sss(dot)pgh(dot)pa(dot)us> writes:

> Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)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.

--
Andrew.

On Fri, Mar 27, 2009 at 08:59:29PM +0000, Andrew Gierth wrote:
> >>>>> "Tom" == Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>
> > Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)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(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Thanks, patch applied.

---------------------------------------------------------------------------

Andrew Gierth wrote:
> >>>>> "Bruce" == Bruce Momjian <bruce(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +