Обсуждение: minor: contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in,..)

contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in,..)
instead of DirectFunctionCall1(inet_in, one_argument).

That doesn't seem right. Does such a thing matter?

--
Jon

Jon Nelson <jnelson+pgsql@jamponi.net> writes:
> contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in,..)
> instead of DirectFunctionCall1(inet_in, one_argument).

> That doesn't seem right. Does such a thing matter?

It's not really incorrect: in a call going through InputFunctionCall(),
which is the normal path, the two extra arguments would be provided
whether the specific datatype input function needed them or not.

However, I think the usual convention for DirectFunctionCall() usage
is to pass exactly what the target function uses, since you know
exactly what you're calling.  Certainly that's what happens in the
two direct calls to inet_in in the core code.

So I tend to agree that we should change this call to match the others,
but it's purely cosmetic.

            regards, tom lane

On Fri, Nov 14, 2014 at 11:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Jon Nelson <jnelson+pgsql@jamponi.net> writes:
>> contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in,..)
>> instead of DirectFunctionCall1(inet_in, one_argument).
>
>> That doesn't seem right. Does such a thing matter?
>
> It's not really incorrect: in a call going through InputFunctionCall(),
> which is the normal path, the two extra arguments would be provided
> whether the specific datatype input function needed them or not.
>
> However, I think the usual convention for DirectFunctionCall() usage
> is to pass exactly what the target function uses, since you know
> exactly what you're calling.  Certainly that's what happens in the
> two direct calls to inet_in in the core code.
>
> So I tend to agree that we should change this call to match the others,
> but it's purely cosmetic.

So, are there any additional steps that you might recommend that I take?
It's such a trivial thing. I could provide a patch, of course, or a
pull request off of github if people use that.


--
Jon

On Fri, Nov 14, 2014 at 01:12:37PM -0600, Jon Nelson wrote:
> On Fri, Nov 14, 2014 at 11:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Jon Nelson <jnelson+pgsql@jamponi.net> writes:
> >> contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in,..)
> >> instead of DirectFunctionCall1(inet_in, one_argument).
> >
> >> That doesn't seem right. Does such a thing matter?
> >
> > It's not really incorrect: in a call going through InputFunctionCall(),
> > which is the normal path, the two extra arguments would be provided
> > whether the specific datatype input function needed them or not.
> >
> > However, I think the usual convention for DirectFunctionCall() usage
> > is to pass exactly what the target function uses, since you know
> > exactly what you're calling.  Certainly that's what happens in the
> > two direct calls to inet_in in the core code.
> >
> > So I tend to agree that we should change this call to match the others,
> > but it's purely cosmetic.
>
> So, are there any additional steps that you might recommend that I take?
> It's such a trivial thing. I could provide a patch, of course, or a
> pull request off of github if people use that.

Patch attached for review.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

On Fri, Mar 20, 2015 at 05:13:02PM -0400, Bruce Momjian wrote:
> On Fri, Nov 14, 2014 at 01:12:37PM -0600, Jon Nelson wrote:
> > On Fri, Nov 14, 2014 at 11:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > Jon Nelson <jnelson+pgsql@jamponi.net> writes:
> > >> contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in,..)
> > >> instead of DirectFunctionCall1(inet_in, one_argument).
> > >
> > >> That doesn't seem right. Does such a thing matter?
> > >
> > > It's not really incorrect: in a call going through InputFunctionCall(),
> > > which is the normal path, the two extra arguments would be provided
> > > whether the specific datatype input function needed them or not.
> > >
> > > However, I think the usual convention for DirectFunctionCall() usage
> > > is to pass exactly what the target function uses, since you know
> > > exactly what you're calling.  Certainly that's what happens in the
> > > two direct calls to inet_in in the core code.
> > >
> > > So I tend to agree that we should change this call to match the others,
> > > but it's purely cosmetic.
> >
> > So, are there any additional steps that you might recommend that I take?
> > It's such a trivial thing. I could provide a patch, of course, or a
> > pull request off of github if people use that.
>
> Patch attached for review.

Patch applied.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

On Fri, Mar 20, 2015 at 4:13 PM, Bruce Momjian <bruce@momjian.us> wrote:
> On Fri, Nov 14, 2014 at 01:12:37PM -0600, Jon Nelson wrote:
>> On Fri, Nov 14, 2014 at 11:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> > Jon Nelson <jnelson+pgsql@jamponi.net> writes:
>> >> contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in,..)
>> >> instead of DirectFunctionCall1(inet_in, one_argument).
>> >
>> >> That doesn't seem right. Does such a thing matter?
>> >
>> > It's not really incorrect: in a call going through InputFunctionCall(),
>> > which is the normal path, the two extra arguments would be provided
>> > whether the specific datatype input function needed them or not.
>> >
>> > However, I think the usual convention for DirectFunctionCall() usage
>> > is to pass exactly what the target function uses, since you know
>> > exactly what you're calling.  Certainly that's what happens in the
>> > two direct calls to inet_in in the core code.
>> >
>> > So I tend to agree that we should change this call to match the others,
>> > but it's purely cosmetic.
>>
>> So, are there any additional steps that you might recommend that I take?
>> It's such a trivial thing. I could provide a patch, of course, or a
>> pull request off of github if people use that.
>
> Patch attached for review.

For as much as it's worth, I did review the patch and it looks just perfect.

--
Jon

On Thu, Mar 26, 2015 at 06:02:12PM -0500, Jon Nelson wrote:
> On Fri, Mar 20, 2015 at 4:13 PM, Bruce Momjian <bruce@momjian.us> wrote:
> > On Fri, Nov 14, 2014 at 01:12:37PM -0600, Jon Nelson wrote:
> >> On Fri, Nov 14, 2014 at 11:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> > Jon Nelson <jnelson+pgsql@jamponi.net> writes:
> >> >> contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in,..)
> >> >> instead of DirectFunctionCall1(inet_in, one_argument).
> >> >
> >> >> That doesn't seem right. Does such a thing matter?
> >> >
> >> > It's not really incorrect: in a call going through InputFunctionCall(),
> >> > which is the normal path, the two extra arguments would be provided
> >> > whether the specific datatype input function needed them or not.
> >> >
> >> > However, I think the usual convention for DirectFunctionCall() usage
> >> > is to pass exactly what the target function uses, since you know
> >> > exactly what you're calling.  Certainly that's what happens in the
> >> > two direct calls to inet_in in the core code.
> >> >
> >> > So I tend to agree that we should change this call to match the others,
> >> > but it's purely cosmetic.
> >>
> >> So, are there any additional steps that you might recommend that I take?
> >> It's such a trivial thing. I could provide a patch, of course, or a
> >> pull request off of github if people use that.
> >
> > Patch attached for review.
>
> For as much as it's worth, I did review the patch and it looks just perfect.

Patch applied.  Thanks for the report.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +