On 2017/02/13 14:59, Michael Paquier wrote:
> On Fri, Feb 10, 2017 at 4:29 PM, Amit Langote wrote:
>> On 2017/02/10 14:32, Michael Paquier wrote:
>>> The visibility checks are localized in pg_visibility.c and the storage
>>> checks in pgstatindex.c, so yes we could have macros in those files.
>>> Or even better: just have a sanity check routine as the error messages
>>> are the same everywhere.
>>
>> If I understand correctly what's being proposed here, tablecmds.c has
>> something called ATWrongRelkindError() which sounds (kind of) similar.
>> It's called by ATSimplePermissions() whenever it finds out that relkind of
>> the table specified in a given ALTER TABLE command is not in the "allowed
>> relkind targets" for the command. Different ALTER TABLE commands call
>> ATSimplePermissions() with an argument that specifies the "allowed relkind
>> targets" for each command. I'm not saying that it should be used
>> elsewhere, but if we do invent a new centralized routine for relkind
>> checks, it would look similar.
>
> You did not get completely what I wrote upthread. This check is
> repeated 3 times in pg_visibility.c:
> + /* Other relkinds don't have visibility info */
> + if (rel->rd_rel->relkind != RELKIND_RELATION &&
> + rel->rd_rel->relkind != RELKIND_MATVIEW &&
> + 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))));
>
> And this one is present 4 times in pgstatindex.c:
> + /* only the following relkinds have storage */
> + 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_FEATURE_NOT_SUPPORTED),
> + errmsg("\"%s\" is not a table, index, materialized
> view, sequence, or TOAST table",
> + RelationGetRelationName(rel))));
>
> So the suggestion is simply to encapsulate such blocks into their own
> function like check_relation_relkind, each one locally in their
> respective contrib's file. And each function includes the error
> message, which should use ERRCODE_WRONG_OBJECT_TYPE as errcode by the
> way.
I see, thanks for explaining. Implemented that in the attached, also
fixing the errcode. Please see if that's what you had in mind.
I was tempted for a moment to name the functions
check_relation_has_storage() with the message "relation %s has no storage"
and check_relation_has_visibility_info() with a similar message, but soon
got over it. ;)
>>>> No new regression tests. I think we should create a dummy partitioned table
>>>> for each contrib and show that it fails.
>>>
>>> Yep, good point. That's easy enough to add.
>>
>> I added tests with a partitioned table to pageinspect's page.sql and
>> pgstattuple.sql. I don't see any tests for pg_visibility module, maybe I
>> should add?
>
> Checking those error code paths would be nice. It would be nice as
> well that the relation types that are expected to be supported and
> unsupported are checked. For pageinspect the check range is correct.
> There are some relkinds missing in pgstatindex though.
Added more tests in pgstattuple and the new ones for pg_visibility,
although I may have overdone the latter.
>> Although, I felt a bit uncomfortable the way the error message looks, for
>> example:
>>
>> + -- check that using any of these functions with a partitioned table
>> would fail
>> + create table test_partitioned (a int) partition by range (a);
>> + select pg_relpages('test_partitioned');
>> + ERROR: "test_partitioned" is not a table, index, materialized view,
>> sequence, or TOAST table
>
> Would it be a problem to mention this relkind as just "partitioned
> table" in the error message.
In certain contexts where a subset of relkinds are allowed and others are
not or vice versa, partitioned tables are still referred to as "tables".
That's because we still use CREATE/DROP TABLE to create/drop them and
perhaps more to the point, their being partitioned is irrelevant.
Examples of where partitioned tables are referred to as tables:
1. The following check in get_relation_by_qualified_name():
case OBJECT_TABLE:
if (relation->rd_rel->relkind != RELKIND_RELATION &&
relation->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("\"%s\" is not a table",
RelationGetRelationName(relation))));
2. The following in DefineQueryRewrite() (from the rewriter's point of view):
/*
* Verify relation is of a type that rules can sensibly be applied to.
* Internal callers can target materialized views, but transformRuleStmt()
* blocks them for users. Don't mention them in the error message.
*/
if (event_relation->rd_rel->relkind != RELKIND_RELATION &&
event_relation->rd_rel->relkind != RELKIND_MATVIEW &&
event_relation->rd_rel->relkind != RELKIND_VIEW &&
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))));
In other contexts, where a table's being partitioned is relevant, the
message is shown as "relation is/is not partitioned table". Examples:
1. The following check in DefineIndex():
else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot create index on partitioned table \"%s\"",
RelationGetRelationName(rel))));
2. One in get_raw_page_internal():
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))));
3. On the other hand, the following check in transformAttachPartition()
prevents an attempt to attach a partition to a a regular table:
if (parentRel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("\"%s\" is not partitioned",
RelationGetRelationName(parentRel))));
>> test_partitioned IS a table but just the kind without storage. This is
>> not the only place however where we do not separate the check for
>> partitioned tables from other unsupported relkinds in respective contexts.
>> Not sure if that would be a better idea.
>
> Well, perhaps I am playing with the words in my last paragraph, but
> the documentation clearly states that any relation defined with
> PARTITION BY is a "partitioned table".
Yes, however as I tried to describe above, the term used in error messages
differs from context to context. I can see that a more consistent
user-facing terminology is important irrespective of the internal
implementation details.
Updated patch attached.
Thanks,
Amit
>From b5868a099f46acd8f522f6e93768176526cc001b Mon Sep 17 00:00:00 2001
From: amit <[email protected]>
Date: Tue, 24 Jan 2017 13:22:17 +0900
Subject: [PATCH] Add relkind checks to certain contrib modules
Contrib functions such a pg_visibility, pg_relpages are only applicable
to certain relkinds; error out if otherwise.
Also, RELKIND_PARTITIONED_TABLE was not added to the pageinspect's and
pgstattuple's list of unsupported relkinds.
Add tests for the newly added checks.
---
contrib/pageinspect/expected/page.out | 5 ++
contrib/pageinspect/rawpage.c | 5 ++
contrib/pageinspect/sql/page.sql | 5 ++
contrib/pg_visibility/.gitignore | 4 ++
contrib/pg_visibility/Makefile | 2 +
contrib/pg_visibility/expected/pg_visibility.out | 68 ++++++++++++++++++++++++
contrib/pg_visibility/pg_visibility.c | 44 ++++++++++-----
contrib/pg_visibility/sql/pg_visibility.sql | 49 +++++++++++++++++
contrib/pgstattuple/expected/pgstattuple.out | 29 ++++++++++
contrib/pgstattuple/pgstatindex.c | 30 +++++++++++
contrib/pgstattuple/pgstattuple.c | 3 ++
contrib/pgstattuple/sql/pgstattuple.sql | 24 +++++++++
12 files changed, 254 insertions(+), 14 deletions(-)
create mode 100644 contrib/pg_visibility/.gitignore
create mode 100644 contrib/pg_visibility/expected/pg_visibility.out
create mode 100644 contrib/pg_visibility/sql/pg_visibility.sql
diff --git a/contrib/pageinspect/expected/page.out b/contrib/pageinspect/expected/page.out
index 13964cd878..85b183db4d 100644
--- a/contrib/pageinspect/expected/page.out
+++ b/contrib/pageinspect/expected/page.out
@@ -71,3 +71,8 @@ SELECT * FROM fsm_page_contents(get_raw_page('test1', 'fsm', 0));
(1 row)
DROP TABLE test1;
+-- check that using any of these functions with a partitioned table would fail
+create table test_partitioned (a int) partition by range (a);
+select get_raw_page('test_partitioned', 0);
+ERROR: cannot get raw page from partitioned table "test_partitioned"
+drop table test_partitioned;
diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index 102f360c6f..1ccc3ff320 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -123,6 +123,11 @@ get_raw_page_internal(text *relname, ForkNumber forknum, BlockNumber blkno)
(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))));
/*
* Reject attempts to read non-local temporary relations; we would be
diff --git a/contrib/pageinspect/sql/page.sql b/contrib/pageinspect/sql/page.sql
index 97eef9829a..fa3533f2af 100644
--- a/contrib/pageinspect/sql/page.sql
+++ b/contrib/pageinspect/sql/page.sql
@@ -28,3 +28,8 @@ SELECT tuple_data_split('test1'::regclass, t_data, t_infomask, t_infomask2, t_bi
SELECT * FROM fsm_page_contents(get_raw_page('test1', 'fsm', 0));
DROP TABLE test1;
+
+-- check that using any of these functions with a partitioned table would fail
+create table test_partitioned (a int) partition by range (a);
+select get_raw_page('test_partitioned', 0);
+drop table test_partitioned;
diff --git a/contrib/pg_visibility/.gitignore b/contrib/pg_visibility/.gitignore
new file mode 100644
index 0000000000..5dcb3ff972
--- /dev/null
+++ b/contrib/pg_visibility/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/contrib/pg_visibility/Makefile b/contrib/pg_visibility/Makefile
index 379591a098..bc42944426 100644
--- a/contrib/pg_visibility/Makefile
+++ b/contrib/pg_visibility/Makefile
@@ -7,6 +7,8 @@ EXTENSION = pg_visibility
DATA = pg_visibility--1.1.sql pg_visibility--1.0--1.1.sql
PGFILEDESC = "pg_visibility - page visibility information"
+REGRESS = pg_visibility
+
ifdef USE_PGXS
PG_CONFIG = pg_config
PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/contrib/pg_visibility/expected/pg_visibility.out b/contrib/pg_visibility/expected/pg_visibility.out
new file mode 100644
index 0000000000..7aadf24a8b
--- /dev/null
+++ b/contrib/pg_visibility/expected/pg_visibility.out
@@ -0,0 +1,68 @@
+CREATE EXTENSION pg_visibility;
+--
+-- check that using the module's functions with unsupported relations will fail
+--
+create table test_partitioned (a int) partition by list (a);
+select pg_visibility('test_partitioned', 0);
+ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+select pg_visibility_map('test_partitioned');
+ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+select pg_visibility_map_summary('test_partitioned');
+ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+select pg_check_frozen('test_partitioned');
+ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+select pg_truncate_visibility_map('test_partitioned');
+ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+create table test_partition partition of test_partitioned for values in (1);
+create index test_index on test_partition (a);
+select pg_visibility('test_index', 0);
+ERROR: "test_index" is not a table, materialized view, or TOAST table
+select pg_visibility_map('test_index');
+ERROR: "test_index" is not a table, materialized view, or TOAST table
+select pg_visibility_map_summary('test_index');
+ERROR: "test_index" is not a table, materialized view, or TOAST table
+select pg_check_frozen('test_index');
+ERROR: "test_index" is not a table, materialized view, or TOAST table
+select pg_truncate_visibility_map('test_index');
+ERROR: "test_index" is not a table, materialized view, or TOAST table
+create view test_view as select 1;
+select pg_visibility('test_view', 0);
+ERROR: "test_view" is not a table, materialized view, or TOAST table
+select pg_visibility_map('test_view');
+ERROR: "test_view" is not a table, materialized view, or TOAST table
+select pg_visibility_map_summary('test_view');
+ERROR: "test_view" is not a table, materialized view, or TOAST table
+select pg_check_frozen('test_view');
+ERROR: "test_view" is not a table, materialized view, or TOAST table
+select pg_truncate_visibility_map('test_view');
+ERROR: "test_view" is not a table, materialized view, or TOAST table
+create sequence test_sequence;
+select pg_visibility('test_sequence', 0);
+ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+select pg_visibility_map('test_sequence');
+ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+select pg_visibility_map_summary('test_sequence');
+ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+select pg_check_frozen('test_sequence');
+ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+select pg_truncate_visibility_map('test_sequence');
+ERROR: "test_sequence" is not a table, materialized view, or TOAST table
+create foreign data wrapper dummy;
+create server dummy_server foreign data wrapper dummy;
+create foreign table test_foreign_table () server dummy_server;
+select pg_visibility('test_foreign_table', 0);
+ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
+select pg_visibility_map('test_foreign_table');
+ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
+select pg_visibility_map_summary('test_foreign_table');
+ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
+select pg_check_frozen('test_foreign_table');
+ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
+select pg_truncate_visibility_map('test_foreign_table');
+ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
+drop table test_partitioned, test_partition;
+drop view test_view;
+drop sequence test_sequence;
+drop foreign table test_foreign_table;
+drop server dummy_server;
+drop foreign data wrapper dummy;
diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c
index 5580637870..2ecfa3c7d0 100644
--- a/contrib/pg_visibility/pg_visibility.c
+++ b/contrib/pg_visibility/pg_visibility.c
@@ -95,6 +95,22 @@ pg_visibility_map(PG_FUNCTION_ARGS)
}
/*
+ * 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_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))));
+}
+
+/*
* Visibility map information for a single block of a relation, plus the
* page-level information for the same block.
*/
@@ -114,6 +130,9 @@ pg_visibility(PG_FUNCTION_ARGS)
rel = relation_open(relid, AccessShareLock);
+ /* Only some relkind have the visibility info */
+ check_relation_relkind(rel);
+
if (blkno < 0 || blkno > MaxBlockNumber)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -257,6 +276,10 @@ pg_visibility_map_summary(PG_FUNCTION_ARGS)
bool nulls[2];
rel = relation_open(relid, AccessShareLock);
+
+ /* Only some relkind have the visibility info */
+ check_relation_relkind(rel);
+
nblocks = RelationGetNumberOfBlocks(rel);
for (blkno = 0; blkno < nblocks; ++blkno)
@@ -369,13 +392,8 @@ pg_truncate_visibility_map(PG_FUNCTION_ARGS)
rel = relation_open(relid, AccessExclusiveLock);
- 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))));
+ /* Only some relkind have the visibility info */
+ check_relation_relkind(rel);
RelationOpenSmgr(rel);
rel->rd_smgr->smgr_vm_nblocks = InvalidBlockNumber;
@@ -464,6 +482,9 @@ collect_visibility_data(Oid relid, bool include_pd)
rel = relation_open(relid, AccessShareLock);
+ /* Only some relkind have the visibility info */
+ check_relation_relkind(rel);
+
nblocks = RelationGetNumberOfBlocks(rel);
info = palloc0(offsetof(vbits, bits) +nblocks);
info->next = 0;
@@ -543,13 +564,8 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
rel = relation_open(relid, AccessShareLock);
- 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))));
+ /* Only some relkind have the visibility info */
+ check_relation_relkind(rel);
nblocks = RelationGetNumberOfBlocks(rel);
diff --git a/contrib/pg_visibility/sql/pg_visibility.sql b/contrib/pg_visibility/sql/pg_visibility.sql
new file mode 100644
index 0000000000..2e6c008c39
--- /dev/null
+++ b/contrib/pg_visibility/sql/pg_visibility.sql
@@ -0,0 +1,49 @@
+CREATE EXTENSION pg_visibility;
+
+--
+-- check that using the module's functions with unsupported relations will fail
+--
+create table test_partitioned (a int) partition by list (a);
+select pg_visibility('test_partitioned', 0);
+select pg_visibility_map('test_partitioned');
+select pg_visibility_map_summary('test_partitioned');
+select pg_check_frozen('test_partitioned');
+select pg_truncate_visibility_map('test_partitioned');
+
+create table test_partition partition of test_partitioned for values in (1);
+create index test_index on test_partition (a);
+select pg_visibility('test_index', 0);
+select pg_visibility_map('test_index');
+select pg_visibility_map_summary('test_index');
+select pg_check_frozen('test_index');
+select pg_truncate_visibility_map('test_index');
+
+create view test_view as select 1;
+select pg_visibility('test_view', 0);
+select pg_visibility_map('test_view');
+select pg_visibility_map_summary('test_view');
+select pg_check_frozen('test_view');
+select pg_truncate_visibility_map('test_view');
+
+create sequence test_sequence;
+select pg_visibility('test_sequence', 0);
+select pg_visibility_map('test_sequence');
+select pg_visibility_map_summary('test_sequence');
+select pg_check_frozen('test_sequence');
+select pg_truncate_visibility_map('test_sequence');
+
+create foreign data wrapper dummy;
+create server dummy_server foreign data wrapper dummy;
+create foreign table test_foreign_table () server dummy_server;
+select pg_visibility('test_foreign_table', 0);
+select pg_visibility_map('test_foreign_table');
+select pg_visibility_map_summary('test_foreign_table');
+select pg_check_frozen('test_foreign_table');
+select pg_truncate_visibility_map('test_foreign_table');
+
+drop table test_partitioned, test_partition;
+drop view test_view;
+drop sequence test_sequence;
+drop foreign table test_foreign_table;
+drop server dummy_server;
+drop foreign data wrapper dummy;
diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.out
index 169d1932b2..dfd15b0c63 100644
--- a/contrib/pgstattuple/expected/pgstattuple.out
+++ b/contrib/pgstattuple/expected/pgstattuple.out
@@ -138,3 +138,32 @@ select * from pgstathashindex('test_hashidx');
2 | 4 | 0 | 1 | 0 | 0 | 0 | 100
(1 row)
+-- check that using any of these functions with unsupported relations will fail
+create table test_partitioned (a int) partition by range (a);
+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
+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
+select pg_relpages('test_view');
+ERROR: "test_view" is not a table, index, materialized view, sequence, or TOAST table
+create foreign data wrapper dummy;
+create server dummy_server foreign data wrapper dummy;
+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
+select pg_relpages('test_foreign_table');
+ERROR: "test_foreign_table" is not a table, index, materialized view, sequence, or TOAST table
+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 17a53e3bb7..04b20915f8 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -361,6 +361,24 @@ pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo)
return result;
}
+/*
+ * 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))));
+}
+
/* --------------------------------------------------------
* pg_relpages()
*
@@ -388,6 +406,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 +430,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 +457,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 +479,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);
diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c
index 06a1992bb1..b2432f43ed 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 81fd5d693b..e42ac5db95 100644
--- a/contrib/pgstattuple/sql/pgstattuple.sql
+++ b/contrib/pgstattuple/sql/pgstattuple.sql
@@ -51,3 +51,27 @@ select * from pgstatginindex('test_ginidx');
create index test_hashidx on test using hash (b);
select * from pgstathashindex('test_hashidx');
+
+-- check that using any of these functions with unsupported relations will fail
+create table test_partitioned (a int) partition by range (a);
+select pgstattuple('test_partitioned');
+select pgstattuple_approx('test_partitioned');
+select pg_relpages('test_partitioned');
+
+create view test_view as select 1;
+select pgstattuple('test_view');
+select pgstattuple_approx('test_view');
+select pg_relpages('test_view');
+
+create foreign data wrapper dummy;
+create server dummy_server foreign data wrapper dummy;
+create foreign table test_foreign_table () server dummy_server;
+select pgstattuple('test_foreign_table');
+select pgstattuple_approx('test_foreign_table');
+select pg_relpages('test_foreign_table');
+
+drop table test_partitioned;
+drop view test_view;
+drop foreign table test_foreign_table;
+drop server dummy_server;
+drop foreign data wrapper dummy;
--
2.11.0
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
