Re: review: psql: edit function, show function commands patch

On Sun, Aug 1, 2010 at 12:15 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> 2010/8/1 Robert Haas <robertmhaas(at)gmail(dot)com>:
>> On Sun, Jul 25, 2010 at 11:42 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>>>> I'm setting this as ready for committer.
>>>
>>> Thank you very much
>>
>> I took a look at this tonight and am a bit mystified by the following bit:
>>
>> +                       /*
>> +                        * PL doesn't calculate first row of function's body
>> +                        * when first row is empty. So checks first row, and
>> +                        * correct lineno when it is necessary.
>> +                        */
>>
>> Is that true of any PL, or just some particular PL?  Is the behavior
>> described here a bug that we should fix, or is it, for some reason,
>> considered correct?  Is it documented in our documentation?
>
> it is primary plpgsql issue.

Yeah, that seems like a problem. If your editor is vi, for example,
the following are the same number of keystrokes:

\ef foo 417<enter>
and
\ef foo<enter>417G

So the major advantage of the former over the latter is that you'll
(hopefully) end up on EXACTLY the right line rather than maybe being
off by a few lines. With the current code, that won't necessarily
happen, because PL/pgsql (and any third-party PLs based on it) use one
line-numbering convention and other PLs don't necessarily include that
hack. Admittedly, you'll probably only be off by one line instead of
three or four, so maybe people think that's good enough, but I'm not
totally convinced. It seems like the easiest way to fix this would be
remove the hack from PL/pgsql, but I'm not sure how people feel about
that. If we don't do that, I'm not sure there's any real good
solution here.

>> The implementation of first_row_is_empty() looks pretty kludgey, too.
>> It seems to me that it will fail completely if the text of the
>> function definition happens to contain $function$.
>>
>> CREATE OR REPLACE FUNCTION boom() RETURNS text LANGUAGE plpgsql AS $$
>> BEGIN SELECT '$function$'; END $$;
>>
>
> I can enhance algorithm on client side - but it will not be a pretty
> code - it better do it on server side - for example
> pg_get_formated_functiondef ...
>
> CREATE OR REPLACE FUNCTION pg_get_formated_function_def(in oid,
> linenum bool:= false, OUT funcdef text, OUT first_line_isignored bool)
>
> this can remove any ugly code

I don't really see why this can't be done on the client side - can't
you just scan until you find the second dollars sign and then see
whether the following character is a newline? Seems like this could
be done in a very small number of lines of code using strchr().

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company