wrong relkind error messages
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]/messages/by-id/AANLkTimR_sZ_wKd1cgqVG1PEvTvdr9j7zD+3_NPvfaa_@mail.gmail.com, 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]: /messages/by-id/AANLkTimR_sZ_wKd1cgqVG1PEvTvdr9j7zD+3_NPvfaa_@mail.gmail.com
/messages/by-id/AANLkTimR_sZ_wKd1cgqVG1PEvTvdr9j7zD+3_NPvfaa_@mail.gmail.com
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Improve-error-messages-about-mismatching-relkind.patchtext/plain; charset=UTF-8; name=0001-Improve-error-messages-about-mismatching-relkind.patch; x-mac-creator=0; x-mac-type=0Download
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.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
On Mon, Apr 13, 2020 at 9:55 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
Attached is another attempt to improve this.
Nice effort. Most of these seem like clear improvements, but some I don't like:
+ errmsg("relation \"%s\" is of unsupported kind",
+ RelationGetRelationName(rel)),
+ errdetail_relkind(RelationGetRelationName(rel), rel->rd_rel->relkind)));
It would help to work "pgstattuple" into the message somehow. "cannot
use pgstattuple on relation \"%s\"", perhaps?
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("action cannot be performed on relation \"%s\"",
+ RelationGetRelationName(rel)),
Super-vague.
+ errmsg("cannot set relation options of relation \"%s\"",
+ RelationGetRelationName(rel)),
I suggest "cannot set options for relation \"%s\""; that is, use "for"
instead of "of", and don't say "relation" twice.
+ errmsg("cannot create trigger on relation \"%s\"",
+ RelationGetRelationName(rel)),
+ errmsg("relation \"%s\" cannot have triggers",
+ RelationGetRelationName(rel)),
+ errmsg("relation \"%s\" cannot have triggers",
+ rv->relname),
Maybe use the second wording for all three? And similarly for rules?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
I'm not a fan of error messages like
relation "%s" is not a table, foreign table, or materialized view
Agreed, they're not great.
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.
I see where you'e going, and it seems like a generally-better idea,
but I feel like this phrasing is omitting some critical background
information that users don't necessarily have. At the very least
it's not stating clearly that the failure is *because* ti is an
index. More generally, the whole concept that statistics can only
be defined for certain kinds of relations has disappeared from view.
I fear that users who're less deeply into Postgres hacking than we
are might not have that concept at all, or at least it might not
come to mind immediately when they get this message.
Fixing this while avoiding your concern about proliferation of messages
seems a bit difficult though. The best I can do after a couple minutes'
thought is
ERROR: cannot define statistics for relation "ti"
DETAIL: "ti" is an index, and this operation is not supported for that
kind of relation.
which seems a little long and awkward. Another idea is
ERROR: cannot define statistics for relation "ti"
DETAIL: This operation is not supported for indexes.
which still leaves implicit that "ti" is an index, but probably that's
something the user can figure out.
Maybe someone else can do better?
regards, tom lane
On 4/13/20 11:13 AM, Tom Lane wrote:
ERROR: cannot define statistics for relation "ti"
DETAIL: This operation is not supported for indexes.which still leaves implicit that "ti" is an index, but probably that's
something the user can figure out.
+1 for this. It's clear and succinct.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, Apr 13, 2020 at 11:13:15AM -0400, Tom Lane wrote:
Fixing this while avoiding your concern about proliferation of messages
seems a bit difficult though. The best I can do after a couple minutes'
thought isERROR: cannot define statistics for relation "ti"
DETAIL: "ti" is an index, and this operation is not supported for that
kind of relation.which seems a little long and awkward. Another idea is
ERROR: cannot define statistics for relation "ti"
DETAIL: This operation is not supported for indexes.which still leaves implicit that "ti" is an index, but probably that's
something the user can figure out.Maybe someone else can do better?
"This operation is not supported for put_relkind_here \"%s\"."? I
think that it is better to provide a relation name in the error
message (even optionally a namespace). That's less to guess for the
user.
+int
+errdetail_relkind(const char *relname, char relkind)
+{
+ switch (relkind)
+ {
+ case RELKIND_RELATION:
+ return errdetail("\"%s\" is a table.", relname);
+ case RELKIND_INDEX:
It seems to me that we should optionally add the namespace in the
error message, or just have a separate routine for that. I think that
it would be useful in some cases (see for example the part about the
statistics in the patch), still annoying in some others (instability
in test output for temporary schemas for example) so there is a point
for both in my view.
- 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);
These should be applied separately in my opinion. Nice catch.
- 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))));
This is not great style either and reduces readability, so I would
recommend to split the errhint generation using a switch/case.
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("action cannot be performed on relation
\"%s\"",
+ RelationGetRelationName(rel)),
Echoing Robert upthread, "action" is not really useful for the user,
and it seems to me that it should be reworked as "cannot perform foo
on relation \"hoge\""
+ errmsg("relation \"%s\" does not support comments",
+ RelationGetRelationName(relation)),
This is not project-style as full sentences cannot be used in error
messages, no? The former is not that good either, still, while this
is getting touched... Say, "cannot use COMMENT on relation \"%s\""?
Overall +1 on the patch by the way. Thanks for sending something to
improve the situation
--
Michael
On 2020-Apr-14, Michael Paquier wrote:
On Mon, Apr 13, 2020 at 11:13:15AM -0400, Tom Lane wrote:
ERROR: cannot define statistics for relation "ti"
DETAIL: This operation is not supported for indexes.which still leaves implicit that "ti" is an index, but probably that's
something the user can figure out.Maybe someone else can do better?
"This operation is not supported for put_relkind_here \"%s\"."? I
think that it is better to provide a relation name in the error
message (even optionally a namespace). That's less to guess for the
user.
But the relation name is already in the ERROR line -- why do you care so
much about also having it in the DETAIL? Besides, I think part of the
point Tom was making is that if you say "not supported for the index
foo" is that the user is left wondering whether the operation is not
supported for that particular index only or for any index.
Tom's other proposal
DETAIL: "ti" is an index, and this operation is not supported for that kind of relation.
addresses that problem, but seems excessively verbose.
Also, elsewhere Peter said[1]/messages/by-id/1293803569.19789.6.camel@fsopti579.F-Secure.com that we should not try to list the things
that would be allowed, so it's pointless to try to list the relkinds for
which the operation is permissible.
So I +1 this idea:
ERROR: cannot define statistics for relation "ti"
DETAIL: This operation is not supported for indexes.
[1]: /messages/by-id/1293803569.19789.6.camel@fsopti579.F-Secure.com
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020-Apr-13, Robert Haas wrote:
+ ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("action cannot be performed on relation \"%s\"", + RelationGetRelationName(rel)),Super-vague.
Maybe, but note that the patch proposed to replace this current error
message:
ERROR: foo is not an index or foreign table
with
ERROR: action cannot be performed on "foo"
DETAIL: "foo" is a materialized view.
or, if we're to adopt Tom's proposed wording,
ERROR: cannot perform action on relation "ti"
DETAIL: This operation is not supported for materialized views.
so it's not like this is making things any worse; the error was already
super-vague.
This could be improved if we had stringification of ALTER TABLE
subcommand types:
ERROR: ALTER TABLE ... ADD COLUMN cannot be performed on "foo"
DETAIL: "foo" is a gummy bear.
or
ERROR: ALTER TABLE ... ADD COLUMN cannot be performed on foo
DETAIL: This action cannot be performed on gummy bears.
but that seems material for a different patch.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
On 2020-Apr-13, Robert Haas wrote:
+ ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("action cannot be performed on relation \"%s\"", + RelationGetRelationName(rel)),Super-vague.
Maybe, but note that the patch proposed to replace this current error
message:
ERROR: foo is not an index or foreign table
...
so it's not like this is making things any worse; the error was already
super-vague.
Yeah. I share Robert's feeling that "action" is not really desirable
here, but I have to concur that this is an improvement on the existing
text, which also fails to mention what command is being rejected.
This could be improved if we had stringification of ALTER TABLE
subcommand types:
ERROR: ALTER TABLE ... ADD COLUMN cannot be performed on "foo"
In the meantime could we at least say "ALTER TABLE action cannot
be performed"? The worst aspect of the existing text is that if
an error comes out of a script with a lot of different commands,
it doesn't give you any hint at all about which command failed.
regards, tom lane
On Tue, Apr 14, 2020 at 7:02 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
On 2020-Apr-13, Robert Haas wrote:
+ ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("action cannot be performed on relation \"%s\"", + RelationGetRelationName(rel)),Super-vague.
Maybe, but note that the patch proposed to replace this current error
message:
ERROR: foo is not an index or foreign table
with
ERROR: action cannot be performed on "foo"
DETAIL: "foo" is a materialized view.
Sure, but the point is that this case is not improved nearly as much
as most of the others. In a whole bunch of cases, he made the error
message describe the attempted operation, but here he didn't. I'm not
saying that makes it worse than what we had before, just that it would
be better if we could make this look like the other cases the patch
also changes.
This could be improved if we had stringification of ALTER TABLE
subcommand types:ERROR: ALTER TABLE ... ADD COLUMN cannot be performed on "foo"
DETAIL: "foo" is a gummy bear.
or
ERROR: ALTER TABLE ... ADD COLUMN cannot be performed on foo
DETAIL: This action cannot be performed on gummy bears.but that seems material for a different patch.
Even without that, you could at least say "this form of ALTER TABLE is
not supported for foo" or something like that.
I'm not trying to block the patch. I think it's a good patch. I was
just making an observation about some parts of it where it seems like
we could try slightly harder to do better.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 2020-Apr-15, Robert Haas wrote:
[good arguments]
I don't disagree with anything you said, and I don't have anything to
add for now.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020-04-15 02:15, Tom Lane wrote:
In the meantime could we at least say "ALTER TABLE action cannot
be performed"?
We don't know whether ALTER TABLE was the command. For example, in one
of the affected regression test cases, the command is ALTER VIEW.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 2020-04-15 02:15, Tom Lane wrote:
In the meantime could we at least say "ALTER TABLE action cannot
be performed"?
We don't know whether ALTER TABLE was the command. For example, in one
of the affected regression test cases, the command is ALTER VIEW.
Maybe just "ALTER action cannot be performed"? I share Robert's
dislike of being so vague as to just say "action".
regards, tom lane
On 13.04.20 15:54, Peter Eisentraut wrote:
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.
Another go at this. I believe in the attached patch I have addressed
all the feedback during this thread last year. In particular, I have
rephrased the detail message per discussion, and I have improved the
messages produced by ATSimplePermissions() with more details. Examples:
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: This operation is not supported for indexes.
ALTER FOREIGN TABLE ft1 ALTER CONSTRAINT ft1_c9_check DEFERRABLE; -- ERROR
-ERROR: "ft1" is not a table
+ERROR: ALTER action ALTER CONSTRAINT cannot be performed on relation "ft1"
+DETAIL: This operation is not supported for foreign tables.
There might be room for some wordsmithing in a few places, but generally
I think this is complete.
Attachments:
v2-0001-Improve-error-messages-about-mismatching-relkind.patchtext/plain; charset=UTF-8; name=v2-0001-Improve-error-messages-about-mismatching-relkind.patch; x-mac-creator=0; x-mac-type=0Download
From bc3c11c76427646387718fbbdfb534c56b819e43 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 24 Jun 2021 10:08:03 +0200
Subject: [PATCH v2] 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 possible. Then, in the detail
message, explain that the operation is not supported for the relkind
the object was. To simplify that, add a new function
errdetail_relkind_not_supported() that does this.
In passing, make use of RELKIND_HAS_STORAGE() where appropriate,
instead of listing out the relkinds individually.
---
contrib/amcheck/expected/check_heap.out | 15 +-
contrib/amcheck/verify_heapam.c | 38 +-
contrib/pageinspect/expected/page.out | 6 +-
contrib/pageinspect/rawpage.c | 28 +-
contrib/pg_surgery/expected/heap_surgery.out | 6 +-
contrib/pg_surgery/heap_surgery.c | 54 ++-
.../pg_visibility/expected/pg_visibility.out | 75 ++--
contrib/pg_visibility/pg_visibility.c | 5 +-
contrib/pgstattuple/expected/pgstattuple.out | 30 +-
contrib/pgstattuple/pgstatapprox.c | 5 +-
contrib/pgstattuple/pgstatindex.c | 75 +---
contrib/pgstattuple/pgstattuple.c | 31 +-
src/backend/catalog/Makefile | 1 +
src/backend/catalog/pg_class.c | 52 +++
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 | 356 +++++++++++-------
src/backend/commands/trigger.c | 15 +-
src/backend/parser/parse_utilcmd.c | 5 +-
src/backend/rewrite/rewriteDefine.c | 8 +-
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 +-
32 files changed, 511 insertions(+), 378 deletions(-)
create mode 100644 src/backend/catalog/pg_class.c
diff --git a/contrib/amcheck/expected/check_heap.out b/contrib/amcheck/expected/check_heap.out
index 1fb3823142..ad3086d2aa 100644
--- a/contrib/amcheck/expected/check_heap.out
+++ b/contrib/amcheck/expected/check_heap.out
@@ -139,7 +139,8 @@ CREATE TABLE test_partitioned (a int, b text default repeat('x', 5000))
SELECT * FROM verify_heapam('test_partitioned',
startblock := NULL,
endblock := NULL);
-ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+ERROR: cannot check relation "test_partitioned"
+DETAIL: This operation is not supported for partitioned tables.
-- Check that valid options are not rejected nor corruption reported
-- for an empty partition table (the child one)
CREATE TABLE test_partition partition OF test_partitioned FOR VALUES IN (1);
@@ -165,19 +166,22 @@ CREATE INDEX test_index ON test_partition (a);
SELECT * FROM verify_heapam('test_index',
startblock := NULL,
endblock := NULL);
-ERROR: "test_index" is not a table, materialized view, or TOAST table
+ERROR: cannot check relation "test_index"
+DETAIL: This operation is not supported for indexes.
-- Check that views are rejected
CREATE VIEW test_view AS SELECT 1;
SELECT * FROM verify_heapam('test_view',
startblock := NULL,
endblock := NULL);
-ERROR: "test_view" is not a table, materialized view, or TOAST table
+ERROR: cannot check relation "test_view"
+DETAIL: This operation is not supported for views.
-- Check that sequences are rejected
CREATE SEQUENCE test_sequence;
SELECT * FROM verify_heapam('test_sequence',
startblock := NULL,
endblock := NULL);
-ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+ERROR: cannot check relation "test_sequence"
+DETAIL: This operation is not supported for sequences.
-- Check that foreign tables are rejected
CREATE FOREIGN DATA WRAPPER dummy;
CREATE SERVER dummy_server FOREIGN DATA WRAPPER dummy;
@@ -185,7 +189,8 @@ CREATE FOREIGN TABLE test_foreign_table () SERVER dummy_server;
SELECT * FROM verify_heapam('test_foreign_table',
startblock := NULL,
endblock := NULL);
-ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
+ERROR: cannot check relation "test_foreign_table"
+DETAIL: This operation is not supported for foreign tables.
-- cleanup
DROP TABLE heaptest;
DROP TABLE test_partition;
diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index a3caee7cdd..226271923a 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -147,7 +147,6 @@ typedef struct HeapCheckContext
} HeapCheckContext;
/* Internal implementation */
-static void sanity_check_relation(Relation rel);
static void check_tuple(HeapCheckContext *ctx);
static void check_toast_tuple(HeapTuple toasttup, HeapCheckContext *ctx,
ToastedAttribute *ta, int32 *expected_chunk_seq,
@@ -300,7 +299,23 @@ verify_heapam(PG_FUNCTION_ARGS)
/* Open relation, check relkind and access method */
ctx.rel = relation_open(relid, AccessShareLock);
- sanity_check_relation(ctx.rel);
+
+ /*
+ * Check that a relation's relkind and access method are both supported.
+ */
+ if (ctx.rel->rd_rel->relkind != RELKIND_RELATION &&
+ ctx.rel->rd_rel->relkind != RELKIND_MATVIEW &&
+ ctx.rel->rd_rel->relkind != RELKIND_TOASTVALUE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot check relation \"%s\"",
+ RelationGetRelationName(ctx.rel)),
+ errdetail_relkind_not_supported(ctx.rel->rd_rel->relkind)));
+
+ if (ctx.rel->rd_rel->relam != HEAP_TABLE_AM_OID)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("only heap AM is supported")));
/* Early exit if the relation is empty */
nblocks = RelationGetNumberOfBlocks(ctx.rel);
@@ -523,25 +538,6 @@ verify_heapam(PG_FUNCTION_ARGS)
PG_RETURN_NULL();
}
-/*
- * Check that a relation's relkind and access method are both supported.
- */
-static void
-sanity_check_relation(Relation rel)
-{
- if (rel->rd_rel->relkind != RELKIND_RELATION &&
- rel->rd_rel->relkind != RELKIND_MATVIEW &&
- 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))));
- if (rel->rd_rel->relam != HEAP_TABLE_AM_OID)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("only heap AM is supported")));
-}
-
/*
* Shared internal implementation for report_corruption and
* report_toast_corruption.
diff --git a/contrib/pageinspect/expected/page.out b/contrib/pageinspect/expected/page.out
index 4da28f0a1d..4e325ae56d 100644
--- a/contrib/pageinspect/expected/page.out
+++ b/contrib/pageinspect/expected/page.out
@@ -180,9 +180,11 @@ DROP TABLE test1;
create table test_partitioned (a int) partition by range (a);
create index test_partitioned_index on test_partitioned (a);
select get_raw_page('test_partitioned', 0); -- error about partitioned table
-ERROR: cannot get raw page from partitioned table "test_partitioned"
+ERROR: cannot get raw page from relation "test_partitioned"
+DETAIL: This operation is not supported for partitioned tables.
select get_raw_page('test_partitioned_index', 0); -- error about partitioned index
-ERROR: cannot get raw page from partitioned index "test_partitioned_index"
+ERROR: cannot get raw page from relation "test_partitioned_index"
+DETAIL: This operation is not supported for partitioned indexes.
-- a regular table which is a member of a partition set should work though
create table test_part1 partition of test_partitioned for values from ( 1 ) to (100);
select get_raw_page('test_part1', 0); -- get farther and error about empty table
diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index 7272b21016..e9fee73bc4 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -155,32 +155,12 @@ get_raw_page_internal(text *relname, ForkNumber forknum, BlockNumber blkno)
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
rel = relation_openrv(relrv, AccessShareLock);
- /* Check that this relation has storage */
- if (rel->rd_rel->relkind == RELKIND_VIEW)
+ if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind))
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot get raw page from view \"%s\"",
- RelationGetRelationName(rel))));
- if (rel->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot get raw page from composite type \"%s\"",
- RelationGetRelationName(rel))));
- if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot get raw page from foreign table \"%s\"",
- RelationGetRelationName(rel))));
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot get raw page from partitioned table \"%s\"",
- RelationGetRelationName(rel))));
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot get raw page from partitioned index \"%s\"",
- RelationGetRelationName(rel))));
+ errmsg("cannot get raw page from relation \"%s\"",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
/*
* Reject attempts to read non-local temporary relations; we would be
diff --git a/contrib/pg_surgery/expected/heap_surgery.out b/contrib/pg_surgery/expected/heap_surgery.out
index d4a757ffa0..df7d13b090 100644
--- a/contrib/pg_surgery/expected/heap_surgery.out
+++ b/contrib/pg_surgery/expected/heap_surgery.out
@@ -170,9 +170,11 @@ rollback;
-- check that it fails on an unsupported relkind
create view vw as select 1;
select heap_force_kill('vw'::regclass, ARRAY['(0, 1)']::tid[]);
-ERROR: "vw" is not a table, materialized view, or TOAST table
+ERROR: cannot operate on relation "vw"
+DETAIL: This operation is not supported for views.
select heap_force_freeze('vw'::regclass, ARRAY['(0, 1)']::tid[]);
-ERROR: "vw" is not a table, materialized view, or TOAST table
+ERROR: cannot operate on relation "vw"
+DETAIL: This operation is not supported for views.
-- cleanup.
drop view vw;
drop extension pg_surgery;
diff --git a/contrib/pg_surgery/heap_surgery.c b/contrib/pg_surgery/heap_surgery.c
index d31e5f31fd..7edfe4f326 100644
--- a/contrib/pg_surgery/heap_surgery.c
+++ b/contrib/pg_surgery/heap_surgery.c
@@ -37,7 +37,6 @@ static int32 tidcmp(const void *a, const void *b);
static Datum heap_force_common(FunctionCallInfo fcinfo,
HeapTupleForceOption heap_force_opt);
static void sanity_check_tid_array(ArrayType *ta, int *ntids);
-static void sanity_check_relation(Relation rel);
static BlockNumber find_tids_one_page(ItemPointer tids, int ntids,
OffsetNumber *next_start_ptr);
@@ -101,8 +100,28 @@ heap_force_common(FunctionCallInfo fcinfo, HeapTupleForceOption heap_force_opt)
rel = relation_open(relid, RowExclusiveLock);
- /* Check target relation. */
- sanity_check_relation(rel);
+ /*
+ * Check target relation.
+ */
+ if (rel->rd_rel->relkind != RELKIND_RELATION &&
+ rel->rd_rel->relkind != RELKIND_MATVIEW &&
+ rel->rd_rel->relkind != RELKIND_TOASTVALUE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot operate on relation \"%s\"",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+
+ if (rel->rd_rel->relam != HEAP_TABLE_AM_OID)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("only heap AM is supported")));
+
+ /* Must be owner of the table or superuser. */
+ if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER,
+ get_relkind_objtype(rel->rd_rel->relkind),
+ RelationGetRelationName(rel));
tids = ((ItemPointer) ARR_DATA_PTR(ta));
@@ -363,35 +382,6 @@ sanity_check_tid_array(ArrayType *ta, int *ntids)
*ntids = ArrayGetNItems(ARR_NDIM(ta), ARR_DIMS(ta));
}
-/*-------------------------------------------------------------------------
- * sanity_check_relation()
- *
- * Perform sanity checks on the given relation.
- * ------------------------------------------------------------------------
- */
-static void
-sanity_check_relation(Relation rel)
-{
- if (rel->rd_rel->relkind != RELKIND_RELATION &&
- rel->rd_rel->relkind != RELKIND_MATVIEW &&
- 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))));
-
- if (rel->rd_rel->relam != HEAP_TABLE_AM_OID)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("only heap AM is supported")));
-
- /* Must be owner of the table or superuser. */
- if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
- aclcheck_error(ACLCHECK_NOT_OWNER,
- get_relkind_objtype(rel->rd_rel->relkind),
- RelationGetRelationName(rel));
-}
-
/*-------------------------------------------------------------------------
* find_tids_one_page()
*
diff --git a/contrib/pg_visibility/expected/pg_visibility.out b/contrib/pg_visibility/expected/pg_visibility.out
index 315633bfea..9de54db2a2 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" is of wrong relation kind
+DETAIL: This operation is not supported for partitioned tables.
select pg_visibility_map('test_partitioned');
-ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+ERROR: relation "test_partitioned" is of wrong relation kind
+DETAIL: This operation is not supported for partitioned tables.
select pg_visibility_map_summary('test_partitioned');
-ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+ERROR: relation "test_partitioned" is of wrong relation kind
+DETAIL: This operation is not supported for partitioned tables.
select pg_check_frozen('test_partitioned');
-ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+ERROR: relation "test_partitioned" is of wrong relation kind
+DETAIL: This operation is not supported for partitioned tables.
select pg_truncate_visibility_map('test_partitioned');
-ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+ERROR: relation "test_partitioned" is of wrong relation kind
+DETAIL: This operation is not supported for partitioned tables.
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" is of wrong relation kind
+DETAIL: This operation is not supported for indexes.
select pg_visibility_map('test_index');
-ERROR: "test_index" is not a table, materialized view, or TOAST table
+ERROR: relation "test_index" is of wrong relation kind
+DETAIL: This operation is not supported for indexes.
select pg_visibility_map_summary('test_index');
-ERROR: "test_index" is not a table, materialized view, or TOAST table
+ERROR: relation "test_index" is of wrong relation kind
+DETAIL: This operation is not supported for indexes.
select pg_check_frozen('test_index');
-ERROR: "test_index" is not a table, materialized view, or TOAST table
+ERROR: relation "test_index" is of wrong relation kind
+DETAIL: This operation is not supported for indexes.
select pg_truncate_visibility_map('test_index');
-ERROR: "test_index" is not a table, materialized view, or TOAST table
+ERROR: relation "test_index" is of wrong relation kind
+DETAIL: This operation is not supported for indexes.
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" is of wrong relation kind
+DETAIL: This operation is not supported for views.
select pg_visibility_map('test_view');
-ERROR: "test_view" is not a table, materialized view, or TOAST table
+ERROR: relation "test_view" is of wrong relation kind
+DETAIL: This operation is not supported for views.
select pg_visibility_map_summary('test_view');
-ERROR: "test_view" is not a table, materialized view, or TOAST table
+ERROR: relation "test_view" is of wrong relation kind
+DETAIL: This operation is not supported for views.
select pg_check_frozen('test_view');
-ERROR: "test_view" is not a table, materialized view, or TOAST table
+ERROR: relation "test_view" is of wrong relation kind
+DETAIL: This operation is not supported for views.
select pg_truncate_visibility_map('test_view');
-ERROR: "test_view" is not a table, materialized view, or TOAST table
+ERROR: relation "test_view" is of wrong relation kind
+DETAIL: This operation is not supported for views.
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" is of wrong relation kind
+DETAIL: This operation is not supported for sequences.
select pg_visibility_map('test_sequence');
-ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+ERROR: relation "test_sequence" is of wrong relation kind
+DETAIL: This operation is not supported for sequences.
select pg_visibility_map_summary('test_sequence');
-ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+ERROR: relation "test_sequence" is of wrong relation kind
+DETAIL: This operation is not supported for sequences.
select pg_check_frozen('test_sequence');
-ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+ERROR: relation "test_sequence" is of wrong relation kind
+DETAIL: This operation is not supported for sequences.
select pg_truncate_visibility_map('test_sequence');
-ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+ERROR: relation "test_sequence" is of wrong relation kind
+DETAIL: This operation is not supported for sequences.
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" is of wrong relation kind
+DETAIL: This operation is not supported for foreign tables.
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" is of wrong relation kind
+DETAIL: This operation is not supported for foreign tables.
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" is of wrong relation kind
+DETAIL: This operation is not supported for foreign tables.
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" is of wrong relation kind
+DETAIL: This operation is not supported for foreign tables.
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" is of wrong relation kind
+DETAIL: This operation is not supported for foreign tables.
-- check some of the allowed relkinds
create table regular_table (a int, b text);
alter table regular_table alter column b set storage external;
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index dd0c124e62..c6d983183d 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -781,6 +781,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\" is of wrong relation kind",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
}
diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.out
index 40f7825ddb..e4ac86f9e3 100644
--- a/contrib/pgstattuple/expected/pgstattuple.out
+++ b/contrib/pgstattuple/expected/pgstattuple.out
@@ -155,13 +155,17 @@ create table test_partitioned (a int) partition by range (a);
create index test_partitioned_index on test_partitioned(a);
-- these should all fail
select pgstattuple('test_partitioned');
-ERROR: "test_partitioned" (partitioned table) is not supported
+ERROR: cannot get tuple-level statistics for relation "test_partitioned"
+DETAIL: This operation is not supported for partitioned tables.
select pgstattuple('test_partitioned_index');
-ERROR: "test_partitioned_index" (partitioned index) is not supported
+ERROR: cannot get tuple-level statistics for relation "test_partitioned_index"
+DETAIL: This operation is not supported for partitioned indexes.
select pgstattuple_approx('test_partitioned');
-ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+ERROR: relation "test_partitioned" is of wrong relation kind
+DETAIL: This operation is not supported for partitioned tables.
select pg_relpages('test_partitioned');
-ERROR: "test_partitioned" is not a table, index, materialized view, sequence, or TOAST table
+ERROR: cannot get page count of relation "test_partitioned"
+DETAIL: This operation is not supported for partitioned tables.
select pgstatindex('test_partitioned');
ERROR: relation "test_partitioned" is not a btree index
select pgstatginindex('test_partitioned');
@@ -171,11 +175,14 @@ ERROR: "test_partitioned" is not an index
create view test_view as select 1;
-- these should all fail
select pgstattuple('test_view');
-ERROR: "test_view" (view) is not supported
+ERROR: cannot get tuple-level statistics for relation "test_view"
+DETAIL: This operation is not supported for views.
select pgstattuple_approx('test_view');
-ERROR: "test_view" is not a table, materialized view, or TOAST table
+ERROR: relation "test_view" is of wrong relation kind
+DETAIL: This operation is not supported for views.
select pg_relpages('test_view');
-ERROR: "test_view" is not a table, index, materialized view, sequence, or TOAST table
+ERROR: cannot get page count of relation "test_view"
+DETAIL: This operation is not supported for views.
select pgstatindex('test_view');
ERROR: relation "test_view" is not a btree index
select pgstatginindex('test_view');
@@ -187,11 +194,14 @@ create server dummy_server foreign data wrapper dummy;
create foreign table test_foreign_table () server dummy_server;
-- these should all fail
select pgstattuple('test_foreign_table');
-ERROR: "test_foreign_table" (foreign table) is not supported
+ERROR: cannot get tuple-level statistics for relation "test_foreign_table"
+DETAIL: This operation is not supported for foreign tables.
select pgstattuple_approx('test_foreign_table');
-ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
+ERROR: relation "test_foreign_table" is of wrong relation kind
+DETAIL: This operation is not supported for foreign tables.
select pg_relpages('test_foreign_table');
-ERROR: "test_foreign_table" is not a table, index, materialized view, sequence, or TOAST table
+ERROR: cannot get page count of relation "test_foreign_table"
+DETAIL: This operation is not supported for foreign tables.
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 1fe193bb25..3b836f370e 100644
--- a/contrib/pgstattuple/pgstatapprox.c
+++ b/contrib/pgstattuple/pgstatapprox.c
@@ -289,8 +289,9 @@ pgstattuple_approx_internal(Oid relid, FunctionCallInfo fcinfo)
rel->rd_rel->relkind == RELKIND_TOASTVALUE))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("\"%s\" is not a table, materialized view, or TOAST table",
- RelationGetRelationName(rel))));
+ errmsg("relation \"%s\" is of wrong relation kind",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(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 5368bb30f0..6c4b053dd0 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -128,8 +128,8 @@ typedef struct HashIndexStat
} HashIndexStat;
static Datum pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo);
+static int64 pg_relpages_impl(Relation rel);
static void GetHashPageStats(Page page, HashIndexStat *stats);
-static void check_relation_relkind(Relation rel);
/* ------------------------------------------------------
* pgstatindex()
@@ -383,7 +383,6 @@ Datum
pg_relpages(PG_FUNCTION_ARGS)
{
text *relname = PG_GETARG_TEXT_PP(0);
- int64 relpages;
Relation rel;
RangeVar *relrv;
@@ -395,16 +394,7 @@ pg_relpages(PG_FUNCTION_ARGS)
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
rel = relation_openrv(relrv, AccessShareLock);
- /* only some relkinds have storage */
- check_relation_relkind(rel);
-
- /* note: this will work OK on non-local temp tables */
-
- relpages = RelationGetNumberOfBlocks(rel);
-
- relation_close(rel, AccessShareLock);
-
- PG_RETURN_INT64(relpages);
+ PG_RETURN_INT64(pg_relpages_impl(rel));
}
/* No need for superuser checks in v1.5, see above */
@@ -412,23 +402,13 @@ Datum
pg_relpages_v1_5(PG_FUNCTION_ARGS)
{
text *relname = PG_GETARG_TEXT_PP(0);
- int64 relpages;
Relation rel;
RangeVar *relrv;
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
rel = relation_openrv(relrv, AccessShareLock);
- /* only some relkinds have storage */
- check_relation_relkind(rel);
-
- /* note: this will work OK on non-local temp tables */
-
- relpages = RelationGetNumberOfBlocks(rel);
-
- relation_close(rel, AccessShareLock);
-
- PG_RETURN_INT64(relpages);
+ PG_RETURN_INT64(pg_relpages_impl(rel));
}
/* Must keep superuser() check, see above. */
@@ -436,7 +416,6 @@ Datum
pg_relpagesbyid(PG_FUNCTION_ARGS)
{
Oid relid = PG_GETARG_OID(0);
- int64 relpages;
Relation rel;
if (!superuser())
@@ -446,16 +425,7 @@ pg_relpagesbyid(PG_FUNCTION_ARGS)
rel = relation_open(relid, AccessShareLock);
- /* only some relkinds have storage */
- check_relation_relkind(rel);
-
- /* note: this will work OK on non-local temp tables */
-
- relpages = RelationGetNumberOfBlocks(rel);
-
- relation_close(rel, AccessShareLock);
-
- PG_RETURN_INT64(relpages);
+ PG_RETURN_INT64(pg_relpages_impl(rel));
}
/* No need for superuser checks in v1.5, see above */
@@ -463,13 +433,24 @@ Datum
pg_relpagesbyid_v1_5(PG_FUNCTION_ARGS)
{
Oid relid = PG_GETARG_OID(0);
- int64 relpages;
Relation rel;
rel = relation_open(relid, AccessShareLock);
- /* only some relkinds have storage */
- check_relation_relkind(rel);
+ PG_RETURN_INT64(pg_relpages_impl(rel));
+}
+
+static int64
+pg_relpages_impl(Relation rel)
+{
+ int64 relpages;
+
+ if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot get page count of relation \"%s\"",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
/* note: this will work OK on non-local temp tables */
@@ -477,7 +458,7 @@ pg_relpagesbyid_v1_5(PG_FUNCTION_ARGS)
relation_close(rel, AccessShareLock);
- PG_RETURN_INT64(relpages);
+ return relpages;
}
/* ------------------------------------------------------
@@ -754,21 +735,3 @@ GetHashPageStats(Page page, HashIndexStat *stats)
}
stats->free_space += PageGetExactFreeSpace(page);
}
-
-/*
- * check_relation_relkind - convenience routine to check that relation
- * is of the relkind supported by the callers
- */
-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)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table, index, materialized view, sequence, or TOAST table",
- RelationGetRelationName(rel))));
-}
diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c
index 21fdeff8af..f408e6d84d 100644
--- a/contrib/pgstattuple/pgstattuple.c
+++ b/contrib/pgstattuple/pgstattuple.c
@@ -284,31 +284,20 @@ pgstat_relation(Relation rel, FunctionCallInfo fcinfo)
err = "unknown index";
break;
}
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("index \"%s\" (%s) is not supported",
+ RelationGetRelationName(rel), err)));
break;
- case RELKIND_VIEW:
- err = "view";
- break;
- case RELKIND_COMPOSITE_TYPE:
- err = "composite type";
- break;
- case RELKIND_FOREIGN_TABLE:
- err = "foreign table";
- break;
- case RELKIND_PARTITIONED_TABLE:
- err = "partitioned table";
- break;
- case RELKIND_PARTITIONED_INDEX:
- err = "partitioned index";
- break;
+
default:
- err = "unknown";
- break;
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot get tuple-level statistics for relation \"%s\"",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
}
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("\"%s\" (%s) is not supported",
- RelationGetRelationName(rel), err)));
return 0; /* should not happen */
}
diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile
index 69f9dd51a7..d297e77361 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/pg_class.c b/src/backend/catalog/pg_class.c
new file mode 100644
index 0000000000..304c0af808
--- /dev/null
+++ b/src/backend/catalog/pg_class.c
@@ -0,0 +1,52 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_class.c
+ * routines to support manipulation of the pg_class relation
+ *
+ * Portions Copyright (c) 1996-2021, 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 that the relkind is not supported for this
+ * operation.
+ */
+int
+errdetail_relkind_not_supported(char relkind)
+{
+ switch (relkind)
+ {
+ case RELKIND_RELATION:
+ return errdetail("This operation is not supported for tables.");
+ case RELKIND_INDEX:
+ return errdetail("This operation is not supported for indexes.");
+ case RELKIND_SEQUENCE:
+ return errdetail("This operation is not supported for sequences.");
+ case RELKIND_TOASTVALUE:
+ return errdetail("This operation is not supported for TOAST tables.");
+ case RELKIND_VIEW:
+ return errdetail("This operation is not supported for views.");
+ case RELKIND_MATVIEW:
+ return errdetail("This operation is not supported for materialized views.");
+ case RELKIND_COMPOSITE_TYPE:
+ return errdetail("This operation is not supported for composite types.");
+ case RELKIND_FOREIGN_TABLE:
+ return errdetail("This operation is not supported for foreign tables.");
+ case RELKIND_PARTITIONED_TABLE:
+ return errdetail("This operation is not supported for partitioned tables.");
+ case RELKIND_PARTITIONED_INDEX:
+ return errdetail("This operation is not supported for partitioned indexes.");
+ default:
+ elog(ERROR, "unrecognized relkind: '%c'", relkind);
+ return 0;
+ }
+}
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index bf81f6ccc5..147b5abc19 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -99,10 +99,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 216b8d3068..834f1a2a3f 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("cannot set comment on relation \"%s\"",
+ RelationGetRelationName(relation)),
+ errdetail_relkind_not_supported(relation->rd_rel->relkind)));
break;
default:
break;
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 76774dce06..c14ca27c5e 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -650,22 +650,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_not_supported(rel->rd_rel->relkind)));
break;
}
diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c
index 34f2270ced..62465bacd8 100644
--- a/src/backend/commands/lockcmds.c
+++ b/src/backend/commands/lockcmds.c
@@ -89,8 +89,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_not_supported(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 6906714298..ddc019cb39 100644
--- a/src/backend/commands/seclabel.c
+++ b/src/backend/commands/seclabel.c
@@ -188,8 +188,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_not_supported(relation->rd_rel->relkind)));
break;
default:
break;
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 0415df9ccb..e3f9f6d53d 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1680,8 +1680,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("sequence cannot be owned by relation \"%s\"",
+ RelationGetRelationName(tablerel)),
+ errdetail_relkind_not_supported(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 b244a0fbd7..4856f4b41d 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -135,8 +135,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_not_supported(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 4e23c7fce5..8dfbe4fee1 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -398,8 +398,7 @@ static void ATRewriteTables(AlterTableStmt *parsetree,
AlterTableUtilityContext *context);
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 ATSimplePermissions(AlterTableType cmdtype, Relation rel, int allowed_targets);
static void ATSimpleRecursion(List **wqueue, Relation rel,
AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode,
AlterTableUtilityContext *context);
@@ -3394,8 +3393,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_not_supported(relkind)));
/*
* permissions checking. only the owner of a class can change its schema.
@@ -4422,7 +4422,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
switch (cmd->subtype)
{
case AT_AddColumn: /* ADD COLUMN */
- ATSimplePermissions(rel,
+ ATSimplePermissions(cmd->subtype, rel,
ATT_TABLE | ATT_COMPOSITE_TYPE | ATT_FOREIGN_TABLE);
ATPrepAddColumn(wqueue, rel, recurse, recursing, false, cmd,
lockmode, context);
@@ -4430,7 +4430,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_ADD_COL;
break;
case AT_AddColumnToView: /* add column via CREATE OR REPLACE VIEW */
- ATSimplePermissions(rel, ATT_VIEW);
+ ATSimplePermissions(cmd->subtype, rel, ATT_VIEW);
ATPrepAddColumn(wqueue, rel, recurse, recursing, true, cmd,
lockmode, context);
/* Recursion occurs during execution phase */
@@ -4444,7 +4444,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
* substitutes default values into INSERTs before it expands
* rules.
*/
- ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
/* No command-specific prep needed */
pass = cmd->def ? AT_PASS_ADD_OTHERCONSTR : AT_PASS_DROP;
@@ -4452,77 +4452,77 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_CookedColumnDefault: /* add a pre-cooked default */
/* This is currently used only in CREATE TABLE */
/* (so the permission check really isn't necessary) */
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
/* This command never recurses */
pass = AT_PASS_ADD_OTHERCONSTR;
break;
case AT_AddIdentity:
- ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
/* This command never recurses */
pass = AT_PASS_ADD_OTHERCONSTR;
break;
case AT_SetIdentity:
- ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
/* This command never recurses */
/* This should run after AddIdentity, so do it in MISC pass */
pass = AT_PASS_MISC;
break;
case AT_DropIdentity:
- ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE);
/* This command never recurses */
pass = AT_PASS_DROP;
break;
case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
ATPrepDropNotNull(rel, recurse, recursing);
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
pass = AT_PASS_DROP;
break;
case AT_SetNotNull: /* ALTER COLUMN SET NOT NULL */
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
/* Need command-specific recursion decision */
ATPrepSetNotNull(wqueue, rel, cmd, recurse, recursing,
lockmode, context);
pass = AT_PASS_COL_ATTRS;
break;
case AT_CheckNotNull: /* check column is already marked NOT NULL */
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
/* No command-specific prep needed */
pass = AT_PASS_COL_ATTRS;
break;
case AT_DropExpression: /* ALTER COLUMN DROP EXPRESSION */
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
ATPrepDropExpression(rel, cmd, recurse, recursing, lockmode);
pass = AT_PASS_DROP;
break;
case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */
- ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_PARTITIONED_INDEX | ATT_FOREIGN_TABLE);
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
case AT_SetOptions: /* ALTER COLUMN SET ( options ) */
case AT_ResetOptions: /* ALTER COLUMN RESET ( options ) */
- ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX | ATT_FOREIGN_TABLE);
/* This command never recurses */
pass = AT_PASS_MISC;
break;
case AT_SetStorage: /* ALTER COLUMN SET STORAGE */
- ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW | ATT_FOREIGN_TABLE);
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
case AT_SetCompression: /* ALTER COLUMN SET COMPRESSION */
- ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW);
/* This command never recurses */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
case AT_DropColumn: /* DROP COLUMN */
- ATSimplePermissions(rel,
+ ATSimplePermissions(cmd->subtype, rel,
ATT_TABLE | ATT_COMPOSITE_TYPE | ATT_FOREIGN_TABLE);
ATPrepDropColumn(wqueue, rel, recurse, recursing, cmd,
lockmode, context);
@@ -4530,13 +4530,13 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_DROP;
break;
case AT_AddIndex: /* ADD INDEX */
- ATSimplePermissions(rel, ATT_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE);
/* This command never recurses */
/* No command-specific prep needed */
pass = AT_PASS_ADD_INDEX;
break;
case AT_AddConstraint: /* ADD CONSTRAINT */
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
/* Recursion occurs during execution phase */
/* No command-specific prep needed except saving recurse flag */
if (recurse)
@@ -4544,13 +4544,13 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_ADD_CONSTR;
break;
case AT_AddIndexConstraint: /* ADD CONSTRAINT USING INDEX */
- ATSimplePermissions(rel, ATT_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE);
/* This command never recurses */
/* No command-specific prep needed */
pass = AT_PASS_ADD_INDEXCONSTR;
break;
case AT_DropConstraint: /* DROP CONSTRAINT */
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
ATCheckPartitionsNotInUse(rel, lockmode);
/* Other recursion occurs during execution phase */
/* No command-specific prep needed except saving recurse flag */
@@ -4559,7 +4559,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_DROP;
break;
case AT_AlterColumnType: /* ALTER COLUMN TYPE */
- ATSimplePermissions(rel,
+ ATSimplePermissions(cmd->subtype, rel,
ATT_TABLE | ATT_COMPOSITE_TYPE | ATT_FOREIGN_TABLE);
/* See comments for ATPrepAlterColumnType */
cmd = ATParseTransformCmd(wqueue, tab, rel, cmd, recurse, lockmode,
@@ -4571,7 +4571,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_ALTER_TYPE;
break;
case AT_AlterColumnGenericOptions:
- ATSimplePermissions(rel, ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_FOREIGN_TABLE);
/* This command never recurses */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
@@ -4583,13 +4583,13 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_ClusterOn: /* CLUSTER ON */
case AT_DropCluster: /* SET WITHOUT CLUSTER */
- ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW);
/* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
case AT_SetLogged: /* SET LOGGED */
- ATSimplePermissions(rel, ATT_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE);
if (tab->chgPersistence)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -4604,7 +4604,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_MISC;
break;
case AT_SetUnLogged: /* SET UNLOGGED */
- ATSimplePermissions(rel, ATT_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE);
if (tab->chgPersistence)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -4619,11 +4619,11 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_MISC;
break;
case AT_DropOids: /* SET WITHOUT OIDS */
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
pass = AT_PASS_DROP;
break;
case AT_SetTableSpace: /* SET TABLESPACE */
- ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX |
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW | ATT_INDEX |
ATT_PARTITIONED_INDEX);
/* This command never recurses */
ATPrepSetTableSpace(tab, rel, cmd->name, lockmode);
@@ -4632,30 +4632,30 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_SetRelOptions: /* SET (...) */
case AT_ResetRelOptions: /* RESET (...) */
case AT_ReplaceRelOptions: /* reset them all, then set just these */
- ATSimplePermissions(rel, ATT_TABLE | ATT_VIEW | ATT_MATVIEW | ATT_INDEX);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_VIEW | ATT_MATVIEW | ATT_INDEX);
/* This command never recurses */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
case AT_AddInherit: /* INHERIT */
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
/* This command never recurses */
ATPrepAddInherit(rel);
pass = AT_PASS_MISC;
break;
case AT_DropInherit: /* NO INHERIT */
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
/* This command never recurses */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
case AT_AlterConstraint: /* ALTER CONSTRAINT */
- ATSimplePermissions(rel, ATT_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE);
/* Recursion occurs during execution phase */
pass = AT_PASS_MISC;
break;
case AT_ValidateConstraint: /* VALIDATE CONSTRAINT */
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
/* Recursion occurs during execution phase */
/* No command-specific prep needed except saving recurse flag */
if (recurse)
@@ -4663,7 +4663,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_MISC;
break;
case AT_ReplicaIdentity: /* REPLICA IDENTITY ... */
- ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_MATVIEW);
pass = AT_PASS_MISC;
/* This command never recurses */
/* No command-specific prep needed */
@@ -4676,7 +4676,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_DisableTrig: /* DISABLE TRIGGER variants */
case AT_DisableTrigAll:
case AT_DisableTrigUser:
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
pass = AT_PASS_MISC;
@@ -4691,28 +4691,28 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_DisableRowSecurity:
case AT_ForceRowSecurity:
case AT_NoForceRowSecurity:
- ATSimplePermissions(rel, ATT_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE);
/* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
case AT_GenericOptions:
- ATSimplePermissions(rel, ATT_FOREIGN_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_FOREIGN_TABLE);
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
case AT_AttachPartition:
- ATSimplePermissions(rel, ATT_TABLE | ATT_PARTITIONED_INDEX);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_PARTITIONED_INDEX);
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
case AT_DetachPartition:
- ATSimplePermissions(rel, ATT_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE);
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
case AT_DetachPartitionFinalize:
- ATSimplePermissions(rel, ATT_TABLE);
+ ATSimplePermissions(cmd->subtype, rel, ATT_TABLE);
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
@@ -5941,6 +5941,145 @@ ATGetQueueEntry(List **wqueue, Relation rel)
return tab;
}
+static const char *
+alter_table_type_to_string(AlterTableType cmdtype)
+{
+ switch (cmdtype)
+ {
+ case AT_AddColumn:
+ case AT_AddColumnRecurse:
+ case AT_AddColumnToView:
+ return "ADD COLUMN";
+ case AT_ColumnDefault:
+ case AT_CookedColumnDefault:
+ return "ALTER COLUMN ... SET DEFAULT";
+ case AT_DropNotNull:
+ return "ALTER COLUMN ... DROP NOT NULL";
+ case AT_SetNotNull:
+ return "ALTER COLUMN ... SET NOT NULL";
+ case AT_DropExpression:
+ return "ALTER COLUMN ... DROP EXPRESSION";
+ case AT_CheckNotNull:
+ return NULL; /* not real grammar */
+ case AT_SetStatistics:
+ return "ALTER COLUMN ... SET STATISTICS";
+ case AT_SetOptions:
+ return "ALTER COLUMN ... SET";
+ case AT_ResetOptions:
+ return "ALTER COLUMN ... RESET";
+ case AT_SetStorage:
+ return "ALTER COLUMN ... SET STORAGE";
+ case AT_SetCompression:
+ return "ALTER COLUMN ... SET COMPRESSION";
+ case AT_DropColumn:
+ case AT_DropColumnRecurse:
+ return "DROP COLUMN";
+ case AT_AddIndex:
+ case AT_ReAddIndex:
+ return NULL; /* not real grammar */
+ case AT_AddConstraint:
+ case AT_AddConstraintRecurse:
+ case AT_ReAddConstraint:
+ case AT_ReAddDomainConstraint:
+ case AT_AddIndexConstraint:
+ return "ADD CONSTRAINT";
+ case AT_AlterConstraint:
+ return "ALTER CONSTRAINT";
+ case AT_ValidateConstraint:
+ case AT_ValidateConstraintRecurse:
+ return "VALIDATE CONSTRAINT";
+ case AT_DropConstraint:
+ case AT_DropConstraintRecurse:
+ return "DROP CONSTRAINT";
+ case AT_ReAddComment:
+ return NULL; /* not real grammar */
+ case AT_AlterColumnType:
+ return "ALTER COLUMN ... SET DATA TYPE";
+ case AT_AlterColumnGenericOptions:
+ return "ALTER COLUMN ... OPTIONS";
+ case AT_ChangeOwner:
+ return "OWNER TO";
+ case AT_ClusterOn:
+ return "CLUSTER ON";
+ case AT_DropCluster:
+ return "SET WITHOUT CLUSTER";
+ case AT_SetLogged:
+ return "SET LOGGED";
+ case AT_SetUnLogged:
+ return "SET UNLOGGED";
+ case AT_DropOids:
+ return "SET WITHOUT OIDS";
+ case AT_SetTableSpace:
+ return "SET TABLESPACE";
+ case AT_SetRelOptions:
+ return "SET";
+ case AT_ResetRelOptions:
+ return "RESET";
+ case AT_ReplaceRelOptions:
+ return NULL; /* not real grammar */
+ case AT_EnableTrig:
+ return "ENABLE TRIGGER";
+ case AT_EnableAlwaysTrig:
+ return "ENABLE ALWAYS TRIGGER";
+ case AT_EnableReplicaTrig:
+ return "ENABLE REPLICA TRIGGER";
+ case AT_DisableTrig:
+ return "DISABLE TRIGGER";
+ case AT_EnableTrigAll:
+ return "ENABLE TRIGGER ALL";
+ case AT_DisableTrigAll:
+ return "DISABLE TRIGGER ALL";
+ case AT_EnableTrigUser:
+ return "ENABLE TRIGGER USER";
+ case AT_DisableTrigUser:
+ return "DISABLE TRIGGER USER";
+ case AT_EnableRule:
+ return "ENABLE RULE";
+ case AT_EnableAlwaysRule:
+ return "ENABLE ALWAYS RULE";
+ case AT_EnableReplicaRule:
+ return "ENABLE REPLICA RULE";
+ case AT_DisableRule:
+ return "DISABLE RULE";
+ case AT_AddInherit:
+ return "INHERIT";
+ case AT_DropInherit:
+ return "NO INHERIT";
+ case AT_AddOf:
+ return "OF";
+ case AT_DropOf:
+ return "NOT OF";
+ case AT_ReplicaIdentity:
+ return "REPLICA IDENTITY";
+ case AT_EnableRowSecurity:
+ return "ENABLE ROW SECURITY";
+ case AT_DisableRowSecurity:
+ return "DISABLE ROW SECURITY";
+ case AT_ForceRowSecurity:
+ return "FORCE ROW SECURITY";
+ case AT_NoForceRowSecurity:
+ return "NO FORCE ROW SECURITY";
+ case AT_GenericOptions:
+ return "OPTIONS";
+ case AT_AttachPartition:
+ return "ATTACH PARTITION";
+ case AT_DetachPartition:
+ return "DETACH PARTITION";
+ case AT_DetachPartitionFinalize:
+ return "DETACH PARTITION FINALIZE";
+ case AT_AddIdentity:
+ return "ALTER COLUMN ... ADD IDENTITY";
+ case AT_SetIdentity:
+ return "ALTER COLUMN ... SET";
+ case AT_DropIdentity:
+ return "ALTER COLUMN ... DROP IDENTITY";
+ case AT_ReAddStatistics:
+ return NULL; /* not real grammar */
+ }
+
+ return NULL;
+}
+
/*
* ATSimplePermissions
*
@@ -5949,7 +6088,7 @@ ATGetQueueEntry(List **wqueue, Relation rel)
* - Ensure that it is not a system table
*/
static void
-ATSimplePermissions(Relation rel, int allowed_targets)
+ATSimplePermissions(AlterTableType cmdtype, Relation rel, int allowed_targets)
{
int actual_target;
@@ -5984,7 +6123,20 @@ ATSimplePermissions(Relation rel, int allowed_targets)
/* Wrong target type? */
if ((actual_target & allowed_targets) == 0)
- ATWrongRelkindError(rel, allowed_targets);
+ {
+ const char *action_str = alter_table_type_to_string(cmdtype);
+
+ if (action_str)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("ALTER action %s cannot be performed on relation \"%s\"",
+ action_str, RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
+ else
+ /* internal error? */
+ elog(ERROR, "invalid ALTER action attempted on relation \"%s\"",
+ RelationGetRelationName(rel));
+ }
/* Permissions checks */
if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
@@ -5998,66 +6150,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
*
@@ -6452,7 +6544,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions((*cmd)->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
if (rel->rd_rel->relispartition && !recursing)
ereport(ERROR,
@@ -8186,7 +8278,7 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(AT_DropColumn, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
/* Initialize addrs on the first invocation */
Assert(!recursing || addrs != NULL);
@@ -8670,7 +8762,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(AT_AddConstraint, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
/*
* Call AddRelationNewConstraints to do the work, making sure it works on
@@ -11286,7 +11378,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(AT_DropConstraint, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
conrel = table_open(ConstraintRelationId, RowExclusiveLock);
@@ -13205,8 +13297,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_not_supported(tuple_class->relkind)));
}
/*
@@ -13621,8 +13714,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 options for relation \"%s\"",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
break;
}
@@ -14176,7 +14270,7 @@ ATExecAddInherit(Relation child_rel, RangeVar *parent, LOCKMODE lockmode)
* Must be owner of both parent and child -- child was checked by
* ATSimplePermissions call in ATPrepCmd
*/
- ATSimplePermissions(parent_rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(AT_AddInherit, parent_rel, ATT_TABLE | ATT_FOREIGN_TABLE);
/* Permanent rels cannot inherit from temporary ones */
if (parent_rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
@@ -16505,17 +16599,27 @@ RangeVarCallbackForAlterRelation(const RangeVar *rv, Oid relid, Oid oldrelid,
* Don't allow ALTER TABLE .. SET SCHEMA on relations that can't be moved
* to a different schema, such as indexes and TOAST tables.
*/
- if (IsA(stmt, AlterObjectSchemaStmt) &&
- relkind != RELKIND_RELATION &&
- relkind != RELKIND_VIEW &&
- relkind != RELKIND_MATVIEW &&
- relkind != RELKIND_SEQUENCE &&
- relkind != RELKIND_FOREIGN_TABLE &&
- 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)));
+ if (IsA(stmt, AlterObjectSchemaStmt))
+ {
+ if (relkind == RELKIND_INDEX || relkind == RELKIND_PARTITIONED_INDEX)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot change schema of index \"%s\"",
+ rv->relname),
+ errhint("Change the schema of the table instead.")));
+ else if (relkind == RELKIND_COMPOSITE_TYPE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot change schema of composite type \"%s\"",
+ rv->relname),
+ errhint("Use ALTER TYPE instead.")));
+ else if (relkind == RELKIND_TOASTVALUE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot change schema of TOAST table \"%s\"",
+ rv->relname),
+ errhint("Change the schema of the table instead.")));
+ }
ReleaseSysCache(tuple);
}
@@ -17077,7 +17181,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd,
* Must be owner of both parent and source table -- parent was checked by
* ATSimplePermissions call in ATPrepCmd
*/
- ATSimplePermissions(attachrel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(AT_AttachPartition, attachrel, ATT_TABLE | ATT_FOREIGN_TABLE);
/* A partition can only have one parent */
if (attachrel->rd_rel->relispartition)
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 07c73f39de..952c8d582a 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -286,8 +286,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("relation \"%s\" cannot have triggers",
+ RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
if (!allowSystemTableMods && IsSystemRelation(rel))
ereport(ERROR,
@@ -1262,8 +1263,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_not_supported(rel->rd_rel->relkind)));
if (!allowSystemTableMods && IsSystemRelation(rel))
ereport(ERROR,
@@ -1368,8 +1370,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_not_supported(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 81d3e7990c..3afcd6b511 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -976,8 +976,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_not_supported(relation->rd_rel->relkind)));
cancel_parser_errposition_callback(&pcbstate);
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 27e4ef911c..6589345ac4 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("relation \"%s\" cannot have rules",
+ RelationGetRelationName(event_relation)),
+ errdetail_relkind_not_supported(event_relation->rd_rel->relkind)));
if (!allowSystemTableMods && IsSystemRelation(event_relation))
ereport(ERROR,
@@ -935,7 +936,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_not_supported(form->relkind)));
if (!allowSystemTableMods && IsSystemClass(relid, form))
ereport(ERROR,
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index 3e37729436..2c3543aac1 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -201,6 +201,7 @@ DECLARE_INDEX(pg_class_tblspc_relfilenode_index, 3455, on pg_class using btree(r
(relkind) == RELKIND_TOASTVALUE || \
(relkind) == RELKIND_MATVIEW)
+extern int errdetail_relkind_not_supported(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 f81bdf513b..8dcb00ac67 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1087,9 +1087,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: ALTER action ALTER COLUMN ... DROP NOT NULL cannot be performed on relation "myview"
+DETAIL: This operation is not supported for views.
alter table myview alter column test set not null;
-ERROR: "myview" is not a table or foreign table
+ERROR: ALTER action ALTER COLUMN ... SET NOT NULL cannot be performed on relation "myview"
+DETAIL: This operation is not supported for views.
drop view myview;
drop table atacc1;
-- set not null verified by constraints
@@ -1387,7 +1389,8 @@ select * from myview;
(0 rows)
alter table myview drop d;
-ERROR: "myview" is not a table, composite type, or foreign table
+ERROR: ALTER action DROP COLUMN cannot be performed on relation "myview"
+DETAIL: This operation is not supported for views.
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 4dc5e6aa5f..7ad5fafe93 100644
--- a/src/test/regress/expected/create_table_like.out
+++ b/src/test/regress/expected/create_table_like.out
@@ -504,9 +504,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: This operation is not supported for sequences.
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 5385f98a0f..809d40a79a 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: This operation is not supported for foreign tables.
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: ALTER action ALTER CONSTRAINT cannot be performed on relation "ft1"
+DETAIL: This operation is not supported for foreign tables.
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 c93f4470c9..193f780191 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: ALTER action DETACH PARTITION cannot be performed on relation "idxpart_c"
+DETAIL: This operation is not supported for partitioned indexes.
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..71c2b0f1df 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: sequence cannot be owned by relation "pg_class_oid_index"
+DETAIL: This operation is not supported for indexes.
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 8c214d8dfc..62b05c79f9 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -211,14 +211,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: This operation is not supported for indexes.
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: This operation is not supported for sequences.
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: This operation is not supported for views.
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: This operation is not supported for composite types.
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.32.0
On Thu, Jun 24, 2021 at 10:12:49AM +0200, Peter Eisentraut wrote:
There might be room for some wordsmithing in a few places, but generally I
think this is complete.
I have been looking at that, and it seems to me that you nailed it.
That's a nice improvement compared to the existing error handling with
multiple relkinds.
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("ALTER action %s cannot be performed on relation \"%s\"",
+ action_str, RelationGetRelationName(rel)),
+ errdetail_relkind_not_supported(rel->rd_rel->relkind)));
Perhaps the result of alter_table_type_to_string() is worth a note for
translators?
+ case AT_DetachPartitionFinalize:
+ return "DETACH PARTITION FINALIZE";
To be exact, I think that this one should be "DETACH PARTITION
... FINALIZE".
+ if (relkind == RELKIND_INDEX || relkind == RELKIND_PARTITIONED_INDEX)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot change schema of index \"%s\"",
+ rv->relname),
+ errhint("Change the schema of the table instead.")));
+ else if (relkind == RELKIND_COMPOSITE_TYPE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot change schema of composite type
\"%s\"",
+ rv->relname),
+ errhint("Use ALTER TYPE instead.")));
I would simplify this part by removing the errhint(), and use "cannot
change schema of relation .." as error string, with a dose of
errdetail_relkind_not_supported().
+ errmsg("relation \"%s\" cannot have triggers",
+ RelationGetRelationName(rel)),
Better as "cannot create/rename/remove triggers on relation \"%s\""
for the three code paths of trigger.c?
+ errmsg("relation \"%s\" cannot have rules",
[...]
+ errmsg("relation \"%s\" cannot have rules",
For rewriteDefine.c, this could be "cannot create/rename rules on
relation".
--
Michael
On 02.07.21 08:25, Michael Paquier wrote:
+ ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("ALTER action %s cannot be performed on relation \"%s\"", + action_str, RelationGetRelationName(rel)), + errdetail_relkind_not_supported(rel->rd_rel->relkind))); Perhaps the result of alter_table_type_to_string() is worth a note for translators?
ok
+ case AT_DetachPartitionFinalize: + return "DETACH PARTITION FINALIZE"; To be exact, I think that this one should be "DETACH PARTITION ... FINALIZE".
ok
+ if (relkind == RELKIND_INDEX || relkind == RELKIND_PARTITIONED_INDEX) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot change schema of index \"%s\"", + rv->relname), + errhint("Change the schema of the table instead."))); + else if (relkind == RELKIND_COMPOSITE_TYPE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot change schema of composite type \"%s\"", + rv->relname), + errhint("Use ALTER TYPE instead."))); I would simplify this part by removing the errhint(), and use "cannot change schema of relation .." as error string, with a dose of errdetail_relkind_not_supported().
I aimed for parity with the error reporting in ATExecChangeOwner() here.
+ errmsg("relation \"%s\" cannot have triggers", + RelationGetRelationName(rel)), Better as "cannot create/rename/remove triggers on relation \"%s\"" for the three code paths of trigger.c?+ errmsg("relation \"%s\" cannot have rules", [...] + errmsg("relation \"%s\" cannot have rules", For rewriteDefine.c, this could be "cannot create/rename rules on relation".
I had it like that, but in previous reviews some people liked it better
this way. ;-) I tend to agree with that, since the error condition
isn't that you can't create a rule/etc. (like, due to incorrect
prerequisite state) but that there cannot be one ever.
On 2021-Jun-24, Peter Eisentraut wrote:
There might be room for some wordsmithing in a few places, but generally I
think this is complete.
This looks good to me. I am +0.1 on your proposal of "cannot have
triggers" vs Michael's "cannot create triggers", but really I could go
with either. Michael's idea has the disadvantage that if the user fails
to see the trailing "s" in "triggers" they could get the idea that it's
possible to create some other trigger; that seems impossible to miss
with your wording. But it's not that bad either.
It seemed odd to me at first that errdetail_relkind_not_supported()
returns int, but I realized that it's a trick to let you write "return
errdetail()" so you don't have to have "break" which would require one
extra line. Looks fine.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
On 02.07.21 18:10, Alvaro Herrera wrote:
On 2021-Jun-24, Peter Eisentraut wrote:
There might be room for some wordsmithing in a few places, but generally I
think this is complete.This looks good to me. I am +0.1 on your proposal of "cannot have
triggers" vs Michael's "cannot create triggers", but really I could go
with either. Michael's idea has the disadvantage that if the user fails
to see the trailing "s" in "triggers" they could get the idea that it's
possible to create some other trigger; that seems impossible to miss
with your wording. But it's not that bad either.It seemed odd to me at first that errdetail_relkind_not_supported()
returns int, but I realized that it's a trick to let you write "return
errdetail()" so you don't have to have "break" which would require one
extra line. Looks fine.
Thanks, committed.
While reviewing the logical decoding of sequences patch, I found a few
more places that could be updated in the new style introduced by this
thread. See attached patch.
Attachments:
0001-More-improvements-of-error-messages-about-mismatchin.patchtext/plain; charset=UTF-8; name=0001-More-improvements-of-error-messages-about-mismatchin.patch; x-mac-creator=0; x-mac-type=0Download
From 92a06ebfa6f856246c2642a4e45c5b2af69a911d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Tue, 20 Jul 2021 16:28:44 +0200
Subject: [PATCH] More improvements of error messages about mismatching relkind
Follow-up to 2ed532ee8c474e9767e76e1f3251cc3a0224358c, a few error
messages in the logical replication area currently only deal with
tables, but if we're anticipating more relkinds such as sequences
being handled, then these messages also fall into the category
affected by the previous patch, so adjust them too.
---
src/backend/catalog/pg_publication.c | 10 +++++-----
src/backend/executor/execReplication.c | 14 +-------------
src/test/regress/expected/publication.out | 8 ++++----
3 files changed, 10 insertions(+), 22 deletions(-)
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index 36bfff9706..2a2fe03c13 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -54,23 +54,23 @@ check_publication_add_relation(Relation targetrel)
RelationGetForm(targetrel)->relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"%s\" is not a table",
+ errmsg("cannot add relation \"%s\" to publication",
RelationGetRelationName(targetrel)),
- errdetail("Only tables can be added to publications.")));
+ errdetail_relkind_not_supported(RelationGetForm(targetrel)->relkind)));
/* Can't be system table */
if (IsCatalogRelation(targetrel))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("\"%s\" is a system table",
+ errmsg("cannot add relation \"%s\" to publication",
RelationGetRelationName(targetrel)),
- errdetail("System tables cannot be added to publications.")));
+ errdetail("This operation is not supported for system tables.")));
/* UNLOGGED and TEMP relations cannot be part of publication. */
if (!RelationIsPermanent(targetrel))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("table \"%s\" cannot be replicated",
+ errmsg("cannot add relation \"%s\" to publication",
RelationGetRelationName(targetrel)),
errdetail("Temporary and unlogged relations cannot be replicated.")));
}
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 1e285e0349..574d7d27fd 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -608,22 +608,10 @@ void
CheckSubscriptionRelkind(char relkind, const char *nspname,
const char *relname)
{
- /*
- * Give a more specific error for foreign tables.
- */
- if (relkind == RELKIND_FOREIGN_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot use relation \"%s.%s\" as logical replication target",
- nspname, relname),
- errdetail("\"%s.%s\" is a foreign table.",
- nspname, relname)));
-
if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot use relation \"%s.%s\" as logical replication target",
nspname, relname),
- errdetail("\"%s.%s\" is not a table.",
- nspname, relname)));
+ errdetail_relkind_not_supported(relkind)));
}
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index b5b065a1b6..4a5ef0bc24 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -160,8 +160,8 @@ DROP TABLE testpub_parted1;
DROP PUBLICATION testpub_forparted, testpub_forparted1;
-- fail - view
CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_view;
-ERROR: "testpub_view" is not a table
-DETAIL: Only tables can be added to publications.
+ERROR: cannot add relation "testpub_view" to publication
+DETAIL: This operation is not supported for views.
SET client_min_messages = 'ERROR';
CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1, pub_test.testpub_nopk;
RESET client_min_messages;
@@ -182,8 +182,8 @@ Tables:
-- fail - view
ALTER PUBLICATION testpub_default ADD TABLE testpub_view;
-ERROR: "testpub_view" is not a table
-DETAIL: Only tables can be added to publications.
+ERROR: cannot add relation "testpub_view" to publication
+DETAIL: This operation is not supported for views.
ALTER PUBLICATION testpub_default ADD TABLE testpub_tbl1;
ALTER PUBLICATION testpub_default SET TABLE testpub_tbl1;
ALTER PUBLICATION testpub_default ADD TABLE pub_test.testpub_nopk;
--
2.32.0
On Tue, Jul 20, 2021 at 05:08:53PM +0200, Peter Eisentraut wrote:
While reviewing the logical decoding of sequences patch, I found a few more
places that could be updated in the new style introduced by this thread.
See attached patch.
Those changes look fine. I am spotting one instance in
init_sequence() that looks worth aligning with the others?
Did you consider changing RangeVarCallbackForAlterRelation() or
ExecGrant_Relation() when it came to this thread? Just noticing that,
while going through the code.
--
Michael
On 21.07.21 04:21, Michael Paquier wrote:
On Tue, Jul 20, 2021 at 05:08:53PM +0200, Peter Eisentraut wrote:
While reviewing the logical decoding of sequences patch, I found a few more
places that could be updated in the new style introduced by this thread.
See attached patch.Those changes look fine. I am spotting one instance in
init_sequence() that looks worth aligning with the others?
I think if you write "ALTER SEQUENCE foo", then "foo is not a sequence"
would be an appropriate error message, so this doesn't need changing.
Did you consider changing RangeVarCallbackForAlterRelation() or
ExecGrant_Relation() when it came to this thread? Just noticing that,
while going through the code.
These might be worth another look, but I'd need to investigate more in
what situations they happen.