On Tue, May 01, 2018 at 12:30:44PM -0400, Robert Haas wrote: > That's probably going to cause some translation problems. The form of > "a" that you need will vary: "a" vs. "an" in English, "un" vs. "una" > in Spanish, etc. And it wouldn't be surprising if there are problems > in some languages even if you make it "This relation is %s". There's > a reason why the documentation advises against building up > translatable strings from constituent parts. If we go this route, > it's probably best to separately translate "This relation is a > table.", "This relation is an index.", etc.
Yeah, I get the feeling that this is not going to be much consistent-proof either, so while I have not been able to come up with a better idea, let's not go through this route. > However, backing up a minute, I don't think "relation \"%s\" is not a > btree index" is such a terrible message. These modules are intended > to be intended by people who Know What They Are Doing. If we do want > to change the message, I submit that the only thing that makes it a > little unclear is that a user might fail to realize that a partitioned > index is not an index. But that could be fixed just by adding a > separate message for that one case (index \"%s\" is partitioned) and > sticking with the existing message for other cases. I have been chewing on that, and I come to agree that there is perhaps little point to complicate the code as long as a failure is properly reported to the user. I propose hence the attached, which adds test cases in the contrib module set for partitioned indexes (amcheck also lacked tests for partition tables and indexes), and fixes a set of code paths to be consistent with the presence of this new relkind. Visibly I have found a bug in git, format-patch reports the following line in the stats: .../pg_visibility/expected/pg_visibility.out | 13 ++++++++++++- But the patch contents are actually correct... -- Michael
From f71b4d1932fd6da3a679f9d755f387f5986c71e8 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Wed, 2 May 2018 10:56:29 +0900
Subject: [PATCH] Fix gaps in modules with handling of partitioned indexes
The following modules lacked handling and/or coverage for partitioned
indexes:
- pgstattuple
- pg_visibility
- pageinspect
- amcheck
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, still the error messages are kept the same to keep the code
simple.
Test cases are added to cover all those additions.
---
contrib/amcheck/expected/check_btree.out | 17 ++++++++++++++++-
contrib/amcheck/sql/check_btree.sql | 13 ++++++++++++-
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 | 1 -
contrib/pgstattuple/pgstattuple.c | 3 +++
contrib/pgstattuple/sql/pgstattuple.sql | 7 +++++++
doc/src/sgml/catalogs.sgml | 3 ++-
14 files changed, 108 insertions(+), 8 deletions(-)
diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out
index e864579774..b288565be9 100644
--- a/contrib/amcheck/expected/check_btree.out
+++ b/contrib/amcheck/expected/check_btree.out
@@ -2,7 +2,9 @@ CREATE TABLE bttest_a(id int8);
CREATE TABLE bttest_b(id int8);
CREATE TABLE bttest_multi(id int8, data int8);
CREATE TABLE delete_test_table (a bigint, b bigint, c bigint, d bigint);
--- Stabalize tests
+CREATE TABLE bttest_partitioned (a int) PARTITION BY RANGE (a);
+CREATE INDEX bttest_partitioned_index ON bttest_partitioned (a);
+-- Stabilize tests
ALTER TABLE bttest_a SET (autovacuum_enabled = false);
ALTER TABLE bttest_b SET (autovacuum_enabled = false);
ALTER TABLE bttest_multi SET (autovacuum_enabled = false);
@@ -48,6 +50,18 @@ SELECT bt_index_check('bttest_a');
ERROR: "bttest_a" is not an index
SELECT bt_index_parent_check('bttest_a');
ERROR: "bttest_a" is not an index
+-- verify partitioned tables are rejected (error)
+SELECT bt_index_check('bttest_partitioned');
+ERROR: "bttest_partitioned" is not an index
+SELECT bt_index_parent_check('bttest_partitioned');
+ERROR: "bttest_partitioned" is not an index
+-- verify partitioned indexes are rejected (error)
+SELECT bt_index_check('bttest_partitioned_index');
+ERROR: only B-Tree indexes are supported as targets for verification
+DETAIL: Relation "bttest_partitioned_index" is not a B-Tree index.
+SELECT bt_index_parent_check('bttest_partitioned_index');
+ERROR: only B-Tree indexes are supported as targets for verification
+DETAIL: Relation "bttest_partitioned_index" is not a B-Tree index.
-- verify non-existing indexes are rejected (error)
SELECT bt_index_check(17);
ERROR: could not open relation with OID 17
@@ -145,5 +159,6 @@ DROP TABLE bttest_a;
DROP TABLE bttest_b;
DROP TABLE bttest_multi;
DROP TABLE delete_test_table;
+DROP TABLE bttest_partitioned;
DROP OWNED BY bttest_role; -- permissions
DROP ROLE bttest_role;
diff --git a/contrib/amcheck/sql/check_btree.sql b/contrib/amcheck/sql/check_btree.sql
index 7b1ab4f148..cad496eda8 100644
--- a/contrib/amcheck/sql/check_btree.sql
+++ b/contrib/amcheck/sql/check_btree.sql
@@ -2,8 +2,10 @@ CREATE TABLE bttest_a(id int8);
CREATE TABLE bttest_b(id int8);
CREATE TABLE bttest_multi(id int8, data int8);
CREATE TABLE delete_test_table (a bigint, b bigint, c bigint, d bigint);
+CREATE TABLE bttest_partitioned (a int) PARTITION BY RANGE (a);
+CREATE INDEX bttest_partitioned_index ON bttest_partitioned (a);
--- Stabalize tests
+-- Stabilize tests
ALTER TABLE bttest_a SET (autovacuum_enabled = false);
ALTER TABLE bttest_b SET (autovacuum_enabled = false);
ALTER TABLE bttest_multi SET (autovacuum_enabled = false);
@@ -42,6 +44,14 @@ RESET ROLE;
SELECT bt_index_check('bttest_a');
SELECT bt_index_parent_check('bttest_a');
+-- verify partitioned tables are rejected (error)
+SELECT bt_index_check('bttest_partitioned');
+SELECT bt_index_parent_check('bttest_partitioned');
+
+-- verify partitioned indexes are rejected (error)
+SELECT bt_index_check('bttest_partitioned_index');
+SELECT bt_index_parent_check('bttest_partitioned_index');
+
-- verify non-existing indexes are rejected (error)
SELECT bt_index_check(17);
SELECT bt_index_parent_check(17);
@@ -93,5 +103,6 @@ DROP TABLE bttest_a;
DROP TABLE bttest_b;
DROP TABLE bttest_multi;
DROP TABLE delete_test_table;
+DROP TABLE bttest_partitioned;
DROP OWNED BY bttest_role; -- permissions
DROP ROLE bttest_role;
diff --git a/contrib/pageinspect/expected/btree.out b/contrib/pageinspect/expected/btree.out
index 2aaa4df53b..cb2f04874b 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: relation "test_partitioned_index" is not a btree index
+SELECT bt_page_stats('test_partitioned_index', 0);
+ERROR: relation "test_partitioned_index" is not a btree index
+SELECT bt_page_items('test_partitioned_index', 0);
+ERROR: relation "test_partitioned_index" is not a btree 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..4b126d3be8 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: relation "test_partitioned_index" is not a btree index
select pgstatginindex('test_partitioned');
ERROR: relation "test_partitioned" is not a GIN index
+select pgstatginindex('test_partitioned_index');
+ERROR: relation "test_partitioned_index" is not a GIN index
select pgstathashindex('test_partitioned');
ERROR: "test_partitioned" is not an index
+select pgstathashindex('test_partitioned_index');
+ERROR: relation "test_partitioned_index" is not a HASH 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..3d50262f83 100644
--- a/contrib/pgstattuple/pgstatindex.c
+++ b/contrib/pgstattuple/pgstatindex.c
@@ -604,7 +604,6 @@ pgstathashindex(PG_FUNCTION_ARGS)
errmsg("relation \"%s\" is not a HASH index",
RelationGetRelationName(rel))));
-
/*
* Reject attempts to read non-local temporary relations; we would be
* likely to get wrong data since we have no visibility into the owning
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 26984b6cba..16181db926 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
