Обсуждение: inappropriate use of NameGetDatum macro

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

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

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