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
>
>
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 |