Add relkind checks to certain contrib modules

The contrib extensions pageinspect, pg_visibility and pgstattuple only
work against regular relations which have storage.  They don't work
against foreign tables, partitioned (parent) tables, views, et al.

Add checks to the user-callable functions to return a useful error
message to the user if they mistakenly pass an invalid relation to a
function which doesn't accept that kind of relation.

In passing, improve some of the existing checks to use ereport() instead
of elog(), add a function to consolidate common checks where
appropriate, and add some regression tests.

Author: Amit Langote, with various changes by me
Reviewed by: Michael Paquier and Corey Huinker
Discussion: https://postgr.es/m/ab91fd9d-4751-ee77-c87b-4dd704c1e59c@lab.ntt.co.jp

4 files changed, 219 insertions, 7 deletions

diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.out
index 169d1932b20..29caf5b52d2 100644
--- a/contrib/pgstattuple/expected/pgstattuple.out
+++ b/contrib/pgstattuple/expected/pgstattuple.out
@@ -138,3 +138,110 @@ select * from pgstathashindex('test_hashidx');
        2 |            4 |              0 |            1 |          0 |          0 |          0 |          100
 (1 row)
 
+-- these should error with the wrong type
+select pgstatginindex('test_pkey');
+ERROR:  relation "test_pkey" is not a GIN index
+select pgstathashindex('test_pkey');
+ERROR:  relation "test_pkey" is not a HASH index
+select pgstatindex('test_ginidx');
+ERROR:  relation "test_ginidx" is not a btree index
+select pgstathashindex('test_ginidx');
+ERROR:  relation "test_ginidx" is not a HASH index
+select pgstatindex('test_hashidx');
+ERROR:  relation "test_hashidx" is not a btree index
+select pgstatginindex('test_hashidx');
+ERROR:  relation "test_hashidx" is not a GIN index
+-- check that using any of these functions with unsupported relations will fail
+create table test_partitioned (a int) partition by range (a);
+-- these should all fail
+select pgstattuple('test_partitioned');
+ERROR:  "test_partitioned" (partitioned table) 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
+select pgstatindex('test_partitioned');
+ERROR:  relation "test_partitioned" is not a btree index
+select pgstatginindex('test_partitioned');
+ERROR:  relation "test_partitioned" is not a GIN index
+select pgstathashindex('test_partitioned');
+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
+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
+select pgstatindex('test_view');
+ERROR:  relation "test_view" is not a btree index
+select pgstatginindex('test_view');
+ERROR:  relation "test_view" is not a GIN index
+select pgstathashindex('test_view');
+ERROR:  "test_view" is not an index
+create foreign data wrapper dummy;
+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
+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
+select pgstatindex('test_foreign_table');
+ERROR:  relation "test_foreign_table" is not a btree index
+select pgstatginindex('test_foreign_table');
+ERROR:  relation "test_foreign_table" is not a GIN index
+select pgstathashindex('test_foreign_table');
+ERROR:  "test_foreign_table" is not an index
+-- a partition of a partitioned table should work though
+create table test_partition partition of test_partitioned for values from (1) to (100);
+select pgstattuple('test_partition');
+     pgstattuple     
+---------------------
+ (0,0,0,0,0,0,0,0,0)
+(1 row)
+
+select pgstattuple_approx('test_partition');
+  pgstattuple_approx   
+-----------------------
+ (0,0,0,0,0,0,0,0,0,0)
+(1 row)
+
+select pg_relpages('test_partition');
+ pg_relpages 
+-------------
+           0
+(1 row)
+
+-- not for the index calls though, of course
+select pgstatindex('test_partition');
+ERROR:  relation "test_partition" is not a btree index
+select pgstatginindex('test_partition');
+ERROR:  relation "test_partition" is not a GIN index
+select pgstathashindex('test_partition');
+ERROR:  "test_partition" is not an index
+-- an actual index of a partitiond table should work though
+create index test_partition_idx on test_partition(a);
+create index test_partition_hash_idx on test_partition using hash (a);
+WARNING:  hash indexes are not WAL-logged and their use is discouraged
+-- these should work
+select pgstatindex('test_partition_idx');
+         pgstatindex          
+------------------------------
+ (2,0,8192,0,0,0,0,0,NaN,NaN)
+(1 row)
+
+select pgstathashindex('test_partition_hash_idx');
+   pgstathashindex   
+---------------------
+ (2,8,0,1,0,0,0,100)
+(1 row)
+
+drop table test_partitioned;
+drop view test_view;
+drop foreign table test_foreign_table;
+drop server dummy_server;
+drop foreign data wrapper dummy;
diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c
index 17a53e3bb7d..c69f9ec0937 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -128,7 +128,7 @@ typedef struct HashIndexStat
 
 static Datum pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo);
 static void GetHashPageStats(Page page, HashIndexStat *stats);
-
+static void check_relation_relkind(Relation rel);
 
 /* ------------------------------------------------------
  * pgstatindex()
@@ -221,8 +221,10 @@ pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo)
 	BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD);
 
 	if (!IS_INDEX(rel) || !IS_BTREE(rel))
-		elog(ERROR, "relation \"%s\" is not a btree index",
-			 RelationGetRelationName(rel));
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("relation \"%s\" is not a btree index",
+						RelationGetRelationName(rel))));
 
 	/*
 	 * Reject attempts to read non-local temporary relations; we would be
@@ -388,6 +390,9 @@ 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);
@@ -409,6 +414,9 @@ pg_relpages_v1_5(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);
@@ -433,6 +441,9 @@ 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);
@@ -452,6 +463,9 @@ pg_relpagesbyid_v1_5(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);
@@ -508,8 +522,10 @@ pgstatginindex_internal(Oid relid, FunctionCallInfo fcinfo)
 	rel = relation_open(relid, AccessShareLock);
 
 	if (!IS_INDEX(rel) || !IS_GIN(rel))
-		elog(ERROR, "relation \"%s\" is not a GIN index",
-			 RelationGetRelationName(rel));
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("relation \"%s\" is not a GIN index",
+						RelationGetRelationName(rel))));
 
 	/*
 	 * Reject attempts to read non-local temporary relations; we would be
@@ -581,9 +597,13 @@ pgstathashindex(PG_FUNCTION_ARGS)
 
 	rel = index_open(relid, AccessShareLock);
 
+	/* index_open() checks that it's an index */
 	if (!IS_HASH(rel))
-		elog(ERROR, "relation \"%s\" is not a HASH index",
-			 RelationGetRelationName(rel));
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("relation \"%s\" is not a HASH index",
+						RelationGetRelationName(rel))));
+
 
 	/*
 	 * Reject attempts to read non-local temporary relations; we would be
@@ -723,3 +743,21 @@ 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 06a1992bb17..b2432f43edf 100644
--- a/contrib/pgstattuple/pgstattuple.c
+++ b/contrib/pgstattuple/pgstattuple.c
@@ -293,6 +293,9 @@ pgstat_relation(Relation rel, FunctionCallInfo fcinfo)
 		case RELKIND_FOREIGN_TABLE:
 			err = "foreign table";
 			break;
+		case RELKIND_PARTITIONED_TABLE:
+			err = "partitioned table";
+			break;
 		default:
 			err = "unknown";
 			break;
diff --git a/contrib/pgstattuple/sql/pgstattuple.sql b/contrib/pgstattuple/sql/pgstattuple.sql
index 81fd5d693b4..25a0daa3312 100644
--- a/contrib/pgstattuple/sql/pgstattuple.sql
+++ b/contrib/pgstattuple/sql/pgstattuple.sql
@@ -51,3 +51,67 @@ select * from pgstatginindex('test_ginidx');
 create index test_hashidx on test using hash (b);
 
 select * from pgstathashindex('test_hashidx');
+
+-- these should error with the wrong type
+select pgstatginindex('test_pkey');
+select pgstathashindex('test_pkey');
+
+select pgstatindex('test_ginidx');
+select pgstathashindex('test_ginidx');
+
+select pgstatindex('test_hashidx');
+select pgstatginindex('test_hashidx');
+
+-- check that using any of these functions with unsupported relations will fail
+create table test_partitioned (a int) partition by range (a);
+-- these should all fail
+select pgstattuple('test_partitioned');
+select pgstattuple_approx('test_partitioned');
+select pg_relpages('test_partitioned');
+select pgstatindex('test_partitioned');
+select pgstatginindex('test_partitioned');
+select pgstathashindex('test_partitioned');
+
+create view test_view as select 1;
+-- these should all fail
+select pgstattuple('test_view');
+select pgstattuple_approx('test_view');
+select pg_relpages('test_view');
+select pgstatindex('test_view');
+select pgstatginindex('test_view');
+select pgstathashindex('test_view');
+
+create foreign data wrapper dummy;
+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');
+select pgstattuple_approx('test_foreign_table');
+select pg_relpages('test_foreign_table');
+select pgstatindex('test_foreign_table');
+select pgstatginindex('test_foreign_table');
+select pgstathashindex('test_foreign_table');
+
+-- a partition of a partitioned table should work though
+create table test_partition partition of test_partitioned for values from (1) to (100);
+select pgstattuple('test_partition');
+select pgstattuple_approx('test_partition');
+select pg_relpages('test_partition');
+
+-- not for the index calls though, of course
+select pgstatindex('test_partition');
+select pgstatginindex('test_partition');
+select pgstathashindex('test_partition');
+
+-- an actual index of a partitiond table should work though
+create index test_partition_idx on test_partition(a);
+create index test_partition_hash_idx on test_partition using hash (a);
+-- these should work
+select pgstatindex('test_partition_idx');
+select pgstathashindex('test_partition_hash_idx');
+
+drop table test_partitioned;
+drop view test_view;
+drop foreign table test_foreign_table;
+drop server dummy_server;
+drop foreign data wrapper dummy;