Michael Paquier wrote: > On Wed, May 02, 2018 at 01:38:22PM +0900, Amit Langote wrote: > > Perhaps, I'm just repeating what's already been said, but I think it might > > be better to have the word "partitioned" in the message. > > That's what Peter is pointing to upthread and what the v1 of upthread > was doing. I would tend to think to just keep the code simple and don't > add those extra checks, but I am fine to be beaten as well.
I pushed some fixes produced here. Attached is the remainder of the patch you submitted. I notice now that we haven't actually fixed Peter's source of complaint, though. AFAICS your patch just adds test cases, and upthread discussion apparently converges on not doing anything about the code. I'm not yet sure what to think of that ... -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 4399d0e7a69faa28403cc211c9b7d6495da23cf3 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Wed, 2 May 2018 10:56:29 +0900 Subject: [PATCH v3] 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/sql/btree.sql | 10 ++++++++++ contrib/pg_visibility/expected/pg_visibility.out | 13 ++++++++++++- contrib/pg_visibility/sql/pg_visibility.sql | 8 +++++++- contrib/pgstattuple/expected/pgstattuple.out | 10 ++++++++++ contrib/pgstattuple/sql/pgstattuple.sql | 5 +++++ 8 files changed, 84 insertions(+), 4 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/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/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 9858ea69d4..96311b8386 100644 --- a/contrib/pgstattuple/expected/pgstattuple.out +++ b/contrib/pgstattuple/expected/pgstattuple.out @@ -160,14 +160,24 @@ 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/sql/pgstattuple.sql b/contrib/pgstattuple/sql/pgstattuple.sql index cfa540302d..1459672cab 100644 --- a/contrib/pgstattuple/sql/pgstattuple.sql +++ b/contrib/pgstattuple/sql/pgstattuple.sql @@ -69,10 +69,15 @@ create index test_partitioned_index on test_partitioned(a); 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 -- 2.11.0