On Mon, Apr 23, 2018 at 05:29:59PM -0700, Peter Geoghegan wrote:
> It looks like local partitioned indexes are a problem for pageinspect:
>
> pg@regression[28736]=# select bt_metap('at_partitioned_b_idx');
> ERROR: relation "at_partitioned_b_idx" is not a btree indexOkay, I can see the point you are making here, in this case that's actually a btree index but it has no on-disk data, so let's improve the error handling. At the same time hash and gin functions don't make much effort either... > I gather that pageinspect simply didn't get the memo about the new > RELKIND_PARTITIONED_INDEX "placeholder" relkind. I think that it > should at least give a friendlier error message than what we see here. > The same applies to amcheck, and possibly a few other modules. So that's another c08d82f3... This needs to be really checked on a yearly-basis as the trend of the last releases is to add at least one new relkind per major version. There are three modules at stake here: - pg_visibility - pgstattuple - pageinspect I get to wonder about anything needed for sepgsql... Let's see that on a separate thread after I look at the problem. > I also noticed that the new 'I' relkind is not documented within the > pg_class docs. I think that that needs to be updated. Good catch. > The docs should > explain that 'I' relations are not backed by storage, since that will > probably affect users of one or two external tools. Hm, the docs about taking backups with the low-level APIs don't care much about relkind now: https://www.postgresql.org/docs/devel/static/continuous-archiving.html#BACKUP-LOWLEVEL-BASE-BACKUP Or do you have another section in the docs in mind? Does the attached patch address everything you have seen? I have tried to cope with anything I noticed as well on the way, which is made of: - pg_visibility should have tests for partitioned indexes, the code handles errors correctly. - in pageinspect, get_raw_page() fails for partitioned indexes: ERROR: could not open file "base/16384/16419": No such file or directory - in pgstattuple, error message is incorrect for both index-related and heap-related functions. Thanks, -- Michael
From e8133ea66a27cd46e187408429d79891e3358978 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 24 Apr 2018 13:53:45 +0900
Subject: [PATCH] Fix gaps in modules with handling of partitioned indexes
The following modules lacked handling for partitioned indexes:
- pgstattuple
- pg_visibility
- pageinspect
For some index-related functions, a partitioned index can be defined of
the same object type as what the function works on but still get a
failure, so tweak those code path so as they complain when trying to
work on partitioned indexes.
Test cases are added to cover all those additions.
---
contrib/pageinspect/btreefuncs.c | 18 ++++++++++++++++++
contrib/pageinspect/expected/btree.out | 12 ++++++++++++
contrib/pageinspect/expected/page.out | 6 +++++-
contrib/pageinspect/rawpage.c | 5 +++++
contrib/pageinspect/sql/btree.sql | 10 ++++++++++
contrib/pageinspect/sql/page.sql | 5 ++++-
.../pg_visibility/expected/pg_visibility.out | 13 ++++++++++++-
contrib/pg_visibility/sql/pg_visibility.sql | 8 +++++++-
contrib/pgstattuple/expected/pgstattuple.out | 13 +++++++++++++
contrib/pgstattuple/pgstatindex.c | 18 ++++++++++++++++++
contrib/pgstattuple/pgstattuple.c | 3 +++
contrib/pgstattuple/sql/pgstattuple.sql | 7 +++++++
doc/src/sgml/catalogs.sgml | 3 ++-
13 files changed, 116 insertions(+), 5 deletions(-)
diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index 558a8c41f4..9ceb2d0b81 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -180,6 +180,12 @@ bt_page_stats(PG_FUNCTION_ARGS)
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
rel = relation_openrv(relrv, AccessShareLock);
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot get summary information of B-tree page for partitioned index \"%s\"",
+ RelationGetRelationName(rel))));
+
if (!IS_INDEX(rel) || !IS_BTREE(rel))
elog(ERROR, "relation \"%s\" is not a btree index",
RelationGetRelationName(rel));
@@ -334,6 +340,12 @@ bt_page_items(PG_FUNCTION_ARGS)
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
rel = relation_openrv(relrv, AccessShareLock);
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot get information of B-tree items for partitioned index \"%s\"",
+ RelationGetRelationName(rel))));
+
if (!IS_INDEX(rel) || !IS_BTREE(rel))
elog(ERROR, "relation \"%s\" is not a btree index",
RelationGetRelationName(rel));
@@ -524,6 +536,12 @@ bt_metap(PG_FUNCTION_ARGS)
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
rel = relation_openrv(relrv, AccessShareLock);
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot get information of B-tree metapage for partitioned index \"%s\"",
+ RelationGetRelationName(rel))));
+
if (!IS_INDEX(rel) || !IS_BTREE(rel))
elog(ERROR, "relation \"%s\" is not a btree index",
RelationGetRelationName(rel));
diff --git a/contrib/pageinspect/expected/btree.out b/contrib/pageinspect/expected/btree.out
index 2aaa4df53b..7b135d18e7 100644
--- a/contrib/pageinspect/expected/btree.out
+++ b/contrib/pageinspect/expected/btree.out
@@ -58,3 +58,15 @@ data | 01 00 00 00 00 00 00 01
SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 2));
ERROR: block number 2 is out of range for relation "test1_a_idx"
DROP TABLE test1;
+-- Partitioned indexes, all should fail
+CREATE TABLE test_partitioned (a int) PARTITION BY RANGE (a);
+CREATE INDEX test_partitioned_index ON test_partitioned (a);
+SELECT bt_metap('test_partitioned_index');
+ERROR: cannot get information of B-tree metapage for partitioned index "test_partitioned_index"
+SELECT bt_page_stats('test_partitioned_index', 0);
+ERROR: cannot get summary information of B-tree page for partitioned index "test_partitioned_index"
+SELECT bt_page_items('test_partitioned_index', 0);
+ERROR: cannot get information of B-tree items for partitioned index "test_partitioned_index"
+SELECT * FROM bt_page_items(get_raw_page('test_partitioned_index', 0));
+ERROR: cannot get raw page from partitioned index "test_partitioned_index"
+DROP TABLE test_partitioned;
diff --git a/contrib/pageinspect/expected/page.out b/contrib/pageinspect/expected/page.out
index 5edb650085..3fcd9fbe6d 100644
--- a/contrib/pageinspect/expected/page.out
+++ b/contrib/pageinspect/expected/page.out
@@ -83,10 +83,14 @@ 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
+-- check that using any of these functions with a partitioned table or index
+-- would fail
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"
+select get_raw_page('test_partitioned_index', 0); -- error about partitioned index
+ERROR: cannot get raw page from partitioned index "test_partitioned_index"
-- 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 72f1d21e1b..d7bf782ccd 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -128,6 +128,11 @@ get_raw_page_internal(text *relname, ForkNumber forknum, BlockNumber blkno)
(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))));
/*
* Reject attempts to read non-local temporary relations; we would be
diff --git a/contrib/pageinspect/sql/btree.sql b/contrib/pageinspect/sql/btree.sql
index 8eac64c7b3..42e90d897d 100644
--- a/contrib/pageinspect/sql/btree.sql
+++ b/contrib/pageinspect/sql/btree.sql
@@ -19,3 +19,13 @@ SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 1));
SELECT * FROM bt_page_items(get_raw_page('test1_a_idx', 2));
DROP TABLE test1;
+
+-- Partitioned indexes, all should fail
+CREATE TABLE test_partitioned (a int) PARTITION BY RANGE (a);
+CREATE INDEX test_partitioned_index ON test_partitioned (a);
+SELECT bt_metap('test_partitioned_index');
+SELECT bt_page_stats('test_partitioned_index', 0);
+SELECT bt_page_items('test_partitioned_index', 0);
+SELECT * FROM bt_page_items(get_raw_page('test_partitioned_index', 0));
+
+DROP TABLE test_partitioned;
diff --git a/contrib/pageinspect/sql/page.sql b/contrib/pageinspect/sql/page.sql
index 8f35830e06..8ac9991837 100644
--- a/contrib/pageinspect/sql/page.sql
+++ b/contrib/pageinspect/sql/page.sql
@@ -33,9 +33,12 @@ 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
+-- check that using any of these functions with a partitioned table or index
+-- would fail
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
+select get_raw_page('test_partitioned_index', 0); -- error about partitioned index
-- 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);
diff --git a/contrib/pg_visibility/expected/pg_visibility.out b/contrib/pg_visibility/expected/pg_visibility.out
index f0dcb897c4..3500e83ebc 100644
--- a/contrib/pg_visibility/expected/pg_visibility.out
+++ b/contrib/pg_visibility/expected/pg_visibility.out
@@ -2,19 +2,30 @@ CREATE EXTENSION pg_visibility;
--
-- check that using the module's functions with unsupported relations will fail
--
--- partitioned tables (the parent ones) don't have visibility maps
+-- partitioned tables and indexes (the parent ones) don't have visibility maps
create table test_partitioned (a int) partition by list (a);
+create index test_partitioned_index on test_partitioned (a);
-- these should all fail
select pg_visibility('test_partitioned', 0);
ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
+select pg_visibility('test_partitioned_index', 0);
+ERROR: "test_partitioned_index" 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('test_partitioned_index');
+ERROR: "test_partitioned_index" 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_visibility_map_summary('test_partitioned_index');
+ERROR: "test_partitioned_index" 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_check_frozen('test_partitioned_index');
+ERROR: "test_partitioned_index" 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
+select pg_truncate_visibility_map('test_partitioned_index');
+ERROR: "test_partitioned_index" 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);
-- indexes do not, so these all fail
diff --git a/contrib/pg_visibility/sql/pg_visibility.sql b/contrib/pg_visibility/sql/pg_visibility.sql
index c2a7f1d9e4..648ee86d05 100644
--- a/contrib/pg_visibility/sql/pg_visibility.sql
+++ b/contrib/pg_visibility/sql/pg_visibility.sql
@@ -4,14 +4,20 @@ CREATE EXTENSION pg_visibility;
-- check that using the module's functions with unsupported relations will fail
--
--- partitioned tables (the parent ones) don't have visibility maps
+-- partitioned tables and indexes (the parent ones) don't have visibility maps
create table test_partitioned (a int) partition by list (a);
+create index test_partitioned_index on test_partitioned (a);
-- these should all fail
select pg_visibility('test_partitioned', 0);
+select pg_visibility('test_partitioned_index', 0);
select pg_visibility_map('test_partitioned');
+select pg_visibility_map('test_partitioned_index');
select pg_visibility_map_summary('test_partitioned');
+select pg_visibility_map_summary('test_partitioned_index');
select pg_check_frozen('test_partitioned');
+select pg_check_frozen('test_partitioned_index');
select pg_truncate_visibility_map('test_partitioned');
+select pg_truncate_visibility_map('test_partitioned_index');
create table test_partition partition of test_partitioned for values in (1);
create index test_index on test_partition (a);
diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.out
index a7087f6d45..c0bbd1440b 100644
--- a/contrib/pgstattuple/expected/pgstattuple.out
+++ b/contrib/pgstattuple/expected/pgstattuple.out
@@ -152,19 +152,32 @@ 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);
+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
+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
+select pgstattuple_approx('test_partitioned_index');
+ERROR: "test_partitioned_index" 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 pg_relpages('test_partitioned_index');
+ERROR: "test_partitioned_index" 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 pgstatindex('test_partitioned_index');
+ERROR: "test_partitioned_index" is a partitioned index
select pgstatginindex('test_partitioned');
ERROR: relation "test_partitioned" is not a GIN index
+select pgstatginindex('test_partitioned_index');
+ERROR: "test_partitioned_index" is a partitioned index
select pgstathashindex('test_partitioned');
ERROR: "test_partitioned" is not an index
+select pgstathashindex('test_partitioned_index');
+ERROR: "test_partitioned_index" is a partitioned index
create view test_view as select 1;
-- these should all fail
select pgstattuple('test_view');
diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.c
index 75317b96a2..3b99ed46c7 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -220,6 +220,12 @@ pgstatindex_impl(Relation rel, FunctionCallInfo fcinfo)
BTIndexStat indexStat;
BufferAccessStrategy bstrategy = GetAccessStrategy(BAS_BULKREAD);
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is a partitioned index",
+ RelationGetRelationName(rel))));
+
if (!IS_INDEX(rel) || !IS_BTREE(rel))
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
@@ -521,6 +527,12 @@ pgstatginindex_internal(Oid relid, FunctionCallInfo fcinfo)
rel = relation_open(relid, AccessShareLock);
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is a partitioned index",
+ RelationGetRelationName(rel))));
+
if (!IS_INDEX(rel) || !IS_GIN(rel))
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
@@ -597,6 +609,12 @@ pgstathashindex(PG_FUNCTION_ARGS)
rel = index_open(relid, AccessShareLock);
+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is a partitioned index",
+ RelationGetRelationName(rel))));
+
/* index_open() checks that it's an index */
if (!IS_HASH(rel))
ereport(ERROR,
diff --git a/contrib/pgstattuple/pgstattuple.c b/contrib/pgstattuple/pgstattuple.c
index b599b6ca21..6d67bd8271 100644
--- a/contrib/pgstattuple/pgstattuple.c
+++ b/contrib/pgstattuple/pgstattuple.c
@@ -296,6 +296,9 @@ pgstat_relation(Relation rel, FunctionCallInfo fcinfo)
case RELKIND_PARTITIONED_TABLE:
err = "partitioned table";
break;
+ case RELKIND_PARTITIONED_INDEX:
+ err = "partitioned index";
+ break;
default:
err = "unknown";
break;
diff --git a/contrib/pgstattuple/sql/pgstattuple.sql b/contrib/pgstattuple/sql/pgstattuple.sql
index a8e341e351..1459672cab 100644
--- a/contrib/pgstattuple/sql/pgstattuple.sql
+++ b/contrib/pgstattuple/sql/pgstattuple.sql
@@ -64,13 +64,20 @@ 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);
+create index test_partitioned_index on test_partitioned(a);
-- these should all fail
select pgstattuple('test_partitioned');
+select pgstattuple('test_partitioned_index');
select pgstattuple_approx('test_partitioned');
+select pgstattuple_approx('test_partitioned_index');
select pg_relpages('test_partitioned');
+select pg_relpages('test_partitioned_index');
select pgstatindex('test_partitioned');
+select pgstatindex('test_partitioned_index');
select pgstatginindex('test_partitioned');
+select pgstatginindex('test_partitioned_index');
select pgstathashindex('test_partitioned');
+select pgstathashindex('test_partitioned_index');
create view test_view as select 1;
-- these should all fail
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 14aeed3076..91003522f2 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -1840,7 +1840,8 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
<literal>m</literal> = materialized view,
<literal>c</literal> = composite type,
<literal>f</literal> = foreign table,
- <literal>p</literal> = partitioned table
+ <literal>p</literal> = partitioned table,
+ <literal>I</literal> = partitioned index
</entry>
</row>
--
2.17.0
signature.asc
Description: PGP signature
