On Mon, Dec 27, 2010 at 10:18 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Robert Haas <robertmh...@gmail.com> writes:
>> or if we go with the some-assembly required version, perhaps:
>
>> "tables do not support %s"
>> "views do not support %s"
>> "indexes do not support %s"
>
> +1 for that way.  Although personally I'd reverse the phrasing:
>
>        /* translator: %s is the name of a SQL command */
>        %s does not support tables

To me, it seems as though it's the object which doesn't support the
operation, rather than the other way around.  Reversing it makes most
sense to me in cases where it's an implementation restriction, such as
"DROP COLUMN does not support views".  But at least to me, the
phrasing "SET WITHOUT CLUSTER does not support views" suggests that
SET WITHOUT CLUSTER is somehow defective, which is not really the
message I think we want to convey there.  But maybe we need some other
votes.

I took a crack at implementing this and the results were mixed - see
attached patch.  It works pretty well for the most part, but there are
a couple of warts.  For ALTER TABLE commands like DROP COLUMN and SET
WITHOUT CLUSTER the results look pretty good, and I find the new error
messages a definite improvement over the old ones.  It's not quite so
good for setting reloptions or attoptions.  You get something like:

ERROR:  views do not support SET (...)
ERROR:  views do not support ALTER COLUMN SET (...)

...which might actually be OK, if not fantastically wonderful.  One
might think of mitigating this problem by writing "ALTER TABLE SET
(...)" rather than just "SET (...)", but that's not easily done
because there are several equivalent forms (for example, a view can be
modified with either ALTER VIEW or ALTER TABLE, for historical - or
perhaps hysterical - reasons).  An even bigger problem is this:

rhaas=# alter view v add primary key (a);
ERROR:  views do not support ADD INDEX

The problem is that alter table actions AT_AddIndex and
AT_AddConstraint don't tie neatly back to a particular piece of
syntax.  The message as written isn't incomprehensible (especially if
you're reading it in English) but it definitely leaves something to be
desired.

Ideas?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c
index b578818..9701233 100644
--- a/src/backend/commands/comment.c
+++ b/src/backend/commands/comment.c
@@ -22,6 +22,7 @@
 #include "catalog/pg_shdescription.h"
 #include "commands/comment.h"
 #include "commands/dbcommands.h"
+#include "commands/tablecmds.h"
 #include "libpq/be-fsstubs.h"
 #include "miscadmin.h"
 #include "parser/parse_func.h"
@@ -583,10 +584,7 @@ CheckAttributeComment(Relation relation)
 	if (relation->rd_rel->relkind != RELKIND_RELATION &&
 		relation->rd_rel->relkind != RELKIND_VIEW &&
 		relation->rd_rel->relkind != RELKIND_COMPOSITE_TYPE)
-		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("\"%s\" is not a table, view, or composite type",
-						RelationGetRelationName(relation))));
+		ErrorWrongRelationType(relation, "COMMENT ON COLUMN");
 }
 
 /*
diff --git a/src/backend/commands/seclabel.c b/src/backend/commands/seclabel.c
index 762bbae..00b64bf 100644
--- a/src/backend/commands/seclabel.c
+++ b/src/backend/commands/seclabel.c
@@ -366,10 +366,7 @@ CheckAttributeSecLabel(Relation relation)
 	if (relation->rd_rel->relkind != RELKIND_RELATION &&
 		relation->rd_rel->relkind != RELKIND_VIEW &&
 		relation->rd_rel->relkind != RELKIND_COMPOSITE_TYPE)
-		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("\"%s\" is not a table, view, or composite type",
-						RelationGetRelationName(relation))));
+		ErrorWrongRelationType(relation, "SECURITY LABEL ON COLUMN");
 }
 
 void
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 6729d83..ee410c4 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -222,6 +222,44 @@ static const struct dropmsgstrings dropmsgstringarray[] = {
 	{'\0', 0, NULL, NULL, NULL, NULL}
 };
 
+/*
+ * Error-reporting support for wrong-object type errors
+ */
+struct wrongtypestrings
+{
+	char		kind;
+	const char *wrongtype_message;
+};
+
+static const struct wrongtypestrings wrongtypestringarray[] = {
+	{RELKIND_RELATION,
+		/* translator: %s is an SQL command */
+		gettext_noop("tables do not support %s")},
+	{RELKIND_SEQUENCE,
+		/* translator: %s is an SQL command */
+		gettext_noop("sequences do not support %s")},
+	{RELKIND_VIEW,
+		/* translator: %s is an SQL command */
+		gettext_noop("views do not support %s")},
+	{RELKIND_INDEX,
+		/* translator: %s is an SQL command */
+		gettext_noop("indexes do not support %s")},
+	{RELKIND_COMPOSITE_TYPE,
+		/* translator: %s is an SQL command */
+		gettext_noop("composite types do not support %s")},
+	{RELKIND_TOASTVALUE,
+		/* translator: %s is an SQL command */
+		gettext_noop("TOAST tables do not support %s")},
+	{'\0', NULL}
+};
+
+/* Convenience strings for ATSimplePermissions */
+const char rk_relation[2] = { RELKIND_RELATION, '\0' };
+const char rk_view[2] = { RELKIND_VIEW, '\0' };
+const char rk_relation_view[3] = { RELKIND_RELATION, RELKIND_VIEW, '\0' };
+const char rk_relation_index[3] = { RELKIND_RELATION, RELKIND_INDEX, '\0' };
+const char rk_relation_type[3] =
+	{ RELKIND_RELATION, RELKIND_COMPOSITE_TYPE, '\0' };
 
 static void truncate_check_rel(Relation rel);
 static List *MergeAttributes(List *schema, List *supers, char relpersistence,
@@ -264,8 +302,8 @@ static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
 static void ATRewriteTables(List **wqueue, LOCKMODE lockmode);
 static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode);
 static AlteredTableInfo *ATGetQueueEntry(List **wqueue, Relation rel);
-static void ATSimplePermissions(Relation rel, bool allowView, bool allowType);
-static void ATSimplePermissionsRelationOrIndex(Relation rel);
+static void ATSimplePermissions(Relation rel, const char *relkinds,
+					const char *command);
 static void ATSimpleRecursion(List **wqueue, Relation rel,
 				  AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode);
 static void ATOneLevelRecursion(List **wqueue, Relation rel,
@@ -1096,10 +1134,7 @@ truncate_check_rel(Relation rel)
 
 	/* Only allow truncate on regular tables */
 	if (rel->rd_rel->relkind != RELKIND_RELATION)
-		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("\"%s\" is not a table",
-						RelationGetRelationName(rel))));
+		ErrorWrongRelationType(rel, "TRUNCATE");
 
 	/* Permissions checks */
 	aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
@@ -1994,8 +2029,7 @@ renameatt_internal(Oid myrelid,
 		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 altered")));
 
 	/*
 	 * permissions checking.  only the owner of a class can change its schema.
@@ -2684,14 +2718,14 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 	switch (cmd->subtype)
 	{
 		case AT_AddColumn:		/* ADD COLUMN */
-			ATSimplePermissions(rel, false, true);
+			ATSimplePermissions(rel, rk_relation_type, "ADD COLUMN");
 			/* Performs own recursion */
 			ATPrepAddColumn(wqueue, rel, recurse, recursing, cmd, lockmode);
 			pass = AT_PASS_ADD_COL;
 			break;
 		case AT_AddColumnToView:		/* add column via CREATE OR REPLACE
 										 * VIEW */
-			ATSimplePermissions(rel, true, false);
+			ATSimplePermissions(rel, rk_view, "ADD COLUMN");
 			/* Performs own recursion */
 			ATPrepAddColumn(wqueue, rel, recurse, recursing, cmd, lockmode);
 			pass = AT_PASS_ADD_COL;
@@ -2704,19 +2738,20 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 			 * substitutes default values into INSERTs before it expands
 			 * rules.
 			 */
-			ATSimplePermissions(rel, true, false);
+			ATSimplePermissions(rel, rk_relation_view,
+				"ALTER COLUMN DEFAULT");
 			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
 			/* No command-specific prep needed */
 			pass = cmd->def ? AT_PASS_ADD_CONSTR : AT_PASS_DROP;
 			break;
 		case AT_DropNotNull:	/* ALTER COLUMN DROP NOT NULL */
-			ATSimplePermissions(rel, false, false);
+			ATSimplePermissions(rel, rk_relation, "DROP NOT NULL");
 			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
 			/* No command-specific prep needed */
 			pass = AT_PASS_DROP;
 			break;
 		case AT_SetNotNull:		/* ALTER COLUMN SET NOT NULL */
-			ATSimplePermissions(rel, false, false);
+			ATSimplePermissions(rel, rk_relation, "SET NOT NULL");
 			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
 			/* No command-specific prep needed */
 			pass = AT_PASS_ADD_CONSTR;
@@ -2728,31 +2763,37 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 			pass = AT_PASS_MISC;
 			break;
 		case AT_SetOptions:		/* ALTER COLUMN SET ( options ) */
+			/* This command never recurses */
+			ATSimplePermissions(rel, rk_relation_index,
+								"ALTER COLUMN SET (...)");
+			pass = AT_PASS_MISC;
+			break;
 		case AT_ResetOptions:	/* ALTER COLUMN RESET ( options ) */
-			ATSimplePermissionsRelationOrIndex(rel);
 			/* This command never recurses */
+			ATSimplePermissions(rel, rk_relation_index,
+								"ALTER COLUMN RESET (...)");
 			pass = AT_PASS_MISC;
 			break;
 		case AT_SetStorage:		/* ALTER COLUMN SET STORAGE */
-			ATSimplePermissions(rel, false, false);
+			ATSimplePermissions(rel, rk_relation, "ALTER COLUMN SET STORAGE");
 			ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
 			/* No command-specific prep needed */
 			pass = AT_PASS_MISC;
 			break;
 		case AT_DropColumn:		/* DROP COLUMN */
-			ATSimplePermissions(rel, false, true);
+			ATSimplePermissions(rel, rk_relation_type, "DROP COLUMN");
 			ATPrepDropColumn(wqueue, rel, recurse, recursing, cmd, lockmode);
 			/* Recursion occurs during execution phase */
 			pass = AT_PASS_DROP;
 			break;
 		case AT_AddIndex:		/* ADD INDEX */
-			ATSimplePermissions(rel, false, false);
+			ATSimplePermissions(rel, rk_relation, "ADD INDEX");
 			/* This command never recurses */
 			/* No command-specific prep needed */
 			pass = AT_PASS_ADD_INDEX;
 			break;
 		case AT_AddConstraint:	/* ADD CONSTRAINT */
-			ATSimplePermissions(rel, false, false);
+			ATSimplePermissions(rel, rk_relation, "ADD CONSTRAINT");
 			/* Recursion occurs during execution phase */
 			/* No command-specific prep needed except saving recurse flag */
 			if (recurse)
@@ -2760,7 +2801,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 			pass = AT_PASS_ADD_CONSTR;
 			break;
 		case AT_DropConstraint:	/* DROP CONSTRAINT */
-			ATSimplePermissions(rel, false, false);
+			ATSimplePermissions(rel, rk_relation, "DROP CONSTRAINT");
 			/* Recursion occurs during execution phase */
 			/* No command-specific prep needed except saving recurse flag */
 			if (recurse)
@@ -2768,7 +2809,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 			pass = AT_PASS_DROP;
 			break;
 		case AT_AlterColumnType:		/* ALTER COLUMN TYPE */
-			ATSimplePermissions(rel, false, true);
+			ATSimplePermissions(rel, rk_relation_type, "ALTER COLUMN TYPE");
 			/* Performs own recursion */
 			ATPrepAlterColumnType(wqueue, tab, rel, recurse, recursing, cmd, lockmode);
 			pass = AT_PASS_ALTER_TYPE;
@@ -2779,21 +2820,26 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 			pass = AT_PASS_MISC;
 			break;
 		case AT_ClusterOn:		/* CLUSTER ON */
+			ATSimplePermissions(rel, rk_relation, "CLUSTER ON");
+			/* These commands never recurse */
+			/* No command-specific prep needed */
+			pass = AT_PASS_MISC;
+			break;
 		case AT_DropCluster:	/* SET WITHOUT CLUSTER */
-			ATSimplePermissions(rel, false, false);
+			ATSimplePermissions(rel, rk_relation, "SET WITHOUT CLUSTER");
 			/* These commands never recurse */
 			/* No command-specific prep needed */
 			pass = AT_PASS_MISC;
 			break;
 		case AT_AddOids:		/* SET WITH OIDS */
-			ATSimplePermissions(rel, false, false);
+			ATSimplePermissions(rel, rk_relation, "SET WITH OIDS");
 			/* Performs own recursion */
 			if (!rel->rd_rel->relhasoids || recursing)
 				ATPrepAddOids(wqueue, rel, recurse, cmd, lockmode);
 			pass = AT_PASS_ADD_COL;
 			break;
 		case AT_DropOids:		/* SET WITHOUT OIDS */
-			ATSimplePermissions(rel, false, false);
+			ATSimplePermissions(rel, rk_relation, "SET WITHOUT OIDS");
 			/* Performs own recursion */
 			if (rel->rd_rel->relhasoids)
 			{
@@ -2807,38 +2853,82 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
 			pass = AT_PASS_DROP;
 			break;
 		case AT_SetTableSpace:	/* SET TABLESPACE */
-			ATSimplePermissionsRelationOrIndex(rel);
+			ATSimplePermissions(rel, rk_relation_index, "SET TABLESPACE");
 			/* This command never recurses */
 			ATPrepSetTableSpace(tab, rel, cmd->name, lockmode);
 			pass = AT_PASS_MISC;	/* doesn't actually matter */
 			break;
 		case AT_SetRelOptions:	/* SET (...) */
+			ATSimplePermissions(rel, rk_relation_index, "SET (...)");
+			/* This command never recurses */
+			/* No command-specific prep needed */
+			pass = AT_PASS_MISC;
 		case AT_ResetRelOptions:		/* RESET (...) */
-			ATSimplePermissionsRelationOrIndex(rel);
+			ATSimplePermissions(rel, rk_relation_index, "RESET (...)");
 			/* This command never recurses */
 			/* No command-specific prep needed */
 			pass = AT_PASS_MISC;
 			break;
 		case AT_AddInherit:		/* INHERIT */
-			ATSimplePermissions(rel, false, false);
+			ATSimplePermissions(rel, rk_relation, "INHERIT");
 			/* This command never recurses */
 			ATPrepAddInherit(rel);
 			pass = AT_PASS_MISC;
 			break;
-		case AT_EnableTrig:		/* ENABLE TRIGGER variants */
-		case AT_EnableAlwaysTrig:
-		case AT_EnableReplicaTrig:
+		case AT_EnableTrig:			/* ENABLE TRIGGER variants */
 		case AT_EnableTrigAll:
 		case AT_EnableTrigUser:
+			ATSimplePermissions(rel, rk_relation, "ENABLE TRIGGER");
+			/* These commands never recurse */
+			/* No command-specific prep needed */
+			pass = AT_PASS_MISC;
+			break;
+		case AT_EnableAlwaysTrig:
+			ATSimplePermissions(rel, rk_relation, "ENABLE ALWAYS TRIGGER");
+			/* These commands never recurse */
+			/* No command-specific prep needed */
+			pass = AT_PASS_MISC;
+			break;
+		case AT_EnableReplicaTrig:
+			ATSimplePermissions(rel, rk_relation, "ENABLE REPLICA TRIGGER");
+			/* These commands never recurse */
+			/* No command-specific prep needed */
+			pass = AT_PASS_MISC;
+			break;
 		case AT_DisableTrig:	/* DISABLE TRIGGER variants */
 		case AT_DisableTrigAll:
 		case AT_DisableTrigUser:
+			ATSimplePermissions(rel, rk_relation, "DISABLE TRIGGER");
+			/* These commands never recurse */
+			/* No command-specific prep needed */
+			pass = AT_PASS_MISC;
+			break;
 		case AT_EnableRule:		/* ENABLE/DISABLE RULE variants */
+			ATSimplePermissions(rel, rk_relation, "ENABLE RULE");
+			/* These commands never recurse */
+			/* No command-specific prep needed */
+			pass = AT_PASS_MISC;
+			break;
 		case AT_EnableAlwaysRule:
+			ATSimplePermissions(rel, rk_relation, "ENABLE ALWAYS RULE");
+			/* These commands never recurse */
+			/* No command-specific prep needed */
+			pass = AT_PASS_MISC;
+			break;
 		case AT_EnableReplicaRule:
+			ATSimplePermissions(rel, rk_relation, "ENABLE REPLICA RULE");
+			/* These commands never recurse */
+			/* No command-specific prep needed */
+			pass = AT_PASS_MISC;
+			break;
 		case AT_DisableRule:
+			ATSimplePermissions(rel, rk_relation, "DISABLE RULE");
+			/* These commands never recurse */
+			/* No command-specific prep needed */
+			pass = AT_PASS_MISC;
+			break;
 		case AT_DropInherit:	/* NO INHERIT */
-			ATSimplePermissions(rel, false, false);
+			ATSimplePermissions(rel, rk_relation, "NO INHERIT");
 			/* These commands never recurse */
 			/* No command-specific prep needed */
 			pass = AT_PASS_MISC;
@@ -3559,66 +3649,15 @@ ATGetQueueEntry(List **wqueue, Relation rel)
 /*
  * ATSimplePermissions
  *
- * - Ensure that it is a relation (or possibly a view)
+ * - Check the relkind against the provided list
  * - Ensure this user is the owner
  * - Ensure that it is not a system table
  */
 static void
-ATSimplePermissions(Relation rel, bool allowView, bool allowType)
+ATSimplePermissions(Relation rel, const char *relkinds, const char *command)
 {
-	if (rel->rd_rel->relkind != RELKIND_RELATION)
-	{
-		if (allowView)
-		{
-			if (rel->rd_rel->relkind != RELKIND_VIEW)
-				ereport(ERROR,
-						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-						 errmsg("\"%s\" is not a table or view",
-								RelationGetRelationName(rel))));
-		}
-		else if (allowType)
-		{
-			if (rel->rd_rel->relkind != RELKIND_COMPOSITE_TYPE)
-				ereport(ERROR,
-						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-						 errmsg("\"%s\" is not a table or composite type",
-								RelationGetRelationName(rel))));
-		}
-		else
-			ereport(ERROR,
-					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-					 errmsg("\"%s\" is not a table",
-							RelationGetRelationName(rel))));
-	}
-
-	/* Permissions checks */
-	if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
-		aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
-					   RelationGetRelationName(rel));
-
-	if (!allowSystemTableMods && IsSystemRelation(rel))
-		ereport(ERROR,
-				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-				 errmsg("permission denied: \"%s\" is a system catalog",
-						RelationGetRelationName(rel))));
-}
-
-/*
- * ATSimplePermissionsRelationOrIndex
- *
- * - Ensure that it is a relation or an index
- * - Ensure this user is the owner
- * - Ensure that it is not a system table
- */
-static void
-ATSimplePermissionsRelationOrIndex(Relation rel)
-{
-	if (rel->rd_rel->relkind != RELKIND_RELATION &&
-		rel->rd_rel->relkind != RELKIND_INDEX)
-		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("\"%s\" is not a table or index",
-						RelationGetRelationName(rel))));
+	if (!strchr(relkinds, rel->rd_rel->relkind))
+		ErrorWrongRelationType(rel, command);
 
 	/* Permissions checks */
 	if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
@@ -4449,10 +4488,7 @@ ATPrepSetStatistics(Relation rel, const char *colName, Node *newValue, LOCKMODE
 	 */
 	if (rel->rd_rel->relkind != RELKIND_RELATION &&
 		rel->rd_rel->relkind != RELKIND_INDEX)
-		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("\"%s\" is not a table or index",
-						RelationGetRelationName(rel))));
+		ErrorWrongRelationType(rel, "ALTER COLUMN .. SET STATISTICS");
 
 	/* Permissions checks */
 	if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
@@ -4692,7 +4728,7 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
 
 	/* At top level, permission check was done in ATPrepCmd, else do it */
 	if (recursing)
-		ATSimplePermissions(rel, false, true);
+		ATSimplePermissions(rel, rk_relation_type, "DROP COLUMN");
 
 	/*
 	 * get the number of the attribute
@@ -4996,7 +5032,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 
 	/* At top level, permission check was done in ATPrepCmd, else do it */
 	if (recursing)
-		ATSimplePermissions(rel, false, false);
+		ATSimplePermissions(rel, rk_relation, "ADD CONSTRAINT");
 
 	/*
 	 * Call AddRelationNewConstraints to do the work, making sure it works on
@@ -5940,7 +5976,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 
 	/* At top level, permission check was done in ATPrepCmd, else do it */
 	if (recursing)
-		ATSimplePermissions(rel, false, false);
+		ATSimplePermissions(rel, rk_relation, "DROP CONSTRAINT");
 
 	conrel = heap_open(ConstraintRelationId, RowExclusiveLock);
 
@@ -6881,10 +6917,7 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lock
 				break;
 			/* FALL THRU */
 		default:
-			ereport(ERROR,
-					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-					 errmsg("\"%s\" is not a table, view, or sequence",
-							NameStr(tuple_class->relname))));
+			ErrorWrongRelationType(target_rel, "OWNER TO");
 	}
 
 	/*
@@ -7188,10 +7221,8 @@ ATExecSetRelOptions(Relation rel, List *defList, bool isReset, LOCKMODE lockmode
 			(void) index_reloptions(rel->rd_am->amoptions, newOptions, true);
 			break;
 		default:
-			ereport(ERROR,
-					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-					 errmsg("\"%s\" is not a table, index, or TOAST table",
-							RelationGetRelationName(rel))));
+			ErrorWrongRelationType(rel, isReset ?
+				"RESET (...)" : "SET (...)");
 			break;
 	}
 
@@ -7543,7 +7574,7 @@ ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode)
 	 * Must be owner of both parent and child -- child was checked by
 	 * ATSimplePermissions call in ATPrepCmd
 	 */
-	ATSimplePermissions(parent_rel, false, false);
+	ATSimplePermissions(parent_rel, rk_relation, "INHERIT");
 
 	/* Permanent rels cannot inherit from temporary ones */
 	if (RelationUsesTempNamespace(parent_rel)
@@ -8186,10 +8217,7 @@ AlterTableNamespace(RangeVar *relation, const char *newschema,
 		case RELKIND_TOASTVALUE:
 			/* FALL THRU */
 		default:
-			ereport(ERROR,
-					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-					 errmsg("\"%s\" is not a table, view, or sequence",
-							RelationGetRelationName(rel))));
+			ErrorWrongRelationType(rel, "SET SCHEMA");
 	}
 
 	/* get schema OID and check its permissions */
@@ -8376,6 +8404,22 @@ AlterSeqNamespaces(Relation classRel, Relation rel,
 	relation_close(depRel, AccessShareLock);
 }
 
+/*
+ * Emit the right error message for a SQL command issue on a relation of
+ * the wrong type.
+ */
+void
+ErrorWrongRelationType(Relation rel, const char *command)
+{
+	const struct wrongtypestrings *entry;
+
+	for (entry = wrongtypestringarray; entry->kind != '\0'; entry++)
+		if (entry->kind == rel->rd_rel->relkind)
+			ereport(ERROR,
+					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+					 errmsg(entry->wrongtype_message, command)));
+	elog(ERROR, "unexpected relkind: %c", rel->rd_rel->relkind);
+}
 
 /*
  * This code supports
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index ad932d3..d802128 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -60,6 +60,8 @@ extern AttrNumber *varattnos_map(TupleDesc olddesc, TupleDesc newdesc);
 extern AttrNumber *varattnos_map_schema(TupleDesc old, List *schema);
 extern void change_varattnos_of_a_node(Node *node, const AttrNumber *newattno);
 
+extern void ErrorWrongRelationType(Relation rel, const char *command);
+
 extern void register_on_commit_action(Oid relid, OnCommitAction action);
 extern void remove_on_commit_action(Oid relid);
 
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e415730..b7917ea 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -599,9 +599,9 @@ ERROR:  cannot alter system column "oid"
 -- try creating a view and altering that, should fail
 create view myview as select * from atacc1;
 alter table myview alter column test drop not null;
-ERROR:  "myview" is not a table
+ERROR:  views do not support DROP NOT NULL
 alter table myview alter column test set not null;
-ERROR:  "myview" is not a table
+ERROR:  views do not support SET NOT NULL
 drop view myview;
 drop table atacc1;
 -- test inheritance
@@ -854,7 +854,7 @@ select * from myview;
 (0 rows)
 
 alter table myview drop d;
-ERROR:  "myview" is not a table or composite type
+ERROR:  views do not support DROP COLUMN
 drop view myview;
 -- test some commands to make sure they fail on the dropped column
 analyze atacc1(a);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to