Hi,
Trying the v7a patch, here are a few comments:
* SIGSEGV with ON HOLD cursors.
Reproducer:
declare c cursor with hold for select oid,relname
from pg_class order by 1 limit 10;
select * from rows_in('c') as x(f1 oid,f2 name);
consumes a bit of time, then crashes and generates a 13 GB core file
without a usable stacktrace:
Core was generated by `postgres: daniel postgres [local] SELECT '.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00007f4c5b2f3dc9 in ?? ()
(gdb) bt
#0 0x00007f4c5b2f3dc9 in ?? ()
#1 0x0000564567efc505 in ?? ()
#2 0x0000000000000001 in ?? ()
#3 0x000056456a4b28f8 in ?? ()
#4 0x000056456a4b2908 in ?? ()
#5 0x000056456a4b2774 in ?? ()
#6 0x000056456a4ad218 in ?? ()
#7 0x000056456a4b1590 in ?? ()
#8 0x0000000000000010 in ?? ()
#9 0x0000000000000000 in ?? ()
* rows_in() does not fetch from the current position of the cursor,
but from the start. For instance, I would expect that if doing
FETCH FROM cursor followed by SELECT * FROM rows_in('cursor'), the first
row would be ignored by rows_in(). That seems more convenient and more
principled.
*
+ <para>
+ This section describes functions that cursors to be manipulated
+ in normal <command>SELECT</command> queries.
+ </para>
A verb seems to be missing.
It should be "function that *allow* cursors to be..." or something
like that?
*
+ The <type>REFCURSOR</type> must be open, and the query must be a
+ <command>SELECT</command> statement. If the <type>REFCURSOR</type>’s
+ output does not
After </type> there is a fancy quote (codepoint U+2019). There is
currently no codepoint outside of US-ASCII in *.sgml ref/*.sgml, so
they're probably not welcome.
* Also: does the community wants it as a built-in function in core?
As mentioned in a previous round of review, a function like this in
plpgsql comes close:
create function rows_in(x refcursor) returns setof record as $$
declare
r record;
begin
loop
fetch x into r;
exit when not found;
return next r;
end loop;
end $$ language plpgsql;
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: https://www.manitou-mail.org
Twitter: @DanielVerite
