I'm regularly surprised that the only non-static function we seem to
have for getting a relation name by oid is get_rel_name. It doesn't
handle schema qualification at all, and it returns NULL rather than
ERROR'ing.
Doing it correctly involves interacting with the syscache, calling
RelationIsVisible and calling get_namespace_name if needed, then
passing the result to quote_qualified_identifier.
so here's what I propose to do:
Add get_qualified_relation_name(bool force_qualified, bool missing_ok)
to ruleutils.c and builtins.h.
(Unless there's somewhere better? I wanted to put it in lsyscache.c
alongside get_rel_name, but it uses quote_qualified_identifier,
StringInfo, etc, so it doesn't fit well in lsyscache.c.)
Replace generate_qualified_relation_name in ruleutils.c with a call to
get_qualified_relation_name(relid, true, false) .
Add comments to RelationGetRelationName, get_rel_name and regclassout
pointing to get_qualified_relation_name.
The only part I don't like here is the two boolean arguments, since
they're ugly to read. But inventing a new flags field seemed a bit
heavy for such a trivial task.
I had a quick look through the codebase for places where this pattern
is repeated and found astonishingly few. In most places we just use
get_rel_name and hope the user can figure out any ambiguity.
I did notice that in heap_copy_data and AlterTableMoveAll we manually
schema-qualify a relation name and fail to call
quote_qualified_identifier, so it's unclear if we mean a relation
named "my.relation" or the relation "relation" in schema "my". But in
diagnostic output, meh, whatever.
Arguably regclassout does the same thing as
get_qualified_relation_name, but I didn't replace it because it cares
about IsBootstrapProcessingMode().
(Prompted by https://dba.stackexchange.com/q/187788/7788)
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From 011cd6e08d6d29854db637d6beb8709615f376cb Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Fri, 6 Oct 2017 10:48:17 +0800
Subject: [PATCH v1] Add get_qualified_relation_name
It was surprisingly hard to just get a table name in PostgreSQL at the
C level.
To simplify that, expose the functionality of generate_qualified_relation_name
and generate_relation_name to callers in the wider codebase and to extensions
in the form of a new get_qualified_relation_name function.
---
src/backend/utils/adt/regproc.c | 2 +
src/backend/utils/adt/ruleutils.c | 81 +++++++++++++++++++++++++++----------
src/backend/utils/cache/lsyscache.c | 5 ++-
src/include/utils/builtins.h | 2 +
src/include/utils/rel.h | 4 ++
5 files changed, 70 insertions(+), 24 deletions(-)
diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c
index 6fe81fa..691efa8 100644
--- a/src/backend/utils/adt/regproc.c
+++ b/src/backend/utils/adt/regproc.c
@@ -967,6 +967,8 @@ to_regclass(PG_FUNCTION_ARGS)
/*
* regclassout - converts class OID to "class_name"
+ *
+ * (See get_qualified_relation_name for a direct-callable version).
*/
Datum
regclassout(PG_FUNCTION_ARGS)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index eb01f35..0eccc02 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -10483,6 +10483,9 @@ quote_qualified_identifier(const char *qualifier,
*
* This differs from the underlying get_rel_name() function in that it will
* throw error instead of silently returning NULL if the OID is bad.
+ *
+ * The returned relation name is not guaranteed to be unique outside the
+ * schema, so it is frequently preferable to use get_qualified_relation_name.
*/
static char *
get_relation_name(Oid relid)
@@ -10495,6 +10498,56 @@ get_relation_name(Oid relid)
}
/*
+ * get_qualified_relation_name
+ *
+ * Get a relation name as a palloc'd string in the current memory context.
+ *
+ * If the relation is not on the current search path, or if force_qualify is
+ * true, the namespace for the relation is prepended.
+ *
+ * ERROR's on a missing relation unless missing_ok is set, in which case returns
+ * NULL.
+ *
+ * The quoting rules used are the same as those for regclass quoting: each
+ * component is quoted if necessary to prevent ambiguity.
+ */
+char *
+get_qualified_relation_name(Oid relid, bool force_qualify, bool missing_ok)
+{
+ HeapTuple tp;
+ Form_pg_class reltup;
+ char *relname;
+ char *nspname = NULL;
+ char *result;
+
+ tp = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+ if (!HeapTupleIsValid(tp))
+ {
+ if (missing_ok)
+ return NULL;
+ else
+ elog(ERROR, "cache lookup failed for relation %u", relid);
+ }
+
+ reltup = (Form_pg_class) GETSTRUCT(tp);
+ relname = NameStr(reltup->relname);
+
+ if (force_qualify || !RelationIsVisible(relid))
+ {
+ nspname = get_namespace_name(reltup->relnamespace);
+ if (!nspname)
+ elog(ERROR, "cache lookup failed for namespace %u",
+ reltup->relnamespace);
+ }
+
+ result = quote_qualified_identifier(nspname, relname);
+
+ ReleaseSysCache(tp);
+
+ return result;
+}
+
+/*
* generate_relation_name
* Compute the name to display for a relation specified by OID
*
@@ -10503,6 +10556,11 @@ get_relation_name(Oid relid)
* If namespaces isn't NIL, it must be a list of deparse_namespace nodes.
* We will forcibly qualify the relation name if it equals any CTE name
* visible in the namespace list.
+ *
+ * This doesn't use get_qualified_relation_name because it has
+ * to do CTE namespace clash checks to decide whether to force-qualify
+ * the name. For that it needs the namespace, and we don't want to do
+ * a second syscache lookup by calling get_qualified_relation_name.
*/
static char *
generate_relation_name(Oid relid, List *namespaces)
@@ -10567,28 +10625,7 @@ generate_relation_name(Oid relid, List *namespaces)
static char *
generate_qualified_relation_name(Oid relid)
{
- HeapTuple tp;
- Form_pg_class reltup;
- char *relname;
- char *nspname;
- char *result;
-
- tp = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
- if (!HeapTupleIsValid(tp))
- elog(ERROR, "cache lookup failed for relation %u", relid);
- reltup = (Form_pg_class) GETSTRUCT(tp);
- relname = NameStr(reltup->relname);
-
- nspname = get_namespace_name(reltup->relnamespace);
- if (!nspname)
- elog(ERROR, "cache lookup failed for namespace %u",
- reltup->relnamespace);
-
- result = quote_qualified_identifier(nspname, relname);
-
- ReleaseSysCache(tp);
-
- return result;
+ return get_qualified_relation_name(relid, true, false);
}
/*
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 82763f8..0c8fdfc 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -1719,8 +1719,9 @@ get_relnatts(Oid relid)
*
* Returns a palloc'd copy of the string, or NULL if no such relation.
*
- * NOTE: since relation name is not unique, be wary of code that uses this
- * for anything except preparing error messages.
+ * NOTE: since relation name is not unique, be wary of code that uses this for
+ * anything except preparing error messages. See get_qualified_relation_name
+ * for a safer option.
*/
char *
get_rel_name(Oid relid)
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 762532f..105daef 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -78,6 +78,8 @@ extern bool quote_all_identifiers;
extern const char *quote_identifier(const char *ident);
extern char *quote_qualified_identifier(const char *qualifier,
const char *ident);
+extern char *get_qualified_relation_name(Oid relid, bool force_qualify,
+ bool missing_ok);
/* varchar.c */
extern int bpchartruelen(char *s, int len);
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 4bc61e5..f68142c 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -432,6 +432,8 @@ typedef struct ViewOptions
* Returns the rel's name.
*
* Note that the name is only unique within the containing namespace.
+ *
+ * See also get_rel_name(...), and get_qualified_relation_name(...).
*/
#define RelationGetRelationName(relation) \
(NameStr((relation)->rd_rel->relname))
@@ -439,6 +441,8 @@ typedef struct ViewOptions
/*
* RelationGetNamespace
* Returns the rel's namespace OID.
+ *
+ * See also get_namespace_name(...).
*/
#define RelationGetNamespace(relation) \
((relation)->rd_rel->relnamespace)
--
2.9.5
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
