From 2e32dce36cd34e6d58c99067969f98de1bb1264b Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 13 Apr 2020 15:29:46 +0200 Subject: [PATCH] Improve error messages about mismatching relkind Most error messages about a relkind that was not supported or appropriate for the command was of the pattern "relation \"%s\" is not a table, foreign table, or materialized view" This style can become verbose and tedious to maintain. Moreover, it's not very helpful: If I'm trying to create a comment on a TOAST table, which is not supported, then the information that I could have created a comment on a materialized view is pointless. Instead, write the primary error message shorter and saying more directly that what was attempted is not supported or possible. Then, in the detail message show show relkind the object was. To simplify that, add a new function errdetail_relkind() that does this. In passing, make use of RELKIND_HAS_STORAGE() where appropriate, instead of listing out the relkinds individually. --- .../pg_visibility/expected/pg_visibility.out | 75 ++++++++++------ contrib/pg_visibility/pg_visibility.c | 5 +- contrib/pgstattuple/expected/pgstattuple.out | 18 ++-- contrib/pgstattuple/pgstatapprox.c | 5 +- contrib/pgstattuple/pgstatindex.c | 11 +-- src/backend/catalog/Makefile | 1 + src/backend/catalog/heap.c | 7 +- src/backend/catalog/pg_class.c | 51 +++++++++++ src/backend/catalog/toasting.c | 6 +- src/backend/commands/comment.c | 5 +- src/backend/commands/indexcmds.c | 16 +--- src/backend/commands/lockcmds.c | 5 +- src/backend/commands/seclabel.c | 5 +- src/backend/commands/sequence.c | 5 +- src/backend/commands/statscmds.c | 5 +- src/backend/commands/tablecmds.c | 88 ++++--------------- src/backend/commands/trigger.c | 15 ++-- src/backend/parser/parse_utilcmd.c | 5 +- src/backend/postmaster/pgstat.c | 6 +- src/backend/rewrite/rewriteDefine.c | 8 +- src/backend/utils/adt/dbsize.c | 73 ++++++--------- src/include/catalog/pg_class.h | 1 + src/test/regress/expected/alter_table.out | 9 +- .../regress/expected/create_table_like.out | 3 +- src/test/regress/expected/foreign_data.out | 6 +- src/test/regress/expected/indexing.out | 3 +- src/test/regress/expected/sequence.out | 3 +- src/test/regress/expected/stats_ext.out | 12 ++- 28 files changed, 235 insertions(+), 217 deletions(-) create mode 100644 src/backend/catalog/pg_class.c diff --git a/contrib/pg_visibility/expected/pg_visibility.out b/contrib/pg_visibility/expected/pg_visibility.out index 2abc1b5107..01ff1361d4 100644 --- a/contrib/pg_visibility/expected/pg_visibility.out +++ b/contrib/pg_visibility/expected/pg_visibility.out @@ -41,66 +41,91 @@ ROLLBACK; create table test_partitioned (a int) partition by list (a); -- these should all fail select pg_visibility('test_partitioned', 0); -ERROR: "test_partitioned" is not a table, materialized view, or TOAST table +ERROR: relation "test_partitioned" does not have a visibility map +DETAIL: "test_partitioned" is a foreign table. select pg_visibility_map('test_partitioned'); -ERROR: "test_partitioned" is not a table, materialized view, or TOAST table +ERROR: relation "test_partitioned" does not have a visibility map +DETAIL: "test_partitioned" is a foreign table. select pg_visibility_map_summary('test_partitioned'); -ERROR: "test_partitioned" is not a table, materialized view, or TOAST table +ERROR: relation "test_partitioned" does not have a visibility map +DETAIL: "test_partitioned" is a foreign table. select pg_check_frozen('test_partitioned'); -ERROR: "test_partitioned" is not a table, materialized view, or TOAST table +ERROR: relation "test_partitioned" does not have a visibility map +DETAIL: "test_partitioned" is a foreign table. select pg_truncate_visibility_map('test_partitioned'); -ERROR: "test_partitioned" is not a table, materialized view, or TOAST table +ERROR: relation "test_partitioned" does not have a visibility map +DETAIL: "test_partitioned" is a foreign table. create table test_partition partition of test_partitioned for values in (1); create index test_index on test_partition (a); -- indexes do not, so these all fail select pg_visibility('test_index', 0); -ERROR: "test_index" is not a table, materialized view, or TOAST table +ERROR: relation "test_index" does not have a visibility map +DETAIL: "test_index" is an index. select pg_visibility_map('test_index'); -ERROR: "test_index" is not a table, materialized view, or TOAST table +ERROR: relation "test_index" does not have a visibility map +DETAIL: "test_index" is an index. select pg_visibility_map_summary('test_index'); -ERROR: "test_index" is not a table, materialized view, or TOAST table +ERROR: relation "test_index" does not have a visibility map +DETAIL: "test_index" is an index. select pg_check_frozen('test_index'); -ERROR: "test_index" is not a table, materialized view, or TOAST table +ERROR: relation "test_index" does not have a visibility map +DETAIL: "test_index" is an index. select pg_truncate_visibility_map('test_index'); -ERROR: "test_index" is not a table, materialized view, or TOAST table +ERROR: relation "test_index" does not have a visibility map +DETAIL: "test_index" is an index. create view test_view as select 1; -- views do not have VMs, so these all fail select pg_visibility('test_view', 0); -ERROR: "test_view" is not a table, materialized view, or TOAST table +ERROR: relation "test_view" does not have a visibility map +DETAIL: "test_view" is a view. select pg_visibility_map('test_view'); -ERROR: "test_view" is not a table, materialized view, or TOAST table +ERROR: relation "test_view" does not have a visibility map +DETAIL: "test_view" is a view. select pg_visibility_map_summary('test_view'); -ERROR: "test_view" is not a table, materialized view, or TOAST table +ERROR: relation "test_view" does not have a visibility map +DETAIL: "test_view" is a view. select pg_check_frozen('test_view'); -ERROR: "test_view" is not a table, materialized view, or TOAST table +ERROR: relation "test_view" does not have a visibility map +DETAIL: "test_view" is a view. select pg_truncate_visibility_map('test_view'); -ERROR: "test_view" is not a table, materialized view, or TOAST table +ERROR: relation "test_view" does not have a visibility map +DETAIL: "test_view" is a view. create sequence test_sequence; -- sequences do not have VMs, so these all fail select pg_visibility('test_sequence', 0); -ERROR: "test_sequence" is not a table, materialized view, or TOAST table +ERROR: relation "test_sequence" does not have a visibility map +DETAIL: "test_sequence" is a sequence. select pg_visibility_map('test_sequence'); -ERROR: "test_sequence" is not a table, materialized view, or TOAST table +ERROR: relation "test_sequence" does not have a visibility map +DETAIL: "test_sequence" is a sequence. select pg_visibility_map_summary('test_sequence'); -ERROR: "test_sequence" is not a table, materialized view, or TOAST table +ERROR: relation "test_sequence" does not have a visibility map +DETAIL: "test_sequence" is a sequence. select pg_check_frozen('test_sequence'); -ERROR: "test_sequence" is not a table, materialized view, or TOAST table +ERROR: relation "test_sequence" does not have a visibility map +DETAIL: "test_sequence" is a sequence. select pg_truncate_visibility_map('test_sequence'); -ERROR: "test_sequence" is not a table, materialized view, or TOAST table +ERROR: relation "test_sequence" does not have a visibility map +DETAIL: "test_sequence" is a sequence. create foreign data wrapper dummy; create server dummy_server foreign data wrapper dummy; create foreign table test_foreign_table () server dummy_server; -- foreign tables do not have VMs, so these all fail select pg_visibility('test_foreign_table', 0); -ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table +ERROR: relation "test_foreign_table" does not have a visibility map +DETAIL: "test_foreign_table" is a foreign table. select pg_visibility_map('test_foreign_table'); -ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table +ERROR: relation "test_foreign_table" does not have a visibility map +DETAIL: "test_foreign_table" is a foreign table. select pg_visibility_map_summary('test_foreign_table'); -ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table +ERROR: relation "test_foreign_table" does not have a visibility map +DETAIL: "test_foreign_table" is a foreign table. select pg_check_frozen('test_foreign_table'); -ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table +ERROR: relation "test_foreign_table" does not have a visibility map +DETAIL: "test_foreign_table" is a foreign table. select pg_truncate_visibility_map('test_foreign_table'); -ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table +ERROR: relation "test_foreign_table" does not have a visibility map +DETAIL: "test_foreign_table" is a foreign table. -- check some of the allowed relkinds create table regular_table (a int); insert into regular_table values (1), (2); diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c index 0cd1160ceb..2084bb80f8 100644 --- a/contrib/pg_visibility/pg_visibility.c +++ b/contrib/pg_visibility/pg_visibility.c @@ -783,6 +783,7 @@ check_relation_relkind(Relation rel) rel->rd_rel->relkind != RELKIND_TOASTVALUE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table, materialized view, or TOAST table", - RelationGetRelationName(rel)))); + errmsg("relation \"%s\" does not have a visibility map", + RelationGetRelationName(rel)), + errdetail_relkind(RelationGetRelationName(rel), rel->rd_rel->relkind))); } diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.out index 9920dbfd40..b66cccec28 100644 --- a/contrib/pgstattuple/expected/pgstattuple.out +++ b/contrib/pgstattuple/expected/pgstattuple.out @@ -159,9 +159,11 @@ ERROR: "test_partitioned" (partitioned table) is not supported select pgstattuple('test_partitioned_index'); ERROR: "test_partitioned_index" (partitioned index) is not supported select pgstattuple_approx('test_partitioned'); -ERROR: "test_partitioned" is not a table or materialized view +ERROR: relation "test_partitioned" is of unsupported kind +DETAIL: "test_partitioned" is a foreign table. select pg_relpages('test_partitioned'); -ERROR: "test_partitioned" is not a table, index, materialized view, sequence, or TOAST table +ERROR: relation "test_partitioned" does not have storage +DETAIL: "test_partitioned" is a foreign table. select pgstatindex('test_partitioned'); ERROR: relation "test_partitioned" is not a btree index select pgstatginindex('test_partitioned'); @@ -173,9 +175,11 @@ create view test_view as select 1; select pgstattuple('test_view'); ERROR: "test_view" (view) is not supported select pgstattuple_approx('test_view'); -ERROR: "test_view" is not a table or materialized view +ERROR: relation "test_view" is of unsupported kind +DETAIL: "test_view" is a view. select pg_relpages('test_view'); -ERROR: "test_view" is not a table, index, materialized view, sequence, or TOAST table +ERROR: relation "test_view" does not have storage +DETAIL: "test_view" is a view. select pgstatindex('test_view'); ERROR: relation "test_view" is not a btree index select pgstatginindex('test_view'); @@ -189,9 +193,11 @@ create foreign table test_foreign_table () server dummy_server; select pgstattuple('test_foreign_table'); ERROR: "test_foreign_table" (foreign table) is not supported select pgstattuple_approx('test_foreign_table'); -ERROR: "test_foreign_table" is not a table or materialized view +ERROR: relation "test_foreign_table" is of unsupported kind +DETAIL: "test_foreign_table" is a foreign table. select pg_relpages('test_foreign_table'); -ERROR: "test_foreign_table" is not a table, index, materialized view, sequence, or TOAST table +ERROR: relation "test_foreign_table" does not have storage +DETAIL: "test_foreign_table" is a foreign table. select pgstatindex('test_foreign_table'); ERROR: relation "test_foreign_table" is not a btree index select pgstatginindex('test_foreign_table'); diff --git a/contrib/pgstattuple/pgstatapprox.c b/contrib/pgstattuple/pgstatapprox.c index 96d837485f..9c5a62d834 100644 --- a/contrib/pgstattuple/pgstatapprox.c +++ b/contrib/pgstattuple/pgstatapprox.c @@ -286,8 +286,9 @@ pgstattuple_approx_internal(Oid relid, FunctionCallInfo fcinfo) rel->rd_rel->relkind == RELKIND_MATVIEW)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("\"%s\" is not a table or materialized view", - RelationGetRelationName(rel)))); + errmsg("relation \"%s\" is of unsupported kind", + RelationGetRelationName(rel)), + errdetail_relkind(RelationGetRelationName(rel), rel->rd_rel->relkind))); if (rel->rd_rel->relam != HEAP_TABLE_AM_OID) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c index b1ce0d77d7..1266862494 100644 --- a/contrib/pgstattuple/pgstatindex.c +++ b/contrib/pgstattuple/pgstatindex.c @@ -758,13 +758,10 @@ GetHashPageStats(Page page, HashIndexStat *stats) static void check_relation_relkind(Relation rel) { - if (rel->rd_rel->relkind != RELKIND_RELATION && - rel->rd_rel->relkind != RELKIND_INDEX && - rel->rd_rel->relkind != RELKIND_MATVIEW && - rel->rd_rel->relkind != RELKIND_SEQUENCE && - rel->rd_rel->relkind != RELKIND_TOASTVALUE) + if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind)) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table, index, materialized view, sequence, or TOAST table", - RelationGetRelationName(rel)))); + errmsg("relation \"%s\" does not have storage", + RelationGetRelationName(rel)), + errdetail_relkind(RelationGetRelationName(rel), rel->rd_rel->relkind))); } diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile index 9499bb33e5..cd500ff2c6 100644 --- a/src/backend/catalog/Makefile +++ b/src/backend/catalog/Makefile @@ -26,6 +26,7 @@ OBJS = \ partition.o \ pg_aggregate.o \ pg_cast.o \ + pg_class.o \ pg_collation.o \ pg_constraint.o \ pg_conversion.o \ diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 632c058b80..a8ebea7e63 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1943,13 +1943,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 && - rel->rd_rel->relkind != RELKIND_FOREIGN_TABLE && - rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) - { + if (RELKIND_HAS_STORAGE(rel->rd_rel->relkind)) RelationDropStorage(rel); - } /* * Close relcache entry, but *keep* AccessExclusiveLock on the relation diff --git a/src/backend/catalog/pg_class.c b/src/backend/catalog/pg_class.c new file mode 100644 index 0000000000..62fb5fd18f --- /dev/null +++ b/src/backend/catalog/pg_class.c @@ -0,0 +1,51 @@ +/*------------------------------------------------------------------------- + * + * pg_class.c + * routines to support manipulation of the pg_class relation + * + * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * + * IDENTIFICATION + * src/backend/catalog/pg_class.c + * + *------------------------------------------------------------------------- + */ +#include "postgres.h" + +#include "catalog/pg_class.h" + +/* + * Issue an errdetail() informing what relkind the given relname is. + */ +int +errdetail_relkind(const char *relname, char relkind) +{ + switch (relkind) + { + case RELKIND_RELATION: + return errdetail("\"%s\" is a table.", relname); + case RELKIND_INDEX: + return errdetail("\"%s\" is an index.", relname); + case RELKIND_SEQUENCE: + return errdetail("\"%s\" is a sequence.", relname); + case RELKIND_TOASTVALUE: + return errdetail("\"%s\" is a TOAST table.", relname); + case RELKIND_VIEW: + return errdetail("\"%s\" is a view.", relname); + case RELKIND_MATVIEW: + return errdetail("\"%s\" is a materialized view.", relname); + case RELKIND_COMPOSITE_TYPE: + return errdetail("\"%s\" is a composite type.", relname); + case RELKIND_FOREIGN_TABLE: + return errdetail("\"%s\" is a foreign table.", relname); + case RELKIND_PARTITIONED_TABLE: + return errdetail("\"%s\" is a foreign table.", relname); + case RELKIND_PARTITIONED_INDEX: + return errdetail("\"%s\" is a partitioned index.", relname); + default: + elog(ERROR, "unrecognized relkind: '%c'", relkind); + return 0; + } +} diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c index 3f7ab8d389..e8e17eae95 100644 --- a/src/backend/catalog/toasting.c +++ b/src/backend/catalog/toasting.c @@ -101,10 +101,8 @@ BootstrapToastTable(char *relName, Oid toastOid, Oid toastIndexOid) if (rel->rd_rel->relkind != RELKIND_RELATION && rel->rd_rel->relkind != RELKIND_MATVIEW) - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table or materialized view", - relName))); + elog(ERROR, "\"%s\" is not a table or materialized view", + relName); /* create_toast_table does all the work */ if (!create_toast_table(rel, toastOid, toastIndexOid, (Datum) 0, diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c index 0ff9ca9f2c..252bca7eab 100644 --- a/src/backend/commands/comment.c +++ b/src/backend/commands/comment.c @@ -98,8 +98,9 @@ CommentObject(CommentStmt *stmt) relation->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table, view, materialized view, composite type, or foreign table", - RelationGetRelationName(relation)))); + errmsg("relation \"%s\" does not support comments", + RelationGetRelationName(relation)), + errdetail_relkind(RelationGetRelationName(relation), relation->rd_rel->relkind))); break; default: break; diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 2baca12c5f..b34a8f186b 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -622,22 +622,12 @@ DefineIndex(Oid relationId, case RELKIND_PARTITIONED_TABLE: /* OK */ break; - case RELKIND_FOREIGN_TABLE: - - /* - * Custom error message for FOREIGN TABLE since the term is close - * to a regular table and can confuse the user. - */ - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("cannot create index on foreign table \"%s\"", - RelationGetRelationName(rel)))); - break; default: ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table or materialized view", - RelationGetRelationName(rel)))); + errmsg("cannot create index on relation \"%s\"", + RelationGetRelationName(rel)), + errdetail_relkind(RelationGetRelationName(rel), rel->rd_rel->relkind))); break; } diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c index d8cafc42bb..a1bc35d987 100644 --- a/src/backend/commands/lockcmds.c +++ b/src/backend/commands/lockcmds.c @@ -88,8 +88,9 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid, relkind != RELKIND_VIEW) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table or view", - rv->relname))); + errmsg("cannot lock relation \"%s\"", + rv->relname), + errdetail_relkind(rv->relname, relkind))); /* * Make note if a temporary relation has been accessed in this diff --git a/src/backend/commands/seclabel.c b/src/backend/commands/seclabel.c index b497c06f1a..e796ff2e91 100644 --- a/src/backend/commands/seclabel.c +++ b/src/backend/commands/seclabel.c @@ -114,8 +114,9 @@ ExecSecLabelStmt(SecLabelStmt *stmt) relation->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table, view, materialized view, composite type, or foreign table", - RelationGetRelationName(relation)))); + errmsg("cannot set security label on relation \"%s\"", + RelationGetRelationName(relation)), + errdetail_relkind(RelationGetRelationName(relation), relation->rd_rel->relkind))); break; default: break; diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index 6aab73bfd4..8ece64b3a6 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -1681,8 +1681,9 @@ process_owned_by(Relation seqrel, List *owned_by, bool for_identity) tablerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("referenced relation \"%s\" is not a table or foreign table", - RelationGetRelationName(tablerel)))); + errmsg("referenced relation \"%s\" must be a table or foreign table", + RelationGetRelationName(tablerel)), + errdetail_relkind(RelationGetRelationName(tablerel), tablerel->rd_rel->relkind))); /* We insist on same owner and schema */ if (seqrel->rd_rel->relowner != tablerel->rd_rel->relowner) diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c index 988cdba6f5..6f89f9d0c4 100644 --- a/src/backend/commands/statscmds.c +++ b/src/backend/commands/statscmds.c @@ -128,8 +128,9 @@ CreateStatistics(CreateStatsStmt *stmt) rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("relation \"%s\" is not a table, foreign table, or materialized view", - RelationGetRelationName(rel)))); + errmsg("cannot define statistics for relation \"%s\"", + RelationGetRelationName(rel)), + errdetail_relkind(RelationGetRelationName(rel), rel->rd_rel->relkind))); /* You must own the relation to create stats on it */ if (!pg_class_ownercheck(RelationGetRelid(rel), stxowner)) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 037d457c3d..6452137738 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -368,7 +368,6 @@ static void ATRewriteTables(AlterTableStmt *parsetree, static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode); static AlteredTableInfo *ATGetQueueEntry(List **wqueue, Relation rel); static void ATSimplePermissions(Relation rel, int allowed_targets); -static void ATWrongRelkindError(Relation rel, int allowed_targets); static void ATSimpleRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode, AlterTableUtilityContext *context); @@ -2953,8 +2952,9 @@ renameatt_check(Oid myrelid, Form_pg_class classform, bool recursing) relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table, view, materialized view, composite type, index, or foreign table", - NameStr(classform->relname)))); + errmsg("cannot rename columns of relation \"%s\"", + NameStr(classform->relname)), + errdetail_relkind(NameStr(classform->relname), relkind))); /* * permissions checking. only the owner of a class can change its schema. @@ -5457,7 +5457,11 @@ ATSimplePermissions(Relation rel, int allowed_targets) /* Wrong target type? */ if ((actual_target & allowed_targets) == 0) - ATWrongRelkindError(rel, allowed_targets); + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("action cannot be performed on relation \"%s\"", + RelationGetRelationName(rel)), + errdetail_relkind(RelationGetRelationName(rel), rel->rd_rel->relkind))); /* Permissions checks */ if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId())) @@ -5471,66 +5475,6 @@ ATSimplePermissions(Relation rel, int allowed_targets) RelationGetRelationName(rel)))); } -/* - * ATWrongRelkindError - * - * Throw an error when a relation has been determined to be of the wrong - * type. - */ -static void -ATWrongRelkindError(Relation rel, int allowed_targets) -{ - char *msg; - - switch (allowed_targets) - { - case ATT_TABLE: - msg = _("\"%s\" is not a table"); - break; - case ATT_TABLE | ATT_VIEW: - msg = _("\"%s\" is not a table or view"); - break; - case ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE: - msg = _("\"%s\" is not a table, view, or foreign table"); - break; - case ATT_TABLE | ATT_VIEW | ATT_MATVIEW | ATT_INDEX: - msg = _("\"%s\" is not a table, view, materialized view, or index"); - break; - case ATT_TABLE | ATT_MATVIEW: - msg = _("\"%s\" is not a table or materialized view"); - break; - case ATT_TABLE | ATT_MATVIEW | ATT_INDEX: - msg = _("\"%s\" is not a table, materialized view, or index"); - break; - case ATT_TABLE | ATT_MATVIEW | ATT_FOREIGN_TABLE: - msg = _("\"%s\" is not a table, materialized view, or foreign table"); - break; - case ATT_TABLE | ATT_FOREIGN_TABLE: - msg = _("\"%s\" is not a table or foreign table"); - break; - case ATT_TABLE | ATT_COMPOSITE_TYPE | ATT_FOREIGN_TABLE: - msg = _("\"%s\" is not a table, composite type, or foreign table"); - break; - case ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_FOREIGN_TABLE: - msg = _("\"%s\" is not a table, materialized view, index, or foreign table"); - break; - case ATT_VIEW: - msg = _("\"%s\" is not a view"); - break; - case ATT_FOREIGN_TABLE: - msg = _("\"%s\" is not a foreign table"); - break; - default: - /* shouldn't get here, add all necessary cases above */ - msg = _("\"%s\" is of the wrong type"); - break; - } - - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg(msg, RelationGetRelationName(rel)))); -} - /* * ATSimpleRecursion * @@ -12372,8 +12316,9 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lock default: ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table, view, sequence, or foreign table", - NameStr(tuple_class->relname)))); + errmsg("cannot change owner of relation \"%s\"", + NameStr(tuple_class->relname)), + errdetail_relkind(NameStr(tuple_class->relname), tuple_class->relkind))); } /* @@ -12789,8 +12734,9 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation, default: ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table, view, materialized view, index, or TOAST table", - RelationGetRelationName(rel)))); + errmsg("cannot set relation options of relation \"%s\"", + RelationGetRelationName(rel)), + errdetail_relkind(RelationGetRelationName(rel), rel->rd_rel->relkind))); break; } @@ -15565,8 +15511,10 @@ RangeVarCallbackForAlterRelation(const RangeVar *rv, Oid relid, Oid oldrelid, relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table, view, materialized view, sequence, or foreign table", - rv->relname))); + errmsg("cannot change schema of relation \"%s\"", + rv->relname), + (relkind == RELKIND_INDEX || relkind == RELKIND_PARTITIONED_INDEX ? errhint("Change the schema of the table instead.") : + (relkind == RELKIND_COMPOSITE_TYPE ? errhint("Use ALTER TYPE instead.") : 0)))); ReleaseSysCache(tuple); } diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index ed551ab73a..a4c881ae21 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -292,8 +292,9 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, else ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table or view", - RelationGetRelationName(rel)))); + errmsg("cannot create trigger on relation \"%s\"", + RelationGetRelationName(rel)), + errdetail_relkind(RelationGetRelationName(rel), rel->rd_rel->relkind))); if (!allowSystemTableMods && IsSystemRelation(rel)) ereport(ERROR, @@ -1197,8 +1198,9 @@ RemoveTriggerById(Oid trigOid) rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table, view, or foreign table", - RelationGetRelationName(rel)))); + errmsg("relation \"%s\" cannot have triggers", + RelationGetRelationName(rel)), + errdetail_relkind(RelationGetRelationName(rel), rel->rd_rel->relkind))); if (!allowSystemTableMods && IsSystemRelation(rel)) ereport(ERROR, @@ -1303,8 +1305,9 @@ RangeVarCallbackForRenameTrigger(const RangeVar *rv, Oid relid, Oid oldrelid, form->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table, view, or foreign table", - rv->relname))); + errmsg("relation \"%s\" cannot have triggers", + rv->relname), + errdetail_relkind(rv->relname, form->relkind))); /* you must own the table to rename one of its triggers */ if (!pg_class_ownercheck(relid, GetUserId())) diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 75c122fe34..495bda793e 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -952,8 +952,9 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla relation->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table, view, materialized view, composite type, or foreign table", - RelationGetRelationName(relation)))); + errmsg("relation \"%s\" is invalid in LIKE clause", + RelationGetRelationName(relation)), + errdetail_relkind(RelationGetRelationName(relation), relation->rd_rel->relkind))); cancel_parser_errposition_callback(&pcbstate); diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index 50eea2e8a8..4997d764e4 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -1801,11 +1801,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_MATVIEW || - relkind == RELKIND_INDEX || - relkind == RELKIND_TOASTVALUE || - relkind == RELKIND_SEQUENCE)) + if (!RELKIND_HAS_STORAGE(relkind)) { rel->pgstat_info = NULL; return; diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c index afc78b3316..40ebacbd8c 100644 --- a/src/backend/rewrite/rewriteDefine.c +++ b/src/backend/rewrite/rewriteDefine.c @@ -268,8 +268,9 @@ DefineQueryRewrite(const char *rulename, event_relation->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table or view", - RelationGetRelationName(event_relation)))); + errmsg("cannot create rule on relation \"%s\"", + RelationGetRelationName(event_relation)), + errdetail_relkind(RelationGetRelationName(event_relation), event_relation->rd_rel->relkind))); if (!allowSystemTableMods && IsSystemRelation(event_relation)) ereport(ERROR, @@ -925,7 +926,8 @@ RangeVarCallbackForRenameRule(const RangeVar *rv, Oid relid, Oid oldrelid, form->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table or view", rv->relname))); + errmsg("relation \"%s\" cannot have rules", rv->relname), + errdetail_relkind(rv->relname, form->relkind))); if (!allowSystemTableMods && IsSystemClass(relid, form)) ereport(ERROR, diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c index 840664429e..2320c06a9b 100644 --- a/src/backend/utils/adt/dbsize.c +++ b/src/backend/utils/adt/dbsize.c @@ -874,25 +874,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_MATVIEW: - 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); @@ -951,38 +944,30 @@ pg_relation_filepath(PG_FUNCTION_ARGS) PG_RETURN_NULL(); relform = (Form_pg_class) GETSTRUCT(tuple); - switch (relform->relkind) + if (RELKIND_HAS_STORAGE(relform->relkind)) + { + /* 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 { - case RELKIND_RELATION: - case RELKIND_MATVIEW: - 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; /* some compilers generate warnings without these next two lines */ rnode.dbNode = InvalidOid; rnode.spcNode = InvalidOid; - break; } if (!OidIsValid(rnode.relNode)) diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h index 78b33b2a7f..44dccd9ccb 100644 --- a/src/include/catalog/pg_class.h +++ b/src/include/catalog/pg_class.h @@ -194,6 +194,7 @@ typedef FormData_pg_class *Form_pg_class; (relkind) == RELKIND_TOASTVALUE || \ (relkind) == RELKIND_MATVIEW) +extern int errdetail_relkind(const char *relname, char relkind); #endif /* EXPOSE_TO_CLIENT_CODE */ diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index f343f9b42e..69551e57fa 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1067,9 +1067,11 @@ ERROR: column "bar" of relation "atacc1" does not exist -- 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 or foreign table +ERROR: action cannot be performed on relation "myview" +DETAIL: "myview" is a view. alter table myview alter column test set not null; -ERROR: "myview" is not a table or foreign table +ERROR: action cannot be performed on relation "myview" +DETAIL: "myview" is a view. drop view myview; drop table atacc1; -- set not null verified by constraints @@ -1367,7 +1369,8 @@ select * from myview; (0 rows) alter table myview drop d; -ERROR: "myview" is not a table, composite type, or foreign table +ERROR: action cannot be performed on relation "myview" +DETAIL: "myview" is a view. 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/create_table_like.out b/src/test/regress/expected/create_table_like.out index 655e8e41dd..395fac01dd 100644 --- a/src/test/regress/expected/create_table_like.out +++ b/src/test/regress/expected/create_table_like.out @@ -409,9 +409,10 @@ DROP TABLE noinh_con_copy, noinh_con_copy1; CREATE TABLE ctlt4 (a int, b text); CREATE SEQUENCE ctlseq1; CREATE TABLE ctlt10 (LIKE ctlseq1); -- fail -ERROR: "ctlseq1" is not a table, view, materialized view, composite type, or foreign table +ERROR: relation "ctlseq1" is invalid in LIKE clause LINE 1: CREATE TABLE ctlt10 (LIKE ctlseq1); ^ +DETAIL: "ctlseq1" is a sequence. CREATE VIEW ctlv1 AS SELECT * FROM ctlt4; CREATE TABLE ctlt11 (LIKE ctlv1); CREATE TABLE ctlt11a (LIKE ctlv1 INCLUDING ALL); diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out index b9e25820bc..aaebc2be8d 100644 --- a/src/test/regress/expected/foreign_data.out +++ b/src/test/regress/expected/foreign_data.out @@ -740,7 +740,8 @@ FDW options: (delimiter ',', quote '"', "be quoted" 'value') (1 row) CREATE INDEX id_ft1_c2 ON ft1 (c2); -- ERROR -ERROR: cannot create index on foreign table "ft1" +ERROR: cannot create index on relation "ft1" +DETAIL: "ft1" is a foreign table. SELECT * FROM ft1; -- ERROR ERROR: foreign-data wrapper "dummy" has no handler EXPLAIN SELECT * FROM ft1; -- ERROR @@ -864,7 +865,8 @@ LINE 1: ALTER FOREIGN TABLE ft1 ADD PRIMARY KEY (c7); ^ ALTER FOREIGN TABLE ft1 ADD CONSTRAINT ft1_c9_check CHECK (c9 < 0) NOT VALID; ALTER FOREIGN TABLE ft1 ALTER CONSTRAINT ft1_c9_check DEFERRABLE; -- ERROR -ERROR: "ft1" is not a table +ERROR: action cannot be performed on relation "ft1" +DETAIL: "ft1" is a foreign table. ALTER FOREIGN TABLE ft1 DROP CONSTRAINT ft1_c9_check; ALTER FOREIGN TABLE ft1 DROP CONSTRAINT no_const; -- ERROR ERROR: constraint "no_const" of relation "ft1" does not exist diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out index f78865ef81..beeae10387 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -137,7 +137,8 @@ select relname, relpartbound from pg_class (2 rows) alter table idxpart_c detach partition idxpart1_c; -ERROR: "idxpart_c" is not a table +ERROR: action cannot be performed on relation "idxpart_c" +DETAIL: "idxpart_c" is a partitioned index. drop table idxpart; -- If a partition already has an index, don't create a duplicative one create table idxpart (a int, b int) partition by range (a, b); diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out index 8b928b2888..dad8086707 100644 --- a/src/test/regress/expected/sequence.out +++ b/src/test/regress/expected/sequence.out @@ -21,7 +21,8 @@ CREATE SEQUENCE sequence_testx OWNED BY nobody; -- nonsense word ERROR: invalid OWNED BY option HINT: Specify OWNED BY table.column or OWNED BY NONE. CREATE SEQUENCE sequence_testx OWNED BY pg_class_oid_index.oid; -- not a table -ERROR: referenced relation "pg_class_oid_index" is not a table or foreign table +ERROR: referenced relation "pg_class_oid_index" must be a table or foreign table +DETAIL: "pg_class_oid_index" is an index. CREATE SEQUENCE sequence_testx OWNED BY pg_class.relname; -- not same schema ERROR: sequence must be in same schema as table it is linked to CREATE TABLE sequence_test_table (a int); diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out index 0ae779a3b9..c56d0daa2b 100644 --- a/src/test/regress/expected/stats_ext.out +++ b/src/test/regress/expected/stats_ext.out @@ -145,14 +145,18 @@ CREATE TABLE tststats.pt (a int, b int, c text) PARTITION BY RANGE (a, b); CREATE TABLE tststats.pt1 PARTITION OF tststats.pt FOR VALUES FROM (-10, -10) TO (10, 10); CREATE STATISTICS tststats.s1 ON a, b FROM tststats.t; CREATE STATISTICS tststats.s2 ON a, b FROM tststats.ti; -ERROR: relation "ti" is not a table, foreign table, or materialized view +ERROR: cannot define statistics for relation "ti" +DETAIL: "ti" is an index. CREATE STATISTICS tststats.s3 ON a, b FROM tststats.s; -ERROR: relation "s" is not a table, foreign table, or materialized view +ERROR: cannot define statistics for relation "s" +DETAIL: "s" is a sequence. CREATE STATISTICS tststats.s4 ON a, b FROM tststats.v; -ERROR: relation "v" is not a table, foreign table, or materialized view +ERROR: cannot define statistics for relation "v" +DETAIL: "v" is a view. CREATE STATISTICS tststats.s5 ON a, b FROM tststats.mv; CREATE STATISTICS tststats.s6 ON a, b FROM tststats.ty; -ERROR: relation "ty" is not a table, foreign table, or materialized view +ERROR: cannot define statistics for relation "ty" +DETAIL: "ty" is a composite type. CREATE STATISTICS tststats.s7 ON a, b FROM tststats.f; CREATE STATISTICS tststats.s8 ON a, b FROM tststats.pt; CREATE STATISTICS tststats.s9 ON a, b FROM tststats.pt1; -- 2.26.0