Re: prokind column (was Re: [HACKERS] SQL procedures)

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> Here is some input from me. ...

I have reviewed this patch and attach an updated version below.
I've rebased it up to today, fixed a few minor errors, and adopted
most of Michael's suggestions. Also, since I remain desperately
unhappy with putting zeroes into prorettype, I changed it to not
do that ;-) ... now procedures have VOIDOID as their prorettype,
and it will be substantially less painful to allow them to return
some other scalar result in future, should we wish to. I believe
I've found all the places that were relying on prorettype == 0 as
a substitute for prokind == 'p'.

I have not touched the tab-complete.c changes. It doesn't really make
sense to worry about that until we see what comes out of the other thread
discussing support for server-version-sensitive tab completion. In the
meantime let's just add an open-item entry reminding us to do something
about it before v11 freezes.

Rather than manually audit the 0002 patch, I made a brain-dead little
Perl script to convert the DATA lines automatically, and attach it
below. If pg_proc.h moves further before this gets committed, the
script could be used to generate an updated 0002 patch.

One point worth noting is that in the attached, I made some edits to
pl_gram.y to preserve the existing handling of RETURN-with-a-value
in a plpgsql procedure. However, that existing handling is pretty
stupid. If we simply drop the pl_gram.y diffs below, what happens
is that RETURN-with-a-value is complained of at compile time not
execution time, which seems far superior to me (and more like what
happens in a normal function returning VOID). I think we should
do that and adjust the regression tests accordingly, but I have not
done the latter here.

I also wonder whether we should drop the stanza at the start of
pg_get_function_result() that causes it to return NULL for a procedure.
Without that, it would now return "void" which I think is at least
as defensible, and certainly more upward-compatible with any future
extension in that area. (I did make some changes in the
information_schema that cause it to report "void" not NULL in the
same case.)

Other than those points, I think this is committable, and I think we
should get it in quickly.

regards, tom lane