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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: prokind column (was Re: [HACKERS] SQL procedures)
Date: 2018-02-28 20:45:40
Message-ID: 1636.1519850740@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
v3-0001-add-prokind-column.patch text/x-diff 103.1 KB
v3-0002-data-line-updates.patch.gz application/x-gzip 80.4 KB
fix_proc_data.pl text/x-perl 583 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2018-02-28 20:45:47 SET TRANSACTION in PL/pgSQL
Previous Message David Steele 2018-02-28 20:45:00 2018-03 Commitfest starts tomorrow