minor: contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in, ..)

Started by Jon Nelsonover 11 years ago7 messagesbugs
Jump to latest
#1Jon Nelson
jnelson+pgsql@jamponi.net

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

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

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jon Nelson (#1)
Re: minor: contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in, ..)

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

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

#3Jon Nelson
jnelson+pgsql@jamponi.net
In reply to: Tom Lane (#2)
Re: minor: contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in, ..)

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

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

#4Bruce Momjian
bruce@momjian.us
In reply to: Jon Nelson (#3)
Re: minor: contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in,..)

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

Attachments:

btree_gin.difftext/x-diff; charset=us-asciiDownload+15-15
#5Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#4)
Re: minor: contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in,..)

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

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

#6Jon Nelson
jnelson+pgsql@jamponi.net
In reply to: Bruce Momjian (#4)
Re: minor: contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in, ..)

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

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

#7Bruce Momjian
bruce@momjian.us
In reply to: Jon Nelson (#6)
Re: minor: contrib/btree_gin/btree_gin.c uses DirectFunctionCall3(inet_in,..)

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

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