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);