On Mon, Feb 12, 2018 at 03:19:27PM -0500, Tom Lane wrote:
> I've not read in detail, but it looks reasonable offhand, modulo
> that I still don't like prorettype == 0 ;-).
> 
> I did notice a tiny typo:
> 
> -      * with.  Hence prefer "$function$", but extend if needed.
> +      * with.  Hence prefer "$function$"/"$procedure", but extend if needed.
> 
> I think you want
> 
> +      * with.  Hence prefer "$function$"/"$procedure$", but extend if needed.

0001 and 0002 are welcome.  I have a small comment on top of Tom's for 0003.

+   appendStringInfoString(&buf, ")\n");
+   if (proc->prorettype)
+   {
+       appendStringInfoString(&buf, " RETURNS ");
+       print_function_rettype(&buf, proctup);
+       appendStringInfoChar(&buf, '\n');
+   }
Could you use a separate boolean variable which is set as
OidIsValid(prorettype), say called isfunction?  The goal is to avoid the
check on prorettype in more than one place.  If pg_proc's shape is
changed depending on the discussion, the current patch is a recipy to
forget updating all those places.  A comment in pg_get_function_result
to mention that prorettype = InvalidOid is here to track that the call
involves a procedure would be nice.

Should the documentation of pg_function_is_visible also mention
procedures?
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to