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
Attachments:
NameGetDatum.patch.1application/octet-stream; name=NameGetDatum.patch.1Download
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index ef48659..0919ad8 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -1260,7 +1260,7 @@ movedb(const char *dbname, const char *tblspcname)
ScanKeyInit(&scankey,
Anum_pg_database_datname,
BTEqualStrategyNumber, F_NAMEEQ,
- NameGetDatum(dbname));
+ CStringGetDatum(dbname));
sysscan = systable_beginscan(pgdbrel, DatabaseNameIndexId, true,
NULL, 1, &scankey);
oldtuple = systable_getnext(sysscan);
@@ -1486,7 +1486,7 @@ AlterDatabase(ParseState *pstate, AlterDatabaseStmt *stmt, bool isTopLevel)
ScanKeyInit(&scankey,
Anum_pg_database_datname,
BTEqualStrategyNumber, F_NAMEEQ,
- NameGetDatum(stmt->dbname));
+ CStringGetDatum(stmt->dbname));
scan = systable_beginscan(rel, DatabaseNameIndexId, true,
NULL, 1, &scankey);
tuple = systable_getnext(scan);
@@ -1603,7 +1603,7 @@ AlterDatabaseOwner(const char *dbname, Oid newOwnerId)
ScanKeyInit(&scankey,
Anum_pg_database_datname,
BTEqualStrategyNumber, F_NAMEEQ,
- NameGetDatum(dbname));
+ CStringGetDatum(dbname));
scan = systable_beginscan(rel, DatabaseNameIndexId, true,
NULL, 1, &scankey);
tuple = systable_getnext(scan);
@@ -1743,7 +1743,7 @@ get_db_info(const char *name, LOCKMODE lockmode,
ScanKeyInit(&scanKey,
Anum_pg_database_datname,
BTEqualStrategyNumber, F_NAMEEQ,
- NameGetDatum(name));
+ CStringGetDatum(name));
scan = systable_beginscan(relation, DatabaseNameIndexId, true,
NULL, 1, &scanKey);
diff --git a/src/backend/commands/proclang.c b/src/backend/commands/proclang.c
index 761d08f..4f870e8 100644
--- a/src/backend/commands/proclang.c
+++ b/src/backend/commands/proclang.c
@@ -463,7 +463,7 @@ find_language_template(const char *languageName)
ScanKeyInit(&key,
Anum_pg_pltemplate_tmplname,
BTEqualStrategyNumber, F_NAMEEQ,
- NameGetDatum(languageName));
+ CStringGetDatum(languageName));
scan = systable_beginscan(rel, PLTemplateNameIndexId, true,
NULL, 1, &key);
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
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