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

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: proposal - using names as primary names of plpgsql function parameters instead $ based names
Date: 2017-09-11 12:26:54
Message-ID: CAFj8pRDCvnnePLeKj1FqpDmp50g42fZqytaCay3XheDMQT1ctA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2017-09-11 9:46 GMT+02:00 Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>:

> Hi Pavel,
>
>
> On Sat, Sep 9, 2017 at 11:42 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
>
>> Hi
>>
>> 2017-09-08 9:36 GMT+02:00 Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)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.
>

Thank you very much

Regards

Pavel

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2017-09-11 12:39:19 Re: PoC plpgsql - possibility to force custom or generic plan
Previous Message Tom Lane 2017-09-11 12:10:14 Re: Constifying numeric.c's local vars