inappropriate use of NameGetDatum macro

Started by Mark Dilgerover 9 years ago3 messageshackers
Jump to latest
#1Mark Dilger
mark.dilger@enterprisedb.com

Friends,

there are several places in the code where variables defined as
(char *) or as (const char *) are passed to the NameGetDatum()
macro. I believe it would be better form to use CStringGetDatum()
in these locations. I am aware that these two macros are internally
the same.

src/backend/commands/proclang.c, line 466.
src/backend/commands/dbcommands.c, lines 1263, 1489, 1606, 1746.

Am I overlooking some reason why the code is correct as is? If not,
I am attaching a patch that applies cleanly for me against master,
compiles, and passes the regression tests.

Thanks,

Mark Dilger

Attachments:

NameGetDatum.patch.1application/octet-stream; name=NameGetDatum.patch.1Download+5-5
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mark Dilger (#1)
Re: inappropriate use of NameGetDatum macro

Mark Dilger <hornschnorter@gmail.com> writes:

there are several places in the code where variables defined as
(char *) or as (const char *) are passed to the NameGetDatum()
macro. I believe it would be better form to use CStringGetDatum()
in these locations. I am aware that these two macros are internally
the same.

Hm, I agree, this feels wrong. I suppose you could argue that the
called functions are expecting Name pointers not CString pointers,
but that type cheat is happening anyway. It would be better form
to explicitly pass a CString datum if that's what we're passing.

I'm tempted to propose that we redefine NameGetDatum as

#define NameGetDatum(X) CStringGetDatum(NameStr(*(X)))

which should do the same thing at runtime, but would result in a
compile error if what's passed isn't declared as Name (or NameData*).
This would be asymmetrical with the way DatumGetName looks, though.

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#2)
Re: inappropriate use of NameGetDatum macro

I wrote:

Mark Dilger <hornschnorter@gmail.com> writes:

there are several places in the code where variables defined as
(char *) or as (const char *) are passed to the NameGetDatum()
macro. I believe it would be better form to use CStringGetDatum()
in these locations. I am aware that these two macros are internally
the same.

I'm tempted to propose that we redefine NameGetDatum as
#define NameGetDatum(X) CStringGetDatum(NameStr(*(X)))

Pushed with that change.

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