Обсуждение: [HACKERS] Renaming PG_GETARG functions (was Re: PG_GETARG_GISTENTRY?)

[ changing subject line to possibly draw more attention ]

Mark Dilger <hornschnorter@gmail.com> writes:
>> On Apr 5, 2017, at 9:23 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> In short, if you are supposed to write
>>     FOO  *val = PG_GETARG_FOO(n);
>> then the macro designer blew it, because the name implies that it
>> returns FOO, not pointer to FOO.  This should be
>>     FOO  *val = PG_GETARG_FOO_P(n);

> I have written a patch to fix these macro definitions across src/ and
> contrib/.

So to summarize, this patch proposes to rename some DatumGetFoo,
PG_GETARG_FOO, and PG_RETURN_FOO macros for these datatypes:

NDBOX (contrib/cube)
HSTORE
LTREE and other contrib/ltree types

PG_GETARG_ANY_ARRAY (and there are some related macros it maybe should
have touched, like PG_RETURN_EXPANDED_ARRAY)

JSONB

RANGE

The contrib types don't seem like much of a problem, but I wonder
whether anyone feels that rationalizing the names for array, JSON,
or range-type macros will break too much code.

One option if we do feel that way is that we could provide the
old names as alternatives, thus not breaking external modules.
But that seems like it's sabotaging the basic goal of improving
consistency of naming.

If there are not objections, I plan to push forward with this.
        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

> On Sep 12, 2017, at 1:07 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> [ changing subject line to possibly draw more attention ]
> 
> Mark Dilger <hornschnorter@gmail.com> writes:
>>> On Apr 5, 2017, at 9:23 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> In short, if you are supposed to write
>>>     FOO  *val = PG_GETARG_FOO(n);
>>> then the macro designer blew it, because the name implies that it
>>> returns FOO, not pointer to FOO.  This should be
>>>     FOO  *val = PG_GETARG_FOO_P(n);
> 
>> I have written a patch to fix these macro definitions across src/ and
>> contrib/.
> 

Thanks, Tom, for reviewing my patch.




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

> On 12 Sep 2017, at 22:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> [ changing subject line to possibly draw more attention ]
>
> Mark Dilger <hornschnorter@gmail.com> writes:
>>> On Apr 5, 2017, at 9:23 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> In short, if you are supposed to write
>>>     FOO  *val = PG_GETARG_FOO(n);
>>> then the macro designer blew it, because the name implies that it
>>> returns FOO, not pointer to FOO.  This should be
>>>     FOO  *val = PG_GETARG_FOO_P(n);
>
>> I have written a patch to fix these macro definitions across src/ and
>> contrib/.
>
> So to summarize, this patch proposes to rename some DatumGetFoo,
> PG_GETARG_FOO, and PG_RETURN_FOO macros for these datatypes:
>
> NDBOX (contrib/cube)
> HSTORE
> LTREE and other contrib/ltree types
>
> PG_GETARG_ANY_ARRAY (and there are some related macros it maybe should
> have touched, like PG_RETURN_EXPANDED_ARRAY)
>
> JSONB
>
> RANGE
>
> The contrib types don't seem like much of a problem, but I wonder
> whether anyone feels that rationalizing the names for array, JSON,
> or range-type macros will break too much code.
>
> One option if we do feel that way is that we could provide the
> old names as alternatives, thus not breaking external modules.
> But that seems like it's sabotaging the basic goal of improving
> consistency of naming.
>
> If there are not objections, I plan to push forward with this.

Judging by this post, I’m updating this to “Ready for Committer”.

cheers ./daniel

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