From: Alvaro Herrera Date: Wed, 19 Dec 2018 14:33:07 -0300 Subject: [PATCH v2] unsuitable relkind --- .../pg_visibility/expected/pg_visibility.out | 75 ++++++++++++------- contrib/pg_visibility/pg_visibility.c | 7 +- contrib/pgstattuple/expected/pgstattuple.out | 9 ++- contrib/pgstattuple/pgstatindex.c | 13 ++-- src/backend/catalog/catalog.c | 45 +++++++++++ src/backend/catalog/toasting.c | 6 +- src/backend/commands/comment.c | 7 +- src/backend/commands/seclabel.c | 6 +- src/include/catalog/catalog.h | 2 + 9 files changed, 127 insertions(+), 43 deletions(-) diff --git a/contrib/pg_visibility/expected/pg_visibility.out b/contrib/pg_visibility/expected/pg_visibility.out index f0dcb897c4..d72ea2f63e 100644 --- a/contrib/pg_visibility/expected/pg_visibility.out +++ b/contrib/pg_visibility/expected/pg_visibility.out @@ -6,66 +6,91 @@ CREATE EXTENSION pg_visibility; 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: cannot examine visibility information for "test_partitioned" +DETAIL: "test_partitioned" is a partitioned table. select pg_visibility_map('test_partitioned'); -ERROR: "test_partitioned" is not a table, materialized view, or TOAST table +ERROR: cannot examine visibility information for "test_partitioned" +DETAIL: "test_partitioned" is a partitioned table. select pg_visibility_map_summary('test_partitioned'); -ERROR: "test_partitioned" is not a table, materialized view, or TOAST table +ERROR: cannot examine visibility information for "test_partitioned" +DETAIL: "test_partitioned" is a partitioned table. select pg_check_frozen('test_partitioned'); -ERROR: "test_partitioned" is not a table, materialized view, or TOAST table +ERROR: cannot examine visibility information for "test_partitioned" +DETAIL: "test_partitioned" is a partitioned table. select pg_truncate_visibility_map('test_partitioned'); -ERROR: "test_partitioned" is not a table, materialized view, or TOAST table +ERROR: cannot examine visibility information for "test_partitioned" +DETAIL: "test_partitioned" is a partitioned 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: cannot examine visibility information for "test_index" +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: cannot examine visibility information for "test_index" +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: cannot examine visibility information for "test_index" +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: cannot examine visibility information for "test_index" +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: cannot examine visibility information for "test_index" +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: cannot examine visibility information for "test_view" +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: cannot examine visibility information for "test_view" +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: cannot examine visibility information for "test_view" +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: cannot examine visibility information for "test_view" +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: cannot examine visibility information for "test_view" +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: cannot examine visibility information for "test_sequence" +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: cannot examine visibility information for "test_sequence" +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: cannot examine visibility information for "test_sequence" +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: cannot examine visibility information for "test_sequence" +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: cannot examine visibility information for "test_sequence" +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: cannot examine visibility information for "test_foreign_table" +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: cannot examine visibility information for "test_foreign_table" +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: cannot examine visibility information for "test_foreign_table" +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: cannot examine visibility information for "test_foreign_table" +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: cannot examine visibility information for "test_foreign_table" +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 3282742b80..24f164f8e8 100644 --- a/contrib/pg_visibility/pg_visibility.c +++ b/contrib/pg_visibility/pg_visibility.c @@ -13,6 +13,7 @@ #include "access/heapam.h" #include "access/htup_details.h" #include "access/visibilitymap.h" +#include "catalog/catalog.h" #include "catalog/pg_type.h" #include "catalog/storage_xlog.h" #include "funcapi.h" @@ -776,6 +777,8 @@ 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("cannot examine visibility information for \"%s\"", + RelationGetRelationName(rel)), + errdetail_unsuitable_relkind(rel->rd_rel->relkind, + RelationGetRelationName(rel)))); } diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.out index 9858ea69d4..753087cdc8 100644 --- a/contrib/pgstattuple/expected/pgstattuple.out +++ b/contrib/pgstattuple/expected/pgstattuple.out @@ -161,7 +161,8 @@ ERROR: "test_partitioned_index" (partitioned index) is not supported select pgstattuple_approx('test_partitioned'); ERROR: "test_partitioned" is not a table or materialized view select pg_relpages('test_partitioned'); -ERROR: "test_partitioned" is not a table, index, materialized view, sequence, or TOAST table +ERROR: cannot compute relpages of "test_partitioned" +DETAIL: "test_partitioned" is a partitioned table. select pgstatindex('test_partitioned'); ERROR: relation "test_partitioned" is not a btree index select pgstatginindex('test_partitioned'); @@ -175,7 +176,8 @@ ERROR: "test_view" (view) is not supported select pgstattuple_approx('test_view'); ERROR: "test_view" is not a table or materialized view select pg_relpages('test_view'); -ERROR: "test_view" is not a table, index, materialized view, sequence, or TOAST table +ERROR: cannot compute relpages of "test_view" +DETAIL: "test_view" is a view. select pgstatindex('test_view'); ERROR: relation "test_view" is not a btree index select pgstatginindex('test_view'); @@ -191,7 +193,8 @@ 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 select pg_relpages('test_foreign_table'); -ERROR: "test_foreign_table" is not a table, index, materialized view, sequence, or TOAST table +ERROR: cannot compute relpages of "test_foreign_table" +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/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c index 2c80753726..02dcf77287 100644 --- a/contrib/pgstattuple/pgstatindex.c +++ b/contrib/pgstattuple/pgstatindex.c @@ -33,6 +33,7 @@ #include "access/nbtree.h" #include "access/relation.h" #include "access/table.h" +#include "catalog/catalog.h" #include "catalog/namespace.h" #include "catalog/pg_am.h" #include "funcapi.h" @@ -758,13 +759,11 @@ 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("cannot compute relpages of \"%s\"", + RelationGetRelationName(rel)), + errdetail_unsuitable_relkind(rel->rd_rel->relkind, + RelationGetRelationName(rel)))); } diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index c39da41d2e..cd096db0a0 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -528,3 +528,48 @@ pg_nextoid(PG_FUNCTION_ARGS) return newoid; } + +/* + * Add an errdetail to the current error message indicating what the relkind of + * the relation is. This is useful when an operation is being attempted on a + * relation that cannot accept it. + */ +int +errdetail_unsuitable_relkind(char relkind, const char *relname) +{ + switch (relkind) + { + case RELKIND_RELATION: + errdetail("\"%s\" is a plain table.", relname); + break; + case RELKIND_INDEX: + errdetail("\"%s\" is an index.", relname); + break; + case RELKIND_SEQUENCE: + errdetail("\"%s\" is a sequence.", relname); + break; + case RELKIND_TOASTVALUE: + errdetail("\"%s\" is a TOAST table.", relname); + break; + case RELKIND_VIEW: + errdetail("\"%s\" is a view.", relname); + break; + case RELKIND_MATVIEW: + errdetail("\"%s\" is a materialized view.", relname); + break; + case RELKIND_COMPOSITE_TYPE: + errdetail("\"%s\" is a composite type.", relname); + break; + case RELKIND_FOREIGN_TABLE: + errdetail("\"%s\" is a foreign table.", relname); + break; + case RELKIND_PARTITIONED_TABLE: + errdetail("\"%s\" is a partitioned table.", relname); + break; + case RELKIND_PARTITIONED_INDEX: + errdetail("\"%s\" is a partitioned index.", relname); + break; + } + + return 0; +} diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c index 77be19175a..5ecb9db90e 100644 --- a/src/backend/catalog/toasting.c +++ b/src/backend/catalog/toasting.c @@ -18,6 +18,7 @@ #include "access/tuptoaster.h" #include "access/xact.h" #include "catalog/binary_upgrade.h" +#include "catalog/catalog.h" #include "catalog/dependency.h" #include "catalog/heap.h" #include "catalog/index.h" @@ -103,8 +104,9 @@ BootstrapToastTable(char *relName, Oid toastOid, Oid toastIndexOid) rel->rd_rel->relkind != RELKIND_MATVIEW) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table or materialized view", - relName))); + errmsg("cannot create toast table for \"%s\"", relName), + errdetail_unsuitable_relkind(rel->rd_rel->relkind, + RelationGetRelationName(rel)))); /* 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 1859fb628f..f4b274e8d6 100644 --- a/src/backend/commands/comment.c +++ b/src/backend/commands/comment.c @@ -18,6 +18,7 @@ #include "access/htup_details.h" #include "access/relation.h" #include "access/table.h" +#include "catalog/catalog.h" #include "catalog/indexing.h" #include "catalog/objectaddress.h" #include "catalog/pg_description.h" @@ -98,8 +99,10 @@ 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 add comment on \"%s\"", + RelationGetRelationName(relation)), + errdetail_unsuitable_relkind(relation->rd_rel->relkind, + RelationGetRelationName(relation)))); break; default: break; diff --git a/src/backend/commands/seclabel.c b/src/backend/commands/seclabel.c index 9db8228028..706b6d7610 100644 --- a/src/backend/commands/seclabel.c +++ b/src/backend/commands/seclabel.c @@ -114,8 +114,10 @@ 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 \"%s\"", + RelationGetRelationName(relation)), + errdetail_unsuitable_relkind(relation->rd_rel->relkind, + RelationGetRelationName(relation)))); break; default: break; diff --git a/src/include/catalog/catalog.h b/src/include/catalog/catalog.h index a58d09d6c7..65d8e2b3c7 100644 --- a/src/include/catalog/catalog.h +++ b/src/include/catalog/catalog.h @@ -38,4 +38,6 @@ extern Oid GetNewOidWithIndex(Relation relation, Oid indexId, extern Oid GetNewRelFileNode(Oid reltablespace, Relation pg_class, char relpersistence); +extern int errdetail_unsuitable_relkind(char relkind, const char *relname); + #endif /* CATALOG_H */ -- 2.17.1 --Nq2Wo0NMKNjxTN9z--