On Mon, Dec 27, 2010 at 2:06 PM, Robert Haas <robertmh...@gmail.com> wrote:
> 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?
Here's a somewhat more complete patch implementing this concept, plus
adding additional messages for non-support of constraints, rules, and
triggers. More could be done in this vein, but this picks a decent
fraction of the low-hanging fruit.
I've had to change some of the heap_open(rv) calls to
relation_open(rv) to avoid having the former throw the wrong error
message before the latter kicks in. I think there might be stylistic
objections to that, but I'm not sure what else to propose. I'm
actually pretty suspicious that many of the heap_open(rv) calls I
*didn't* change are either already a little iffy or likely to become
so once the SQL/MED stuff for foreign tables goes in. They make it
easy to forget that we've got a whole pile of relkinds and you
actually need to really think about which ones you can handle.
For example, on unpatched head:
rhaas=# create view v as select 1 as a;
CREATE VIEW
rhaas=# cluster v;
ERROR: there is no previously clustered index for table "v"
The error message is demonstrably correct in the sense that, first,
there isn't any table v, only a view v, so surely table v has no
clustered index - or anything else; and second, even if we construe
table "v" to mean view "v", it is certainly right to say it has no
clustered index because it does not - and can not - have any indexes
at all. But as undeniably true as that error message is, it's a bad
error message. With the patch:
rhaas=# cluster v;
ERROR: views do not support CLUSTER
That's more like it.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 249067f..1555b61 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -113,7 +113,9 @@ cluster(ClusterStmt *stmt, bool isTopLevel)
Relation rel;
/* Find and lock the table */
- rel = heap_openrv(stmt->relation, AccessExclusiveLock);
+ rel = relation_openrv(stmt->relation, AccessExclusiveLock);
+ if (rel->rd_rel->relkind != RELKIND_RELATION)
+ ErrorWrongRelkind(rel, WRONG_RELKIND_FOR_COMMAND, "CLUSTER");
tableOid = RelationGetRelid(rel);
diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c
index b578818..66df9f8 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,8 @@ 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))));
+ ErrorWrongRelkind(relation, WRONG_RELKIND_FOR_COMMAND,
+ "COMMENT ON COLUMN");
}
/*
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 7b8bee8..488cc80 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -27,6 +27,7 @@
#include "catalog/pg_type.h"
#include "commands/copy.h"
#include "commands/defrem.h"
+#include "commands/tablecmds.h"
#include "commands/trigger.h"
#include "executor/executor.h"
#include "libpq/libpq.h"
@@ -998,8 +999,19 @@ DoCopy(const CopyStmt *stmt, const char *queryString)
cstate->queryDesc = NULL;
/* Open and lock the relation, using the appropriate lock type. */
- cstate->rel = heap_openrv(stmt->relation,
+ cstate->rel = relation_openrv(stmt->relation,
(is_from ? RowExclusiveLock : AccessShareLock));
+ if (cstate->rel->rd_rel->relkind != RELKIND_RELATION)
+ {
+ if (!is_from && cstate->rel->rd_rel->relkind == RELKIND_VIEW)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("views do not support COPY TO"),
+ errhint("Try the COPY (SELECT ...) TO variant.")));
+ else
+ ErrorWrongRelkind(cstate->rel, WRONG_RELKIND_FOR_COMMAND,
+ is_from ? "COPY FROM" : "COPY TO");
+ }
tupDesc = RelationGetDescr(cstate->rel);
@@ -1225,29 +1237,6 @@ DoCopyTo(CopyState cstate)
{
bool pipe = (cstate->filename == NULL);
- 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))));
- }
- }
-
if (pipe)
{
if (whereToSendOutput == DestRemote)
@@ -1701,25 +1690,6 @@ 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))));
- }
-
/*----------
* 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..5c7bf29 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))));
+ ErrorWrongRelkind(relation, "SECURITY LABEL ON COLUMN");
}
void
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 6729d83..80017e8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -222,6 +222,65 @@ 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 *command_message;
+ const char *constraint_message;
+ const char *trigger_message;
+ const char *rule_message;
+};
+
+static const struct wrongtypestrings wrongtypestringarray[] = {
+ {RELKIND_RELATION,
+ /* translator: %s is an SQL command */
+ gettext_noop("tables do not support %s"),
+ NULL, /* constraints are supported, so no message */
+ NULL, /* rules are supported, so no message */
+ NULL}, /* triggers are supported, so no message */
+ {RELKIND_SEQUENCE,
+ /* translator: %s is an SQL command */
+ gettext_noop("sequences do not support %s"),
+ gettext_noop("sequences do not support constraints"),
+ gettext_noop("sequences do not support rules"),
+ gettext_noop("sequences do not support triggers")},
+ {RELKIND_VIEW,
+ /* translator: %s is an SQL command */
+ gettext_noop("views do not support %s"),
+ gettext_noop("views do not support constraints"),
+ NULL, /* rules are supported, so no message */
+ NULL}, /* triggers are supported, so no message */
+ {RELKIND_INDEX,
+ /* translator: %s is an SQL command */
+ gettext_noop("indexes do not support %s"),
+ gettext_noop("indexes do not support constraints"),
+ gettext_noop("indexes do not support rules"),
+ gettext_noop("indexes do not support triggers")},
+ {RELKIND_COMPOSITE_TYPE,
+ /* translator: %s is an SQL command */
+ gettext_noop("composite types do not support %s"),
+ gettext_noop("composite types do not support constraints"),
+ gettext_noop("composite types do not support rules"),
+ gettext_noop("composite types do not support triggers")},
+ {RELKIND_TOASTVALUE,
+ /* translator: %s is an SQL command */
+ gettext_noop("TOAST tables do not support %s"),
+ gettext_noop("TOAST tables do not support constraints"),
+ gettext_noop("TOAST tables do not support rules"),
+ gettext_noop("TOAST tables do not support triggers")},
+ {'\0', NULL, NULL, 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 +323,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,
+ WrongRelkindDetail detail, const char *command);
static void ATSimpleRecursion(List **wqueue, Relation rel,
AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode);
static void ATOneLevelRecursion(List **wqueue, Relation rel,
@@ -841,7 +900,7 @@ ExecuteTruncate(TruncateStmt *stmt)
bool recurse = interpretInhOption(rv->inhOpt);
Oid myrelid;
- rel = heap_openrv(rv, AccessExclusiveLock);
+ rel = relation_openrv(rv, AccessExclusiveLock);
myrelid = RelationGetRelid(rel);
/* don't throw error for "TRUNCATE foo, foo" */
if (list_member_oid(relids, myrelid))
@@ -868,7 +927,7 @@ ExecuteTruncate(TruncateStmt *stmt)
continue;
/* find_all_inheritors already got lock */
- rel = heap_open(childrelid, NoLock);
+ rel = relation_open(childrelid, NoLock);
truncate_check_rel(rel);
rels = lappend(rels, rel);
relids = lappend_oid(relids, childrelid);
@@ -899,7 +958,7 @@ ExecuteTruncate(TruncateStmt *stmt)
Oid relid = lfirst_oid(cell);
Relation rel;
- rel = heap_open(relid, AccessExclusiveLock);
+ rel = relation_open(relid, AccessExclusiveLock);
ereport(NOTICE,
(errmsg("truncate cascades to table \"%s\"",
RelationGetRelationName(rel))));
@@ -1096,10 +1155,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))));
+ ErrorWrongRelkind(rel, WRONG_RELKIND_FOR_COMMAND, "TRUNCATE");
/* Permissions checks */
aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
@@ -1994,8 +2050,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 +2739,17 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
switch (cmd->subtype)
{
case AT_AddColumn: /* ADD COLUMN */
- ATSimplePermissions(rel, false, true);
+ ATSimplePermissions(rel, rk_relation_type,
+ WRONG_RELKIND_FOR_COMMAND, "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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "ADD COLUMN");
/* Performs own recursion */
ATPrepAddColumn(wqueue, rel, recurse, recursing, cmd, lockmode);
pass = AT_PASS_ADD_COL;
@@ -2704,19 +2762,25 @@ 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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "SET NOT NULL");
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
/* No command-specific prep needed */
pass = AT_PASS_ADD_CONSTR;
@@ -2728,31 +2792,47 @@ 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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "ALTER COLUMN RESET (...)");
pass = AT_PASS_MISC;
break;
case AT_SetStorage: /* ALTER COLUMN SET STORAGE */
- ATSimplePermissions(rel, false, false);
+ ATSimplePermissions(rel, rk_relation,
+ WRONG_RELKIND_FOR_COMMAND,
+ "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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "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,
+ WRONG_RELKIND_FOR_CONSTRAINTS,
+ NULL);
/* 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,
+ WRONG_RELKIND_FOR_CONSTRAINTS,
+ NULL);
/* Recursion occurs during execution phase */
/* No command-specific prep needed except saving recurse flag */
if (recurse)
@@ -2760,7 +2840,9 @@ 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,
+ WRONG_RELKIND_FOR_CONSTRAINTS,
+ NULL);
/* Recursion occurs during execution phase */
/* No command-specific prep needed except saving recurse flag */
if (recurse)
@@ -2768,7 +2850,9 @@ 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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "ALTER COLUMN TYPE");
/* Performs own recursion */
ATPrepAlterColumnType(wqueue, tab, rel, recurse, recursing, cmd, lockmode);
pass = AT_PASS_ALTER_TYPE;
@@ -2779,21 +2863,34 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_MISC;
break;
case AT_ClusterOn: /* CLUSTER ON */
+ ATSimplePermissions(rel, rk_relation,
+ WRONG_RELKIND_FOR_COMMAND,
+ "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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "SET WITHOUT OIDS");
/* Performs own recursion */
if (rel->rd_rel->relhasoids)
{
@@ -2807,20 +2904,32 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_DROP;
break;
case AT_SetTableSpace: /* SET TABLESPACE */
- ATSimplePermissionsRelationOrIndex(rel);
+ ATSimplePermissions(rel, rk_relation_index,
+ WRONG_RELKIND_FOR_COMMAND,
+ "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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "INHERIT");
/* This command never recurses */
ATPrepAddInherit(rel);
pass = AT_PASS_MISC;
@@ -2833,12 +2942,28 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_DisableTrig: /* DISABLE TRIGGER variants */
case AT_DisableTrigAll:
case AT_DisableTrigUser:
+ ATSimplePermissions(rel, rk_relation,
+ WRONG_RELKIND_FOR_TRIGGERS,
+ NULL);
+ /* These commands never recurse */
+ /* No command-specific prep needed */
+ pass = AT_PASS_MISC;
+ break;
case AT_EnableRule: /* ENABLE/DISABLE RULE variants */
case AT_EnableAlwaysRule:
case AT_EnableReplicaRule:
case AT_DisableRule:
+ ATSimplePermissions(rel, rk_relation,
+ WRONG_RELKIND_FOR_RULES,
+ NULL);
+ /* 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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "NO INHERIT");
/* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
@@ -3559,66 +3684,16 @@ ATGetQueueEntry(List **wqueue, Relation rel)
/*
* ATSimplePermissions
*
- * - Ensure that it is a relation (or possibly a view)
- * - Ensure this user is the owner
- * - Ensure that it is not a system table
- */
-static void
-ATSimplePermissions(Relation rel, bool allowView, bool allowType)
-{
- 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
+ * - Check the relkind against the provided list
* - Ensure this user is the owner
* - Ensure that it is not a system table
*/
static void
-ATSimplePermissionsRelationOrIndex(Relation rel)
+ATSimplePermissions(Relation rel, const char *relkinds,
+ WrongRelkindDetail detail, const char *command)
{
- 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))
+ ErrorWrongRelkind(rel, detail, command);
/* Permissions checks */
if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
@@ -4449,10 +4524,8 @@ 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))));
+ ErrorWrongRelkind(rel, WRONG_RELKIND_FOR_COMMAND,
+ "ALTER COLUMN .. SET STATISTICS");
/* Permissions checks */
if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
@@ -4692,7 +4765,9 @@ 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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "DROP COLUMN");
/*
* get the number of the attribute
@@ -4996,7 +5071,9 @@ 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,
+ WRONG_RELKIND_FOR_CONSTRAINTS,
+ NULL);
/*
* Call AddRelationNewConstraints to do the work, making sure it works on
@@ -5940,7 +6017,9 @@ 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,
+ WRONG_RELKIND_FOR_CONSTRAINTS,
+ NULL);
conrel = heap_open(ConstraintRelationId, RowExclusiveLock);
@@ -6881,10 +6960,8 @@ 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))));
+ ErrorWrongRelkind(target_rel, WRONG_RELKIND_FOR_COMMAND,
+ "OWNER TO");
}
/*
@@ -7188,10 +7265,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))));
+ ErrorWrongRelkind(rel, WRONG_RELKIND_FOR_COMMAND, isReset ?
+ "RESET (...)" : "SET (...)");
break;
}
@@ -7543,7 +7618,9 @@ 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,
+ WRONG_RELKIND_FOR_COMMAND,
+ "INHERIT");
/* Permanent rels cannot inherit from temporary ones */
if (RelationUsesTempNamespace(parent_rel)
@@ -8186,10 +8263,8 @@ 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))));
+ ErrorWrongRelkind(rel, WRONG_RELKIND_FOR_COMMAND,
+ "SET SCHEMA");
}
/* get schema OID and check its permissions */
@@ -8376,6 +8451,50 @@ 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
+ErrorWrongRelkind(Relation rel, WrongRelkindDetail detail,
+ const char *command)
+{
+ const struct wrongtypestrings *entry;
+
+ for (entry = wrongtypestringarray; entry->kind != '\0'; entry++)
+ if (entry->kind == rel->rd_rel->relkind)
+ break;
+ if (entry->kind == '\0')
+ elog(ERROR, "unexpected relkind: %c", rel->rd_rel->relkind);
+ switch (detail)
+ {
+ case WRONG_RELKIND_FOR_COMMAND:
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg(entry->command_message, command)));
+ break;
+ case WRONG_RELKIND_FOR_CONSTRAINTS:
+ Assert(entry->constraint_message != NULL);
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("%s", _(entry->constraint_message))));
+ break;
+ case WRONG_RELKIND_FOR_TRIGGERS:
+ Assert(entry->trigger_message != NULL);
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("%s", _(entry->trigger_message))));
+ break;
+ case WRONG_RELKIND_FOR_RULES:
+ Assert(entry->rule_message != NULL);
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("%s", _(entry->rule_message))));
+ break;
+ default:
+ elog(ERROR, "unexpected WrongRelkindDetail");
+ }
+}
/*
* This code supports
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 8195392..3c2d352 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -27,6 +27,7 @@
#include "catalog/pg_type.h"
#include "commands/dbcommands.h"
#include "commands/defrem.h"
+#include "commands/tablecmds.h"
#include "commands/trigger.h"
#include "executor/executor.h"
#include "executor/instrument.h"
@@ -149,7 +150,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
* need to take an AccessExclusiveLock to add one of those, just as we do
* with ON SELECT rules.
*/
- rel = heap_openrv(stmt->relation, ShareRowExclusiveLock);
+ rel = relation_openrv(stmt->relation, ShareRowExclusiveLock);
/*
* Triggers must be on tables or views, and there are additional
@@ -187,10 +188,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
errdetail("Views cannot have TRUNCATE triggers.")));
}
else
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table or view",
- RelationGetRelationName(rel))));
+ ErrorWrongRelkind(rel, WRONG_RELKIND_FOR_TRIGGERS, NULL);
if (!allowSystemTableMods && IsSystemRelation(rel))
ereport(ERROR,
@@ -1089,10 +1087,7 @@ RemoveTriggerById(Oid trigOid)
if (rel->rd_rel->relkind != RELKIND_RELATION &&
rel->rd_rel->relkind != RELKIND_VIEW)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table or view",
- RelationGetRelationName(rel))));
+ ErrorWrongRelkind(rel, WRONG_RELKIND_FOR_TRIGGERS, NULL);
if (!allowSystemTableMods && IsSystemRelation(rel))
ereport(ERROR,
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index aa7c144..90bf17e 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -1406,7 +1406,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
int count;
Assert(IsA(inh, RangeVar));
- rel = heap_openrv(inh, AccessShareLock);
+ rel = relation_openrv(inh, AccessShareLock);
if (rel->rd_rel->relkind != RELKIND_RELATION)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 4354897..d81bce6 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -22,6 +22,7 @@
#include "catalog/objectaccess.h"
#include "catalog/pg_rewrite.h"
#include "catalog/storage.h"
+#include "commands/tablecmds.h"
#include "miscadmin.h"
#include "nodes/nodeFuncs.h"
#include "parser/parse_utilcmd.h"
@@ -255,10 +256,7 @@ DefineQueryRewrite(char *rulename,
*/
if (event_relation->rd_rel->relkind != RELKIND_RELATION &&
event_relation->rd_rel->relkind != RELKIND_VIEW)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table or view",
- RelationGetRelationName(event_relation))));
+ ErrorWrongRelkind(event_relation, WRONG_RELKIND_FOR_RULES, NULL);
if (!allowSystemTableMods && IsSystemRelation(event_relation))
ereport(ERROR,
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index ad932d3..9a16488 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -60,6 +60,16 @@ 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);
+typedef enum
+{
+ WRONG_RELKIND_FOR_COMMAND,
+ WRONG_RELKIND_FOR_CONSTRAINTS,
+ WRONG_RELKIND_FOR_TRIGGERS,
+ WRONG_RELKIND_FOR_RULES
+} WrongRelkindDetail;
+extern void ErrorWrongRelkind(Relation rel, WrongRelkindDetail detail,
+ 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);
diff --git a/src/test/regress/expected/copyselect.out b/src/test/regress/expected/copyselect.out
index cbc140c..d7a04a0 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: views do not support COPY TO
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: views do not support COPY TO
HINT: Try the COPY (SELECT ...) TO variant.
-\copy: ERROR: cannot copy from view "v_test1"
+\copy: ERROR: views do not support COPY TO
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
