From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Jan Urbański <wulczer(at)wulczer(dot)org>, Postgres - Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: review: psql: edit function, show function commands patch |
Date: | 2010-08-01 13:20:20 |
Message-ID: | AANLkTimyM2kehDeSs2duVa_bA_LfXfSSDspmsKXDf62_@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
2010/8/1 Robert Haas <robertmhaas(at)gmail(dot)com>:
> 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
>
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
>
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2010-08-01 14:47:57 | Re: review: psql: edit function, show function commands patch |
Previous Message | Robert Haas | 2010-08-01 12:54:44 | Re: ANALYZE versus expression indexes with nondefault opckeytype |