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

Jeevan Chalke <jeevan.chalke@enterprisedb.com>

From: Jeevan Chalke <jeevan.chalke@enterprisedb.com>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2017-09-11T07:46:22Z
Lists: pgsql-hackers

Attachments

Hi Pavel,


On Sat, Sep 9, 2017 at 11:42 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:

> Hi
>
> 2017-09-08 9:36 GMT+02:00 Jeevan Chalke <jeevan.chalke@enterprisedb.com>:
>
>> 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

Commits

  1. Prefer argument name over "$n" for the refname of a plpgsql argument.