Re: [HACKERS] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

[Pavel Stehule]

2017-09-11 22:28 GMT+02:00 Tom Lane :

> Jeevan Chalke  writes:
> [ psql-named-arguments-03-jeevan.patch ]
>
> Pushed with minor simplification of the test case.
>
> I'm not quite as convinced as Pavel that this is an improvement ---
> it will make error messages inconsistent between named and unnamed
> arguments.  Still, I follow the point that when there are a lot of
> arguments, $n is pretty unhelpful.  We can always revert this if
> we get complaints.
>

Thank you very much

Regards

Pavel


>
> regards, tom lane
>

Re: [HACKERS] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

[Tom Lane]

Jeevan Chalke  writes:
[ psql-named-arguments-03-jeevan.patch ]

Pushed with minor simplification of the test case.

I'm not quite as convinced as Pavel that this is an improvement ---
it will make error messages inconsistent between named and unnamed
arguments.  Still, I follow the point that when there are a lot of
arguments, $n is pretty unhelpful.  We can always revert this if
we get complaints.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

[Pavel Stehule]

2017-09-11 9:46 GMT+02:00 Jeevan Chalke :

> Hi Pavel,
>
>
> On Sat, Sep 9, 2017 at 11:42 AM, Pavel Stehule 
> wrote:
>
>> Hi
>>
>> 2017-09-08 9:36 GMT+02:00 Jeevan Chalke :
>>
>>> Hi Pavel,
>>> I like the idea of using parameter name instead of $n symbols.
>>>
>>> However, I am slightly worried that, at execution time if we want to
>>> know the parameter position in the actual function signature, then it
>>> will become difficult to get that from the corresponding datum
>>> variable. I don't have any use-case for that though. But apart from
>>> this concern, idea looks good to me.
>>>
>>
>> Understand - but it was reason why I implemented this function - when I
>> have to search parameter name via offset, I cannot to use string searching.
>> When you know the parameter name, you can use a string searching in text
>> editor, in pager.
>>
>> It is better supported now, then current behave.
>>
>
> Make sense.
>
>
>>
>>
>>>
>>> BTW, instead of doing all these changes, I have done these changes this
>>> way:
>>>
>>> -   /* Build variable and add to datum list */
>>> -   argvariable = plpgsql_build_variable(buf, 0,
>>> -argdtype, false);
>>> +   /*
>>> +* Build variable and add to datum list.  If there's a
>>> name for
>>> +* the argument, then use that else use $n name.
>>> +*/
>>> +   argvariable = plpgsql_build_variable((argnames &&
>>> argnames[i][0] != '\0') ?
>>> +argnames[i] : buf,
>>> +0, argdtype,
>>> false);
>>>
>>> This requires no new variable and thus no more changes elsewhere.
>>>
>>> Attached patch with these changes. Please have a look.
>>>
>>
>> Looks great - I added check to NULL only
>>
>
> Looks good.
> I have not made those changes in my earlier patch as I did not want to
> update other code which is not touched by this patch.
>
> Anyways, your changes related to NULL check seems reasonable.
> However, in attached patch I have fixed indentation.
>
> Passing it on to the committer.
>

Thank you very much

Regards

Pavel


>
> Thanks
> --
> Jeevan Chalke
> Principal Software Engineer, Product Development
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
>
>

Re: [HACKERS] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

[Jeevan Chalke]

Hi Pavel,


On Sat, Sep 9, 2017 at 11:42 AM, Pavel Stehule 
wrote:

> Hi
>
> 2017-09-08 9:36 GMT+02:00 Jeevan Chalke :
>
>> Hi Pavel,
>> I like the idea of using parameter name instead of $n symbols.
>>
>> However, I am slightly worried that, at execution time if we want to
>> know the parameter position in the actual function signature, then it
>> will become difficult to get that from the corresponding datum
>> variable. I don't have any use-case for that though. But apart from
>> this concern, idea looks good to me.
>>
>
> Understand - but it was reason why I implemented this function - when I
> have to search parameter name via offset, I cannot to use string searching.
> When you know the parameter name, you can use a string searching in text
> editor, in pager.
>
> It is better supported now, then current behave.
>

Make sense.


>
>
>>
>> BTW, instead of doing all these changes, I have done these changes this
>> way:
>>
>> -   /* Build variable and add to datum list */
>> -   argvariable = plpgsql_build_variable(buf, 0,
>> -argdtype, false);
>> +   /*
>> +* Build variable and add to datum list.  If there's a
>> name for
>> +* the argument, then use that else use $n name.
>> +*/
>> +   argvariable = plpgsql_build_variable((argnames &&
>> argnames[i][0] != '\0') ?
>> +argnames[i] : buf,
>> +0, argdtype, false);
>>
>> This requires no new variable and thus no more changes elsewhere.
>>
>> Attached patch with these changes. Please have a look.
>>
>
> Looks great - I added check to NULL only
>

Looks good.
I have not made those changes in my earlier patch as I did not want to
update other code which is not touched by this patch.

Anyways, your changes related to NULL check seems reasonable.
However, in attached patch I have fixed indentation.

Passing it on to the committer.

Thanks
-- 
Jeevan Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index e9d7ef5..f450156 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -433,9 +433,14 @@ do_compile(FunctionCallInfo fcinfo,
 			 errmsg("PL/pgSQL functions cannot accept type %s",
 	format_type_be(argtypeid;
 
-/* Build variable and add to datum list */
-argvariable = plpgsql_build_variable(buf, 0,
-	 argdtype, false);
+/*
+ * Build variable and add to datum list.  If there's a name for
+ * the argument, then use that else use $n name.
+ */
+argvariable = plpgsql_build_variable((argnames != NULL &&
+	  argnames[i][0] != '\0') ?
+	 argnames[i] : buf,
+	 0, argdtype, false);
 
 if (argvariable->dtype == PLPGSQL_DTYPE_VAR)
 {
@@ -461,7 +466,7 @@ do_compile(FunctionCallInfo fcinfo,
 add_parameter_name(argitemtype, argvariable->dno, buf);
 
 /* If there's a name for the argument, make an alias */
-if (argnames && argnames[i][0] != '\0')
+if (argnames != NULL && argnames[i][0] != '\0')
 	add_parameter_name(argitemtype, argvariable->dno,
 	   argnames[i]);
 			}
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 7109996..cf589d5 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -6029,3 +6029,17 @@ SELECT * FROM list_partitioned_table() AS t;
  2
 (2 rows)
 
+--
+-- Check argument name is used instead of $n
+--
+CREATE TYPE ct AS (a int, b int);
+-- Should fail, error message should contain argument name instead of $1
+CREATE OR REPLACE FUNCTION fx(x ct) RETURNS void AS $$
+BEGIN
+  GET DIAGNOSTICS x = ROW_COUNT;
+  RETURN;
+END; $$ LANGUAGE plpgsql;
+ERROR:  "x" is not a scalar variable
+LINE 3:   GET DIAGNOSTICS x = ROW_COUNT;
+  ^
+DROP TYPE ct;
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 771d682..42f51e9 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -4811,3 +4811,17 @@ BEGIN
 END; $$ LANGUAGE plpgsql;
 
 SELECT * FROM list_partitioned_table() AS t;
+
+--
+-- Check argument name is used instead of $n
+--
+CREATE TYPE ct AS (a int, b int);
+
+-- Should fail, error message should contain argument name instead of $1
+CREATE OR REPLACE FUNCTION fx(x ct) RETURNS void AS $$
+BEGIN
+  GET DIAGNOSTICS x = ROW_COUNT;
+  RETURN;
+END; $$ LANGUAGE plpgsql;
+
+DROP TYPE ct;

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

[Pavel Stehule]

Hi

2017-09-08 9:36 GMT+02:00 Jeevan Chalke :

> Hi Pavel,
>
> On Sat, May 20, 2017 at 11:55 AM, Pavel Stehule 
> wrote:
>
>>
>>
>>
>> 2017-05-19 5:48 GMT+02:00 Pavel Stehule :
>>
>>>
>>>
>>> 2017-05-19 3:14 GMT+02:00 Peter Eisentraut <
>>> peter.eisentr...@2ndquadrant.com>:
>>>
 On 5/15/17 14:34, Pavel Stehule wrote:
 > Now, I when I working on plpgsql_check, I have to check function
 > parameters. I can use fn_vargargnos and out_param_varno for list
 of
 > arguments and related varno(s). when I detect some issue, I am
 using
 > refname. It is not too nice now, because these refnames are $
 based.
 > Long names are alias only. There are not a possibility to find
 > related alias.
 >
 > So, my proposal. Now, we can use names as refname of parameter
 > variable. $ based name can be used as alias. From user perspective
 > there are not any change.
 >
 > Comments, notes?
 >
 > here is a patch


> I like the idea of using parameter name instead of $n symbols.
>
> However, I am slightly worried that, at execution time if we want to
> know the parameter position in the actual function signature, then it
> will become difficult to get that from the corresponding datum
> variable. I don't have any use-case for that though. But apart from
> this concern, idea looks good to me.
>

Understand - but it was reason why I implemented this function - when I
have to search parameter name via offset, I cannot to use string searching.
When you know the parameter name, you can use a string searching in text
editor, in pager.

It is better supported now, then current behave.


>
> Here are review comments on the patch:
>
> 1.
> +char   *argname = NULL;
>
> There is no need to initialize argname here. The Later code does that.
>
> 2.
> +argname = (argnames && argnames[i][0] != 0) ? argnames[i]
> : NULL;
>
> It will be better to check '\0' instead of 0, like we have that already.
>

This pattern is somewhere in PLpgSQL code. Your proposal is better.


>
> 3.
> Check for argname exists is not consistent. At one place you have used
> "argname != NULL" and other place it is "argname != '\0'".
> Better to have "argname != NULL" at both the places.
>

sure

>
> 4.
> -- should fail -- message should to contain argument name
> Should be something like this:
> -- Should fail, error message should contain argument name
>
> 5.
> +argvariable = plpgsql_build_variable(argname != NULL ?
> +   argname : buf,
> +   0, argdtype,
> false);
>
> Please correct indentation.
>
> ---
>
> BTW, instead of doing all these changes, I have done these changes this
> way:
>
> -   /* Build variable and add to datum list */
> -   argvariable = plpgsql_build_variable(buf, 0,
> -argdtype, false);
> +   /*
> +* Build variable and add to datum list.  If there's a
> name for
> +* the argument, then use that else use $n name.
> +*/
> +   argvariable = plpgsql_build_variable((argnames &&
> argnames[i][0] != '\0') ?
> +argnames[i] : buf,
> +0, argdtype, false);
>
> This requires no new variable and thus no more changes elsewhere.
>
> Attached patch with these changes. Please have a look.
>

Looks great - I added check to NULL only

Thank you

Pavel


>
> Thanks
>
>
> --
> Jeevan Chalke
> Principal Software Engineer, Product Development
> EnterpriseDB Corporation
> The Enterprise PostgreSQL Company
>
>
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index e9d7ef55e9..f320059fd0 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -433,9 +433,14 @@ do_compile(FunctionCallInfo fcinfo,
 			 errmsg("PL/pgSQL functions cannot accept type %s",
 	format_type_be(argtypeid;
 
-/* Build variable and add to datum list */
-argvariable = plpgsql_build_variable(buf, 0,
-	 argdtype, false);
+/*
+ * Build variable and add to datum list.  If there's a name for
+ * the argument, then use that else use $n name.
+ */
+argvariable = plpgsql_build_variable((argnames != NULL
+		  && argnames[i][0] != '\0') ?
+	 argnames[i] : buf,
+	 0, argdtype, false);
 
 if (argvariable->dtype == PLPGSQL_DTYPE_VAR)
 {
@@ -461,7 +466,7 @@ do_compile(FunctionCallInfo fcinfo,
 add_parameter_name(argitemtype, argvariable->dno, buf);
 
 /* If there's a name for the argument, make an alias */
-if (argnames && argnames[i][0] != '\0')
+if 

Re: [HACKERS] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

[Jeevan Chalke]

Hi Pavel,

On Sat, May 20, 2017 at 11:55 AM, Pavel Stehule 
wrote:

>
>
>
> 2017-05-19 5:48 GMT+02:00 Pavel Stehule :
>
>>
>>
>> 2017-05-19 3:14 GMT+02:00 Peter Eisentraut > com>:
>>
>>> On 5/15/17 14:34, Pavel Stehule wrote:
>>> > Now, I when I working on plpgsql_check, I have to check function
>>> > parameters. I can use fn_vargargnos and out_param_varno for list of
>>> > arguments and related varno(s). when I detect some issue, I am
>>> using
>>> > refname. It is not too nice now, because these refnames are $
>>> based.
>>> > Long names are alias only. There are not a possibility to find
>>> > related alias.
>>> >
>>> > So, my proposal. Now, we can use names as refname of parameter
>>> > variable. $ based name can be used as alias. From user perspective
>>> > there are not any change.
>>> >
>>> > Comments, notes?
>>> >
>>> > here is a patch
>>>
>>>
I like the idea of using parameter name instead of $n symbols.

However, I am slightly worried that, at execution time if we want to
know the parameter position in the actual function signature, then it
will become difficult to get that from the corresponding datum
variable. I don't have any use-case for that though. But apart from
this concern, idea looks good to me.

Here are review comments on the patch:

1.
+char   *argname = NULL;

There is no need to initialize argname here. The Later code does that.

2.
+argname = (argnames && argnames[i][0] != 0) ? argnames[i]
: NULL;

It will be better to check '\0' instead of 0, like we have that already.

3.
Check for argname exists is not consistent. At one place you have used
"argname != NULL" and other place it is "argname != '\0'".
Better to have "argname != NULL" at both the places.

4.
-- should fail -- message should to contain argument name
Should be something like this:
-- Should fail, error message should contain argument name

5.
+argvariable = plpgsql_build_variable(argname != NULL ?
+   argname : buf,
+   0, argdtype,
false);

Please correct indentation.

---

BTW, instead of doing all these changes, I have done these changes this way:

-   /* Build variable and add to datum list */
-   argvariable = plpgsql_build_variable(buf, 0,
-argdtype, false);
+   /*
+* Build variable and add to datum list.  If there's a name
for
+* the argument, then use that else use $n name.
+*/
+   argvariable = plpgsql_build_variable((argnames &&
argnames[i][0] != '\0') ?
+argnames[i] : buf,
+0, argdtype, false);

This requires no new variable and thus no more changes elsewhere.

Attached patch with these changes. Please have a look.

Thanks


-- 
Jeevan Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index e9d7ef5..dd17bb5 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -433,9 +433,13 @@ do_compile(FunctionCallInfo fcinfo,
 			 errmsg("PL/pgSQL functions cannot accept type %s",
 	format_type_be(argtypeid;
 
-/* Build variable and add to datum list */
-argvariable = plpgsql_build_variable(buf, 0,
-	 argdtype, false);
+/*
+ * Build variable and add to datum list.  If there's a name for
+ * the argument, then use that else use $n name.
+ */
+argvariable = plpgsql_build_variable((argnames && argnames[i][0] != '\0') ?
+	 argnames[i] : buf,
+	 0, argdtype, false);
 
 if (argvariable->dtype == PLPGSQL_DTYPE_VAR)
 {
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 7109996..cf589d5 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -6029,3 +6029,17 @@ SELECT * FROM list_partitioned_table() AS t;
  2
 (2 rows)
 
+--
+-- Check argument name is used instead of $n
+--
+CREATE TYPE ct AS (a int, b int);
+-- Should fail, error message should contain argument name instead of $1
+CREATE OR REPLACE FUNCTION fx(x ct) RETURNS void AS $$
+BEGIN
+  GET DIAGNOSTICS x = ROW_COUNT;
+  RETURN;
+END; $$ LANGUAGE plpgsql;
+ERROR:  "x" is not a scalar variable
+LINE 3:   GET DIAGNOSTICS x = ROW_COUNT;
+  ^
+DROP TYPE ct;
diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
index 771d682..42f51e9 100644
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -4811,3 +4811,17 @@ BEGIN
 END; $$ LANGUAGE 

Re: [HACKERS] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

[Pavel Stehule]

2017-05-19 5:48 GMT+02:00 Pavel Stehule :

>
>
> 2017-05-19 3:14 GMT+02:00 Peter Eisentraut  com>:
>
>> On 5/15/17 14:34, Pavel Stehule wrote:
>> > Now, I when I working on plpgsql_check, I have to check function
>> > parameters. I can use fn_vargargnos and out_param_varno for list of
>> > arguments and related varno(s). when I detect some issue, I am using
>> > refname. It is not too nice now, because these refnames are $ based.
>> > Long names are alias only. There are not a possibility to find
>> > related alias.
>> >
>> > So, my proposal. Now, we can use names as refname of parameter
>> > variable. $ based name can be used as alias. From user perspective
>> > there are not any change.
>> >
>> > Comments, notes?
>> >
>> > here is a patch
>>
>> I don't understand what this is changing.  There are not documentation
>> or test changes.
>>
>
> This change is visible only for tools like plpgsql_check probably and
> similar tools. Now, this info is not available from user space (maybe only
> from some error message, I have to recheck it)
>
> What is changed.
>
> PLpgSQL variables has field refname - it can be used if you iterate over
> variables or it is used for some error messages. When some variables is
> searching, then namespace aliases are used. Now for any function argument
> is created variable with refname "$x" and namespace aliases "$x" and "name"
> if name exists. There are not any way, how to get a aliases related to
> variable. When I raise a warning in plpgsql about function arguments I have
> to print $x based messages, what is not too readable if function has lot of
> parameters.
>
> The proposal is the change of refname "$x" to "name" for all variables
> created for function arguments.
>
> There are another possibilities - maintain list of all aliases for
> variables or dynamically search all related aliases in namespace tree. Both
> little bit more code.
>

I wrote small regression test, where expected behave is visible

master:

postgres=# create or replace function fx(x ct)
returns void as $$
begin
  perform 1;
  get diagnostics x = row_count;
end;
$$ language plpgsql;
ERROR:  "$1" is not a scalar variable
LINE 5:   get diagnostics x = row_count;

patched:

 postgres=# create or replace function fx(x ct)
returns void as $$
begin
  perform 1;
  get diagnostics x = row_count;
end;
$$ language plpgsql;
ERROR:  "x" is not a scalar variable
LINE 5:   get diagnostics x = row_count;



> Regards
>
> Pavel
>
>
>>
>> --
>> Peter Eisentraut  http://www.2ndQuadrant.com/
>> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>>
>
>
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index a637551..49327b2 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -409,6 +409,7 @@ do_compile(FunctionCallInfo fcinfo,
 			for (i = 0; i < numargs; i++)
 			{
 char		buf[32];
+char	   *argname = NULL;
 Oid			argtypeid = argtypes[i];
 char		argmode = argmodes ? argmodes[i] : PROARGMODE_IN;
 PLpgSQL_type *argdtype;
@@ -433,9 +434,12 @@ do_compile(FunctionCallInfo fcinfo,
 		   errmsg("PL/pgSQL functions cannot accept type %s",
   format_type_be(argtypeid;
 
+argname = (argnames && argnames[i][0] != 0) ? argnames[i] : NULL;
+
 /* Build variable and add to datum list */
-argvariable = plpgsql_build_variable(buf, 0,
-	 argdtype, false);
+argvariable = plpgsql_build_variable(argname != NULL ?
+		   argname : buf,
+		   0, argdtype, false);
 
 if (argvariable->dtype == PLPGSQL_DTYPE_VAR)
 {
@@ -461,9 +465,9 @@ do_compile(FunctionCallInfo fcinfo,
 add_parameter_name(argitemtype, argvariable->dno, buf);
 
 /* If there's a name for the argument, make an alias */
-if (argnames && argnames[i][0] != '\0')
+if (argname != '\0')
 	add_parameter_name(argitemtype, argvariable->dno,
-	   argnames[i]);
+	   argname);
 			}
 
 			/*
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 7ebbde6..a9fe736 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -5979,3 +5979,15 @@ LINE 1: SELECT (SELECT string_agg(id || '=' || name, ',') FROM d)
^
 QUERY:  SELECT (SELECT string_agg(id || '=' || name, ',') FROM d)
 CONTEXT:  PL/pgSQL function alter_table_under_transition_tables_upd_func() line 3 at RAISE
+-- should fail -- message should to contain argument name
+CREATE TYPE ct AS (a int, b int);
+CREATE OR REPLACE FUNCTION fx(x ct)
+RETURNS void AS $$
+BEGIN
+  GET DIAGNOSTICS x = ROW_COUNT;
+end;
+$$ language plpgsql;
+ERROR:  "x" is not a scalar variable
+LINE 4:   GET DIAGNOSTICS x = ROW_COUNT;
+  ^
+DROP TYPE ct;
diff --git a/src/test/regress/sql/plpgsql.sql 

Re: [HACKERS] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

[Pavel Stehule]

2017-05-19 3:14 GMT+02:00 Peter Eisentraut :

> On 5/15/17 14:34, Pavel Stehule wrote:
> > Now, I when I working on plpgsql_check, I have to check function
> > parameters. I can use fn_vargargnos and out_param_varno for list of
> > arguments and related varno(s). when I detect some issue, I am using
> > refname. It is not too nice now, because these refnames are $ based.
> > Long names are alias only. There are not a possibility to find
> > related alias.
> >
> > So, my proposal. Now, we can use names as refname of parameter
> > variable. $ based name can be used as alias. From user perspective
> > there are not any change.
> >
> > Comments, notes?
> >
> > here is a patch
>
> I don't understand what this is changing.  There are not documentation
> or test changes.
>

This change is visible only for tools like plpgsql_check probably and
similar tools. Now, this info is not available from user space (maybe only
from some error message, I have to recheck it)

What is changed.

PLpgSQL variables has field refname - it can be used if you iterate over
variables or it is used for some error messages. When some variables is
searching, then namespace aliases are used. Now for any function argument
is created variable with refname "$x" and namespace aliases "$x" and "name"
if name exists. There are not any way, how to get a aliases related to
variable. When I raise a warning in plpgsql about function arguments I have
to print $x based messages, what is not too readable if function has lot of
parameters.

The proposal is the change of refname "$x" to "name" for all variables
created for function arguments.

There are another possibilities - maintain list of all aliases for
variables or dynamically search all related aliases in namespace tree. Both
little bit more code.

Regards

Pavel


>
> --
> Peter Eisentraut  http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>

Re: [HACKERS] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

[Peter Eisentraut]

On 5/15/17 14:34, Pavel Stehule wrote:
> Now, I when I working on plpgsql_check, I have to check function
> parameters. I can use fn_vargargnos and out_param_varno for list of
> arguments and related varno(s). when I detect some issue, I am using
> refname. It is not too nice now, because these refnames are $ based.
> Long names are alias only. There are not a possibility to find
> related alias.
> 
> So, my proposal. Now, we can use names as refname of parameter
> variable. $ based name can be used as alias. From user perspective
> there are not any change. 
> 
> Comments, notes?
> 
> here is a patch

I don't understand what this is changing.  There are not documentation
or test changes.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

[Pavel Stehule]

2017-05-13 18:26 GMT+02:00 Pavel Stehule :

> Hi
>
> Now, I when I working on plpgsql_check, I have to check function
> parameters. I can use fn_vargargnos and out_param_varno for list of
> arguments and related varno(s). when I detect some issue, I am using
> refname. It is not too nice now, because these refnames are $ based. Long
> names are alias only. There are not a possibility to find related alias.
>
> So, my proposal. Now, we can use names as refname of parameter variable. $
> based name can be used as alias. From user perspective there are not any
> change.
>
> Comments, notes?
>

here is a patch

Regards

Pavel


>
> Regards
>
> Pavel
>


psql-named-arguments
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers