From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: wrong relkind error messages
Date: Mon, 13 Apr 2020 15:54:48 +0200
Message-ID: <dc35a398-37d0-75ce-07ea-1dd71d98f8ec@2ndquadrant.com> (raw)
I'm not a fan of error messages like
relation "%s" is not a table, foreign table, or materialized view
It doesn't tell me what's wrong, it only tells me what else could have
worked. It's also tedious to maintain and the number of combinations
grows over time.
This was discussed many years ago in [0], with the same arguments, and
there appeared to have been general agreement to change this, but then
the thread stalled somehow on some technical details.
Attached is another attempt to improve this. I have rewritten the
primary error messages using the principle of "cannot do this with that"
and then added a detail message to show what relkind the object has.
For example:
-ERROR: relation "ti" is not a table, foreign table, or materialized view
+ERROR: cannot define statistics for relation "ti"
+DETAIL: "ti" is an index.
and
-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.
You can see more instances of this in the test diffs in the attached patch.
In passing, I also changed a few places to use the RELKIND_HAS_STORAGE()
macro. This is related because it allows writing more helpful error
messages, such as in pgstatindex.c.
One question on a detail arose:
check_relation_relkind() in pg_visibility.c accepts RELKIND_RELATION,
RELKIND_MATVIEW, and RELKIND_TOASTVALUE, but pgstatapprox.c only accepts
RELKIND_RELATION and RELKIND_MATVIEW, even though they both look for a
visibility map. Is that an intentional omission? If so, it should be
commented better.
[0]:
https://www.postgresql.org/message-id/flat/AANLkTimR_sZ_wKd1cgqVG1PEvTvdr9j7zD%2B3_NPvfaa_%40mail.gm...
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 2e32dce36cd34e6d58c99067969f98de1bb1264b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
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.outindex 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.cindex 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.outindex 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.cindex 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.cindex 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/Makefileindex 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.cindex 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.cnew file mode 100644index 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.hindex 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.outindex 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.outindex 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.outindex 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.outindex 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.outindex 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.outindex 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
Attachments:0001-Improve-error-messages-about-mismatching-relkind.patch (text/plain, 41.3K) download
From 2e32dce36cd34e6d58c99067969f98de1bb1264b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
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.outindex 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.cindex 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.outindex 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.cindex 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.cindex 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/Makefileindex 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.cindex 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.cnew file mode 100644index 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.cindex 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.hindex 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.outindex 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.outindex 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.outindex 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.outindex 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.outindex 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.outindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter.eisentraut@2ndquadrant.com
Subject: Re: wrong relkind error messages
In-Reply-To: <dc35a398-37d0-75ce-07ea-1dd71d98f8ec@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox