Re: pl/pgsql feature request: shorthand for argument and local variable references

st 17. 3. 2021 v 23:16 odesílatel Hannu Krosing <hannuk(at)google(dot)com> napsal:

> why are you using yet another special syntax for this ?
>
> would it not be better to do something like this:
> CREATE FUNCTION a_reall_long_and_winding_function_name(i int, out o int)
> LANGUAGE plpgsql AS $plpgsql$
> DECLARE
> args function_name_alias
> BEGIN
> args.o = 2 * args.i
> END;
> $plpgsql$;
>
> or at least do something based on block labels (see end of
> https://www.postgresql.org/docs/13/plpgsql-structure.html )
>
> CREATE FUNCTION somefunc() RETURNS integer AS $$
> << outerblock >>
> DECLARE
> ...
>
> maybe extend this to
>
> CREATE FUNCTION somefunc() RETURNS integer AS $$
> << functionlabel:args >>
> << outerblock >>
> DECLARE
> ...
>
> or replace 'functionlabel' with something less ugly :)
>
> maybe <<< args >>>
>

There are few main reasons:

a) compile options are available already - so I don't need invent new
syntax - #OPTION DUMP, #PRINT_STRICT ON, #VARIABLE_CONFLICT ERROR

b) compile options are processed before any any other operations, so
implementation can be very simple

c) implementation is very simple

I don't like an idea - <<prefix:value>> because you mix label syntax with
some functionality, and <<x>> and <<<x>>> are visually similar

"#routine_label xxx" is not maybe visually nice (like other compile options
in plpgsql), but has good readability, simplicity, verbosity

Because we already have this functionality (compile options), and because
this functionality is working well (there are not any limits from this
syntax), I strongly prefer to use it. We should not invite new syntax when
some already available syntax can work well. We can talk about the keyword
ROUTINE_LABEL - maybe #OPTION ROUTINE_LABEL xxx can look better, but I
think so using the compile option is a good solution.

Regards

Pavel

> Cheers
> Hannu
>
>
>
> On Wed, Mar 17, 2021 at 5:05 PM Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >
> >
> >
> > st 17. 3. 2021 v 9:20 odesílatel Michael Paquier <michael(at)paquier(dot)xyz>
> napsal:
> >>
> >> On Wed, Mar 17, 2021 at 02:06:57PM +0800, Julien Rouhaud wrote:
> >> > I also think that there should be a single usable top label,
> otherwise it will
> >> > lead to confusing code and it can be a source of bug.
> >>
> >> Okay, that's fine by me. Could it be possible to come up with an
> >> approach that does not hijack the namespace list contruction and the
> >> lookup logic as much as it does now? I get it that the patch is done
> >> this way because of the ordering of operations done for the initial ns
> >> list creation and the grammar parsing that adds the routine label on
> >> top of the root one, but I'd like to believe that there are more solid
> >> ways to achieve that, like for example something that decouples the
> >> root label and its alias, or something that associates an alias
> >> directly with its PLpgSQL_nsitem entry?
> >
> >
> > I am checking it now, and I don't see any easy solution. The namespace
> is a one direction tree - only backward iteration from leafs to roots is
> supported. At the moment, when I try to replace the name of root ns_item,
> this root ns_item is referenced by the function's arguments (NS_TYPE_VAR
> type). So anytime I need some helper ns_item node, that can be a
> persistent target for these nodes. In this case is a memory overhead of
> just one ns_item.
> >
> > orig_label <- argument1 <- argument2
> >
> > The patch has to save the original orig_label because it can be
> referenced from argument1 or by "FOUND" variable. New graphs looks like
> >
> > new_label <- invalidated orig_label <- argument1 <- ...
> >
> > This tree has a different direction than is usual, and then replacing
> the root node is not simple.
> >
> > Second solution (significantly more simple) is an additional pointer in
> ns_item structure. In almost all cases this pointer will not be used.
> Because ns_item uses a flexible array, then union cannot be used. I
> implemented this in a patch marked as "alias-implementation".
> >
> > What do you think about it?
> >
> > Pavel
> >
> >
> >
> >
> >
> >
> >>
> >> --
> >> Michael
>