Any reason not to return row_count in cursor of plpgsql?

Started by laserover 17 years ago8 messages
#1laser
laserlist@pgsqldb.com

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

#2Bruce Momjian
bruce@momjian.us
In reply to: laser (#1)
Re: Any reason not to return row_count in cursor of plpgsql?

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. +

#3Bruce Momjian
bruce@momjian.us
In reply to: laser (#1)
Re: Any reason not to return row_count in cursor of plpgsql?

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. +

#4Andrew Gierth
andrew@tao11.riddles.org.uk
In reply to: Bruce Momjian (#3)
Re: Any reason not to return row_count in cursor of plpgsql?

"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)

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Gierth (#4)
Re: Any reason not to return row_count in cursor of plpgsql?

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

#6Andrew Gierth
andrew@tao11.riddles.org.uk
In reply to: Tom Lane (#5)
Re: Any reason not to return row_count in cursor of plpgsql?

"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.

--
Andrew.

#7David Fetter
david@fetter.org
In reply to: Andrew Gierth (#6)
Re: Any reason not to return row_count in cursor of plpgsql?

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

#8Bruce Momjian
bruce@momjian.us
In reply to: Andrew Gierth (#4)
Re: Any reason not to return row_count in cursor of plpgsql?

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. +