Here's another attempt to reduce the number of places in the code that
need to be updated when adding a new relkind. It adds a few macros --
RELKIND_HAS_STORAGE(), RELKIND_HAS_SYSTEM_ATTS(), and
RELKIND_HAS_SYSTEM_GENERATED_ATTNAMES() and uses them in place of more
ad-hoc tests for the same conditions -- and it rewords a few
problematic messages (some of which are already slightly inaccurate)
so that they won't need updating when we add a new relkind for foreign
tables.
Thoughts? This is not a complete solution by any means, but I don't
think it makes anything worse (unless you hate the proposed wording, I
suppose) and it has the added benefit of simplifying the code in a few
places.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index f341a72..c2eeb9c 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -109,16 +109,10 @@ get_raw_page_internal(text *relname, ForkNumber forknum, BlockNumber blkno)
rel = relation_openrv(relrv, AccessShareLock);
/* Check that this relation has storage */
- if (rel->rd_rel->relkind == RELKIND_VIEW)
+ if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind))
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot get raw page from view \"%s\"",
- RelationGetRelationName(rel))));
- if (rel->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot get raw page from composite type \"%s\"",
- RelationGetRelationName(rel))));
+ errmsg("cannot get raw page from relation without storage")));
/*
* Reject attempts to read non-local temporary relations; we would be
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 438e8b0..48e9d0a 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1174,7 +1174,7 @@ heap_reloptions(char relkind, Datum reloptions, bool validate)
case RELKIND_RELATION:
return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
default:
- /* sequences, composite types and views are not supported */
+ /* other relation types are not supported */
return NULL;
}
}
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 8027d74..2a0cf0c 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -262,34 +262,13 @@ heap_create(const char *relname,
errdetail("System catalog modifications are currently disallowed.")));
/*
- * Decide if we need storage or not, and handle a couple other special
- * cases for particular relkinds.
+ * Decide if we need storage or not. If not, force reltablespace to zero,
+ * mostly just for cleanliness' sake. We also force reltablespace to zero
+ * for sequences, since we don't support moving them around.
*/
- switch (relkind)
- {
- case RELKIND_VIEW:
- case RELKIND_COMPOSITE_TYPE:
- create_storage = false;
-
- /*
- * Force reltablespace to zero if the relation has no physical
- * storage. This is mainly just for cleanliness' sake.
- */
- reltablespace = InvalidOid;
- break;
- case RELKIND_SEQUENCE:
- create_storage = true;
-
- /*
- * Force reltablespace to zero for sequences, since we don't
- * support moving them around into different tablespaces.
- */
- reltablespace = InvalidOid;
- break;
- default:
- create_storage = true;
- break;
- }
+ create_storage = RELKIND_HAS_STORAGE(relkind);
+ if (!create_storage || relkind == RELKIND_SEQUENCE)
+ reltablespace = InvalidOid;
/*
* Never allow a pg_class entry to explicitly specify the database's
@@ -390,7 +369,7 @@ CheckAttributeNamesTypes(TupleDesc tupdesc, char relkind,
* Skip this for a view or type relation, since those don't have system
* attributes.
*/
- if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE)
+ if (RELKIND_HAS_SYSTEM_ATTS(relkind))
{
for (i = 0; i < natts; i++)
{
@@ -613,11 +592,11 @@ AddNewAttributeTuples(Oid new_rel_oid,
}
/*
- * Next we add the system attributes. Skip OID if rel has no OIDs. Skip
- * all for a view or type relation. We don't bother with making datatype
- * dependencies here, since presumably all these types are pinned.
+ * Next we add the system attributes, if they're needed for this relkind.
+ * We don't bother with making datatype dependencies here, since presumably
+ * all these types are pinned.
*/
- if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE)
+ if (RELKIND_HAS_SYSTEM_ATTS(relkind))
{
for (i = 0; i < (int) lengthof(SysAtt); i++)
{
@@ -1592,11 +1571,8 @@ heap_drop_with_catalog(Oid relid)
/*
* Schedule unlinking of the relation's physical files at commit.
*/
- if (rel->rd_rel->relkind != RELKIND_VIEW &&
- rel->rd_rel->relkind != RELKIND_COMPOSITE_TYPE)
- {
+ if (RELKIND_HAS_STORAGE(rel->rd_rel->relkind))
RelationDropStorage(rel);
- }
/*
* Close relcache entry, but *keep* AccessExclusiveLock on the relation
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 15464c3..94b6b0d 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -187,7 +187,7 @@ analyze_rel(Oid relid, VacuumStmt *vacstmt,
/* No need for a WARNING if we already complained during VACUUM */
if (!(vacstmt->options & VACOPT_VACUUM))
ereport(WARNING,
- (errmsg("skipping \"%s\" --- cannot analyze indexes, views, or special system tables",
+ (errmsg("skipping \"%s\" --- cannot analyze non-tables or special system tables",
RelationGetRelationName(onerel))));
relation_close(onerel, ShareUpdateExclusiveLock);
return;
diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c
index b578818..aa8345a 100644
--- a/src/backend/commands/comment.c
+++ b/src/backend/commands/comment.c
@@ -574,19 +574,17 @@ CheckAttributeComment(Relation relation)
RelationGetRelationName(relation));
/*
- * Allow comments only on columns of tables, views, and composite types
- * (which are the only relkinds for which pg_dump will dump per-column
- * comments). In particular we wish to disallow comments on index
- * columns, because the naming of an index's columns may change across PG
- * versions, so dumping per-column comments could create reload failures.
+ * Allow comments only on columns of relations for which the user controls
+ * the attribute names (which should correspond to the relkinds for which
+ * pg_dump will dump per-column comments). In particular we wish to
+ * disallow comments on index columns, because the naming of an index's
+ * columns may change across PG versions, so dumping per-column comments
+ * could create reload failures.
*/
- if (relation->rd_rel->relkind != RELKIND_RELATION &&
- relation->rd_rel->relkind != RELKIND_VIEW &&
- relation->rd_rel->relkind != RELKIND_COMPOSITE_TYPE)
+ if (RELKIND_HAS_SYSTEM_GENERATED_ATTNAMES(relation->rd_rel->relkind))
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table, view, or composite type",
- RelationGetRelationName(relation))));
+ errmsg("comments on system-generated column names are not supported")));
}
/*
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 7b8bee8..6ad450d 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1227,25 +1227,14 @@ DoCopyTo(CopyState cstate)
if (cstate->rel)
{
- if (cstate->rel->rd_rel->relkind != RELKIND_RELATION)
- {
- if (cstate->rel->rd_rel->relkind == RELKIND_VIEW)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy from view \"%s\"",
- RelationGetRelationName(cstate->rel)),
- errhint("Try the COPY (SELECT ...) TO variant.")));
- else if (cstate->rel->rd_rel->relkind == RELKIND_SEQUENCE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy from sequence \"%s\"",
- RelationGetRelationName(cstate->rel))));
- else
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy from non-table relation \"%s\"",
- RelationGetRelationName(cstate->rel))));
- }
+ char relkind = cstate->rel->rd_rel->relkind;
+
+ if (relkind != RELKIND_RELATION)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot copy from non-table relation"),
+ relkind == RELKIND_VIEW ?
+ errhint("Try the COPY (SELECT ...) TO variant.") : 0));
}
if (pipe)
@@ -1702,23 +1691,9 @@ CopyFrom(CopyState cstate)
Assert(cstate->rel);
if (cstate->rel->rd_rel->relkind != RELKIND_RELATION)
- {
- if (cstate->rel->rd_rel->relkind == RELKIND_VIEW)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy to view \"%s\"",
- RelationGetRelationName(cstate->rel))));
- else if (cstate->rel->rd_rel->relkind == RELKIND_SEQUENCE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy to sequence \"%s\"",
- RelationGetRelationName(cstate->rel))));
- else
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy to non-table relation \"%s\"",
- RelationGetRelationName(cstate->rel))));
- }
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot copy to non-table relation")));
/*----------
* Check to see if we can avoid writing WAL
diff --git a/src/backend/commands/seclabel.c b/src/backend/commands/seclabel.c
index 762bbae..9dc33c4 100644
--- a/src/backend/commands/seclabel.c
+++ b/src/backend/commands/seclabel.c
@@ -360,16 +360,14 @@ CheckAttributeSecLabel(Relation relation)
RelationGetRelationName(relation));
/*
- * Allow security labels only on columns of tables, views, and composite
- * types (which are the only relkinds for which pg_dump will dump labels).
+ * Allow security labels only on columns of relations for which the user
+ * controls the attribute names (which should be the same relkinds for
+ * which pg_dump will dump labels).
*/
- if (relation->rd_rel->relkind != RELKIND_RELATION &&
- relation->rd_rel->relkind != RELKIND_VIEW &&
- relation->rd_rel->relkind != RELKIND_COMPOSITE_TYPE)
+ if (RELKIND_HAS_SYSTEM_GENERATED_ATTNAMES(relation->rd_rel->relkind))
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table, view, or composite type",
- RelationGetRelationName(relation))));
+ errmsg("security labels on system-generated column names are not supported")));
}
void
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3f6b814..e60b164 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1986,16 +1986,17 @@ renameatt_internal(Oid myrelid,
* references are by attnum. But it doesn't seem right to allow users to
* change names that are hardcoded into the system, hence the following
* restriction.
+ *
+ * NB: Index column names are also system-generated, and we elsewhere treat
+ * them as such (e.g. comments and security labels on index columns are
+ * refused), but we overlook that detail here for historical reasons.
*/
relkind = RelationGetForm(targetrelation)->relkind;
- if (relkind != RELKIND_RELATION &&
- relkind != RELKIND_VIEW &&
- relkind != RELKIND_COMPOSITE_TYPE &&
- relkind != RELKIND_INDEX)
+ if (RELKIND_HAS_SYSTEM_GENERATED_ATTNAMES(relkind)
+ && relkind != RELKIND_INDEX)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table, view, composite type or index",
- RelationGetRelationName(targetrelation))));
+ errmsg("system-generated column names may not be changed")));
/*
* permissions checking. only the owner of a class can change its schema.
@@ -4137,12 +4138,10 @@ ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
* returned by AddRelationNewConstraints, so that the right thing happens
* when a datatype's default applies.
*
- * We skip this step completely for views. For a view, we can only get
- * here from CREATE OR REPLACE VIEW, which historically doesn't set up
- * defaults, not even for domain-typed columns. And in any case we
- * mustn't invoke Phase 3 on a view, since it has no storage.
+ * We skip this step completely for relations which do not have storage,
+ * since phase 3 must not be invoked on such relations.
*/
- if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE && attribute.attnum > 0)
+ if (RELKIND_HAS_STORAGE(relkind) && attribute.attnum > 0)
{
defval = (Expr *) build_column_default(rel, attribute.attnum);
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 2f68df4..028b36b 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -894,7 +894,7 @@ vacuum_rel(Oid relid, VacuumStmt *vacstmt, bool do_toast, bool for_wraparound,
onerel->rd_rel->relkind != RELKIND_TOASTVALUE)
{
ereport(WARNING,
- (errmsg("skipping \"%s\" --- cannot vacuum indexes, views, or special system tables",
+ (errmsg("skipping \"%s\" --- cannot vacuum non-tables or special system tables",
RelationGetRelationName(onerel))));
relation_close(onerel, lmode);
PopActiveSnapshot();
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 856daa7..9db81e1 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -1492,10 +1492,7 @@ pgstat_initstats(Relation rel)
char relkind = rel->rd_rel->relkind;
/* We only count stats for things that have storage */
- if (!(relkind == RELKIND_RELATION ||
- relkind == RELKIND_INDEX ||
- relkind == RELKIND_TOASTVALUE ||
- relkind == RELKIND_SEQUENCE))
+ if (!RELKIND_HAS_STORAGE(relkind))
{
rel->pgstat_info = NULL;
return;
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index f33c29e..e43c336 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -528,24 +528,18 @@ pg_relation_filenode(PG_FUNCTION_ARGS)
PG_RETURN_NULL();
relform = (Form_pg_class) GETSTRUCT(tuple);
- switch (relform->relkind)
+ if (RELKIND_HAS_STORAGE(relform->relkind))
{
- case RELKIND_RELATION:
- case RELKIND_INDEX:
- case RELKIND_SEQUENCE:
- case RELKIND_TOASTVALUE:
- /* okay, these have storage */
- if (relform->relfilenode)
- result = relform->relfilenode;
- else /* Consult the relation mapper */
- result = RelationMapOidToFilenode(relid,
- relform->relisshared);
- break;
-
- default:
- /* no storage, return NULL */
- result = InvalidOid;
- break;
+ if (relform->relfilenode)
+ result = relform->relfilenode;
+ else /* Consult the relation mapper */
+ result = RelationMapOidToFilenode(relid,
+ relform->relisshared);
+ }
+ else
+ {
+ /* no storage, return NULL */
+ result = InvalidOid;
}
ReleaseSysCache(tuple);
@@ -576,34 +570,27 @@ pg_relation_filepath(PG_FUNCTION_ARGS)
PG_RETURN_NULL();
relform = (Form_pg_class) GETSTRUCT(tuple);
- switch (relform->relkind)
+ if (RELKIND_HAS_STORAGE(relform->relkind))
{
- case RELKIND_RELATION:
- case RELKIND_INDEX:
- case RELKIND_SEQUENCE:
- case RELKIND_TOASTVALUE:
- /* okay, these have storage */
-
- /* This logic should match RelationInitPhysicalAddr */
- if (relform->reltablespace)
- rnode.spcNode = relform->reltablespace;
- else
- rnode.spcNode = MyDatabaseTableSpace;
- if (rnode.spcNode == GLOBALTABLESPACE_OID)
- rnode.dbNode = InvalidOid;
- else
- rnode.dbNode = MyDatabaseId;
- if (relform->relfilenode)
- rnode.relNode = relform->relfilenode;
- else /* Consult the relation mapper */
- rnode.relNode = RelationMapOidToFilenode(relid,
- relform->relisshared);
- break;
-
- default:
- /* no storage, return NULL */
- rnode.relNode = InvalidOid;
- break;
+ /* This logic should match RelationInitPhysicalAddr */
+ if (relform->reltablespace)
+ rnode.spcNode = relform->reltablespace;
+ else
+ rnode.spcNode = MyDatabaseTableSpace;
+ if (rnode.spcNode == GLOBALTABLESPACE_OID)
+ rnode.dbNode = InvalidOid;
+ else
+ rnode.dbNode = MyDatabaseId;
+ if (relform->relfilenode)
+ rnode.relNode = relform->relfilenode;
+ else /* Consult the relation mapper */
+ rnode.relNode = RelationMapOidToFilenode(relid,
+ relform->relisshared);
+ }
+ else
+ {
+ /* no storage, return NULL */
+ rnode.relNode = InvalidOid;
}
if (!OidIsValid(rnode.relNode))
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index 39f9743..eb46c83 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -149,6 +149,14 @@ DESCR("");
#define RELKIND_VIEW 'v' /* view */
#define RELKIND_COMPOSITE_TYPE 'c' /* composite type */
+#define RELKIND_HAS_STORAGE(c) \
+ ((c) != RELKIND_VIEW && (c) != RELKIND_COMPOSITE_TYPE)
+#define RELKIND_HAS_SYSTEM_ATTS(c) \
+ ((c) != RELKIND_VIEW && (c) != RELKIND_COMPOSITE_TYPE)
+#define RELKIND_HAS_SYSTEM_GENERATED_ATTNAMES(c) \
+ ((c) != RELKIND_RELATION && (c) != RELKIND_VIEW && \
+ (c) != RELKIND_COMPOSITE_TYPE)
+
#define RELPERSISTENCE_PERMANENT 'p'
#define RELPERSISTENCE_UNLOGGED 'u'
#define RELPERSISTENCE_TEMP 't'
diff --git a/src/test/regress/expected/copyselect.out b/src/test/regress/expected/copyselect.out
index cbc140c..a98de6c 100644
--- a/src/test/regress/expected/copyselect.out
+++ b/src/test/regress/expected/copyselect.out
@@ -30,7 +30,7 @@ copy test1 to stdout;
-- This should fail
--
copy v_test1 to stdout;
-ERROR: cannot copy from view "v_test1"
+ERROR: cannot copy from non-table relation
HINT: Try the COPY (SELECT ...) TO variant.
--
-- Test COPY (select) TO
@@ -109,9 +109,9 @@ t
-- This should fail
--
\copy v_test1 to stdout
-ERROR: cannot copy from view "v_test1"
+ERROR: cannot copy from non-table relation
HINT: Try the COPY (SELECT ...) TO variant.
-\copy: ERROR: cannot copy from view "v_test1"
+\copy: ERROR: cannot copy from non-table relation
HINT: Try the COPY (SELECT ...) TO variant.
--
-- Test \copy (select ...)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
