plpgsql + named parameters

I followed the past discussions regarding the syntax for named
parameters and I am currently using Pavel Stehule's patch for named
and mixed notation on top of the 8.4 beta.

It seems the way plpgsql substitutes $1, $2, etc for the parameters is
going to reduce the usefulness of this feature. Consider these two
functions:

CREATE FUNCTION fun1(a INT DEFAULT 1) RETURNS INT AS 'SELECT $1'
LANGUAGE SQL;
CREATE FUNCTION fun2(a INT) RETURNS INT AS $$
DECLARE
t INT;
BEGIN
t := fun1(1 as a); -- syntax error: "SELECT fun1(1 as $1 )"
t := fun1(a as a); -- syntax error: "SELECT fun1( $1 as $1 )"
RETURN 0;
END;
$$ LANGUAGE plpgsql;

I would think this would be a very common scenario where one function
calls another similar function that has similar parameter names.

Am I missing something or are there any obvious solutions to this?

Pavel's patch:
http://archives.postgresql.org/message-id/162867790903042341o477b115dtb6b351dd8ff758cc@mail.gmail.com

Thanks,
-Steve

On Tue, May 19, 2009 at 5:59 PM, Steve Prentice <prentice(at)cisco(dot)com> wrote:
> I followed the past discussions regarding the syntax for named parameters
> and I am currently using Pavel Stehule's patch for named and mixed notation
> on top of the 8.4 beta.
>
> It seems the way plpgsql substitutes $1, $2, etc for the parameters is going
> to reduce the usefulness of this feature. Consider these two functions:
>
> CREATE FUNCTION fun1(a INT DEFAULT 1) RETURNS INT AS 'SELECT $1' LANGUAGE
> SQL;
> CREATE FUNCTION fun2(a INT) RETURNS INT AS $$
> DECLARE
>        t INT;
> BEGIN
>        t := fun1(1 as a);      -- syntax error: "SELECT  fun1(1 as  $1 )"
>        t := fun1(a as a);      -- syntax error: "SELECT  fun1( $1  as  $1 )"

you have a name conflict here...is it deliberate? I've learned the
hard way to always, always prefix arguments and locals to plpgsql
functions with '_'. Or are you trying to do something fancier?

merlin

>> t := fun1(1 as a); -- syntax error: "SELECT fun1(1 as
>> $1 )"
>> t := fun1(a as a); -- syntax error: "SELECT fun1( $1
>> as $1 )"

On May 19, 2009, at 6:42 PM, Merlin Moncure wrote:
> you have a name conflict here...is it deliberate? I've learned the
> hard way to always, always prefix arguments and locals to plpgsql
> functions with '_'. Or are you trying to do something fancier?

The conflict is deliberate to illustrate the limitations the named
parameter feature (on the list for the first 8.5 CommitFest) is going
to have if parameter substitution is not addressed at the same time.

-Steve

2009/5/20 Steve Prentice <prentice(at)cisco(dot)com>:
>        t := fun1(1 as a);      -- syntax error: "SELECT  fun1(1 as  $1 )"
>
>        t := fun1(a as a);      -- syntax error: "SELECT  fun1( $1  as  $1 )"
>
> On May 19, 2009, at 6:42 PM, Merlin Moncure wrote:
>
> you have a name conflict here...is it deliberate? I've learned the
> hard way to always, always prefix arguments and locals to plpgsql
> functions with '_'.  Or are you trying to do something fancier?
>
> The conflict is deliberate to illustrate the limitations the named parameter
> feature (on the list for the first 8.5 CommitFest) is going to have if
> parameter substitution is not addressed at the same time.
> -Steve

this problem is little bit deeper and is related to plpgsql method for
SQL query processing.

I thing so there are two solutions:

a) use dynamic SQL
b) use double quotes for identifier - identifiers have to be lower

t := fun1(a as "a");

regards
Pavel Stehule

On May 20, 2009, at 10:24 AM, Pavel Stehule wrote:
> this problem is little bit deeper and is related to plpgsql method for
> SQL query processing.
>
> I thing so there are two solutions:
>
> a) use dynamic SQL
> b) use double quotes for identifier - identifiers have to be lower
>
> t := fun1(a as "a");

plpgsql substitutes an expression parameter for the double-quoted
identifier as well and I'm less than thrilled about using dynamic SQL
to make all my function calls. I was hoping we could modify the
grammar so that identifiers after the AS keyword are passed through.

Something like this patch:

diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
index 06704cf..66d12d8 100644
--- a/src/pl/plpgsql/src/gram.y
+++ b/src/pl/plpgsql/src/gram.y
@@ -177,6 +177,7 @@ static List *read_raise_options(void);
* Keyword tokens
*/
%token K_ALIAS
+%token K_AS
%token K_ASSIGN
%token K_BEGIN
%token K_BY
@@ -1977,6 +1978,7 @@ read_sql_construct(int until,
int *endtoken)
{
int tok;
+ int prevtok = 0;
int lno;
PLpgSQL_dstring ds;
int parenlevel = 0;
@@ -1989,7 +1991,7 @@ read_sql_construct(int until,
plpgsql_dstring_init(&ds);
plpgsql_dstring_append(&ds, sqlstart);

- for (;;)
+ for (;;prevtok = tok)
{
tok = yylex();
if (tok == until && parenlevel == 0)
@@ -2034,10 +2036,22 @@ read_sql_construct(int until,
switch (tok)
{
case T_SCALAR:
- snprintf(buf, sizeof(buf), " $%d ",
- assign_expr_param(yylval.scalar->dno,
- params, &nparams));
- plpgsql_dstring_append(&ds, buf);
+ /*
+ * If the previous token is AS, then we pass the scalar
+ * through as a label. Otherwise, make the scalar an
+ * expression parameter.
+ */
+ if (prevtok == K_AS)
+ {
+ plpgsql_dstring_append(&ds, yytext);
+ }
+ else
+ {
+ snprintf(buf, sizeof(buf), " $%d ",
+ assign_expr_param(yylval.scalar->dno,
+ params, &nparams));
+ plpgsql_dstring_append(&ds, buf);
+ }
break;

case T_ROW:
diff --git a/src/pl/plpgsql/src/scan.l b/src/pl/plpgsql/src/scan.l
index 1917eef..e3a5c45 100644
--- a/src/pl/plpgsql/src/scan.l
+++ b/src/pl/plpgsql/src/scan.l
@@ -149,6 +149,7 @@ param \${digit}+
= { return K_ASSIGN; }
\.\. { return K_DOTDOT; }
alias { return K_ALIAS; }
+as { return K_AS; }
begin { return K_BEGIN; }
by { return K_BY; }
case { return K_CASE; }

2009/5/21 Steve Prentice <prentice(at)cisco(dot)com>:
> On May 20, 2009, at 10:24 AM, Pavel Stehule wrote:
>>
>> this problem is little bit deeper and is related to plpgsql method for
>> SQL query processing.
>>
>> I thing so there are two solutions:
>>
>> a) use dynamic SQL
>> b) use double quotes for identifier - identifiers have to be lower
>>
>> t := fun1(a as "a");
>
> plpgsql substitutes an expression parameter for the double-quoted identifier
> as well and I'm less than thrilled about using dynamic SQL to make all my
> function calls. I was hoping we could modify the grammar so that identifiers
> after the AS keyword are passed through.
>
> Something like this patch:
>
> diff --git a/src/pl/plpgsql/src/gram.y b/src/pl/plpgsql/src/gram.y
> index 06704cf..66d12d8 100644
> --- a/src/pl/plpgsql/src/gram.y
> +++ b/src/pl/plpgsql/src/gram.y
> @@ -177,6 +177,7 @@ static List
> *read_raise_options(void);
>                 * Keyword tokens
>                 */
> %token  K_ALIAS
> +%token K_AS
> %token  K_ASSIGN
> %token  K_BEGIN
> %token  K_BY
> @@ -1977,6 +1978,7 @@ read_sql_construct(int until,
>                                   int *endtoken)
> {
>        int                                     tok;
> +       int                                     prevtok = 0;
>        int                                     lno;
>        PLpgSQL_dstring         ds;
>        int                                     parenlevel = 0;
> @@ -1989,7 +1991,7 @@ read_sql_construct(int until,
>        plpgsql_dstring_init(&ds);
>        plpgsql_dstring_append(&ds, sqlstart);
>
> -       for (;;)
> +       for (;;prevtok = tok)
>        {
>                tok = yylex();
>                if (tok == until && parenlevel == 0)
> @@ -2034,10 +2036,22 @@ read_sql_construct(int until,
>                switch (tok)
>                {
>                        case T_SCALAR:
> -                               snprintf(buf, sizeof(buf), " $%d ",
> -
>  assign_expr_param(yylval.scalar->dno,
> -
>        params, &nparams));
> -                               plpgsql_dstring_append(&ds, buf);
> +                               /*
> +                                * If the previous token is AS, then we pass
> the scalar
> +                                * through as a label. Otherwise, make the
> scalar an
> +                                * expression parameter.
> +                                */
> +                               if (prevtok == K_AS)
> +                               {
> +                                       plpgsql_dstring_append(&ds, yytext);
> +                               }
> +                               else
> +                               {
> +                                       snprintf(buf, sizeof(buf), " $%d ",
> +
>  assign_expr_param(yylval.scalar->dno,
> +
>                params, &nparams));
> +                                       plpgsql_dstring_append(&ds, buf);
> +                               }
>                                break;
>
>                        case T_ROW:
> diff --git a/src/pl/plpgsql/src/scan.l b/src/pl/plpgsql/src/scan.l
> index 1917eef..e3a5c45 100644
> --- a/src/pl/plpgsql/src/scan.l
> +++ b/src/pl/plpgsql/src/scan.l
> @@ -149,6 +149,7 @@ param                       \${digit}+
> =                               { return K_ASSIGN;                      }
> \.\.                    { return K_DOTDOT;                      }
> alias                   { return K_ALIAS;                       }
> +as                             { return K_AS;                          }
> begin                   { return K_BEGIN;                       }
> by                              { return K_BY;                          }
> case                    { return K_CASE;                        }
>

+1

please append your patch to commitfest page

Pavel