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

2010/8/1 Robert Haas <robertmhaas@gmail.com>:
> On Sun, Aug 1, 2010 at 12:15 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> 2010/8/1 Robert Haas <robertmhaas@gmail.com>:
>>> On Sun, Jul 25, 2010 at 11:42 AM, Pavel Stehule <pavel.stehule@gmail.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
>

it not is too much important, navigation command is secondary
function. Primary functionality is showing of source code with correct
row numbers.

> 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.

.. :( without this feature - this patch has minimal value.

I don't believe so change plpgsql numbering is good way. It was
changed from good reasons. Because empty row is invisible but it isn't
empty for people, because there are " AS " keyword.

This patch must to working with plpgsql and have to work correctly.

>
>>> 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().

you have a true - it is possible, but still I am supply a parser on
client side. On server side I have all data in structured form.

but I don't would to complicate a possible commiting

so my plan

a) fix problem with ambiguous $function* like you proposed
b) fix problem with "first row excepting" - I can activate a detection
only for plpgsql language - I can identify LANGUAGE before.

all will be done on client

It is acceptable for you?

Regards

Pavel Stehule


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