Hi Pavel,
On Sat, Sep 9, 2017 at 11:42 AM, Pavel Stehule <[email protected]>
wrote:
> Hi
>
> 2017-09-08 9:36 GMT+02:00 Jeevan Chalke <[email protected]>:
>
>> 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
