From: Alexander Lakhin <exclusion@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
To: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table
Date: Fri, 15 Dec 2023 16:00:01 +0300
Message-ID: <11c0916f-82de-decf-c27a-c01a4f8dfcef@gmail.com> (raw)
In-Reply-To: <876b6e52-f068-648a-3f0e-7eaabd57cf77@gmail.com>
References: <18246-f4d9ff7cb3af77e6@postgresql.org>
<ZXm9Js8D_W_cFWzW@paquier.xyz>
<876b6e52-f068-648a-3f0e-7eaabd57cf77@gmail.com>
13.12.2023 21:00, Alexander Lakhin wrote:
> Hello Michael,
>
> 13.12.2023 17:18, Michael Paquier wrote:
>> On Wed, Dec 13, 2023 at 09:00:01AM +0000, PG Bug reporting form wrote:
>>> This anomaly can be observed since 8b08f7d48 from 2018-01-19, but IMO the
>>> culprit is e759854a0 from 2017-02-03, which introduced the following
>>> asymmetry in pgstatindex.c:
>>> if (!IS_INDEX(rel) || !IS_BTREE(rel))
>>> if (!IS_INDEX(rel) || !IS_GIN(rel))
>>> But:
>>> if (!IS_HASH(rel))
>> Fun, let's fix that. Would you like to write a patch?
>
> Yes, please look at the attached,
Looking around, I found the similar index_open() usage within pageinspect,
function hash_bitmap_info():
CREATE EXTENSION pageinspect;
CREATE TABLE t (a int, b int[], c int) PARTITION BY RANGE (a);
CREATE INDEX c_idx ON t USING hash(c);
SELECT hash_bitmap_info('c_idx'::regclass, 1);
Leads to:
TRAP: failed Assert("false"), File: "bufmgr.c", Line: 3606, PID: 3984550
I'm inclined to leave index_open() there for the internal consistency
inside pageinspect, and to use the same condition as in
bt_index_block_validate(), which called by functions bt_page_stats(...),
bt_multi_page_stats(...), bt_page_items(relname text, blkno bigint).
Functions for other index types, brin/gin/gist, don't read the index,
instead they receive a page as input.
Please look at the additional patch.
Best regards,
Alexander
Attachments:
[text/x-patch] v1-fix-hash_bitmap_info.patch (792B, ../11c0916f-82de-decf-c27a-c01a4f8dfcef@gmail.com/2-v1-fix-hash_bitmap_info.patch)
download | inline diff:diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.cindex f43f0928b1..59af47ffe7 100644--- a/contrib/pageinspect/hashfuncs.c+++ b/contrib/pageinspect/hashfuncs.c@@ -27,6 +27,7 @@ PG_FUNCTION_INFO_V1(hash_page_items);
PG_FUNCTION_INFO_V1(hash_bitmap_info);
PG_FUNCTION_INFO_V1(hash_metapage_info);
+#define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
#define IS_HASH(r) ((r)->rd_rel->relam == HASH_AM_OID)
/* ------------------------------------------------
@@ -415,7 +416,7 @@ hash_bitmap_info(PG_FUNCTION_ARGS)
indexRel = index_open(indexRelid, AccessShareLock);
- if (!IS_HASH(indexRel))+ if (!IS_INDEX(indexRel) || !IS_HASH(indexRel))
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("\"%s\" is not a %s index",
[text/x-patch] v1-fix-pgstathashindex.patch (4.2K, ../11c0916f-82de-decf-c27a-c01a4f8dfcef@gmail.com/3-v1-fix-pgstathashindex.patch)
download | inline diff:diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.outindex e4ac86f9e3..283856e109 100644--- a/contrib/pgstattuple/expected/pgstattuple.out+++ b/contrib/pgstattuple/expected/pgstattuple.out@@ -153,6 +153,7 @@ 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);
+create index test_partitioned_hash_index on test_partitioned using hash(a);
-- these should all fail
select pgstattuple('test_partitioned');
ERROR: cannot get tuple-level statistics for relation "test_partitioned"
@@ -171,7 +172,9 @@ 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+ERROR: relation "test_partitioned" is not a hash index+select pgstathashindex('test_partitioned_hash_index');+ERROR: relation "test_partitioned_hash_index" is not a hash index
create view test_view as select 1;
-- these should all fail
select pgstattuple('test_view');
@@ -188,7 +191,7 @@ 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+ERROR: relation "test_view" is not a hash index
create foreign data wrapper dummy;
create server dummy_server foreign data wrapper dummy;
create foreign table test_foreign_table () server dummy_server;
@@ -207,7 +210,7 @@ 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+ERROR: relation "test_foreign_table" is not a hash 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');
@@ -253,7 +256,7 @@ 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+ERROR: relation "test_partition" is not a hash index
-- an actual index of a partitioned 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);
diff --git a/contrib/pgstattuple/pgstatindex.c b/contrib/pgstattuple/pgstatindex.cindex 8e5a4d6a66..5c06ba6db4 100644--- a/contrib/pgstattuple/pgstatindex.c+++ b/contrib/pgstattuple/pgstatindex.c@@ -600,10 +600,9 @@ pgstathashindex(PG_FUNCTION_ARGS)
float8 free_percent;
uint64 total_space;
- rel = index_open(relid, AccessShareLock);+ rel = relation_open(relid, AccessShareLock);- /* index_open() checks that it's an index */- if (!IS_HASH(rel))+ if (!IS_INDEX(rel) || !IS_HASH(rel))
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("relation \"%s\" is not a hash index",
diff --git a/contrib/pgstattuple/sql/pgstattuple.sql b/contrib/pgstattuple/sql/pgstattuple.sqlindex 5111be0e62..b08c31c21b 100644--- a/contrib/pgstattuple/sql/pgstattuple.sql+++ b/contrib/pgstattuple/sql/pgstattuple.sql@@ -65,6 +65,7 @@ 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);
+create index test_partitioned_hash_index on test_partitioned using hash(a);
-- these should all fail
select pgstattuple('test_partitioned');
select pgstattuple('test_partitioned_index');
@@ -73,6 +74,7 @@ select pg_relpages('test_partitioned');
select pgstatindex('test_partitioned');
select pgstatginindex('test_partitioned');
select pgstathashindex('test_partitioned');
+select pgstathashindex('test_partitioned_hash_index');
create view test_view as select 1;
-- these should all fail
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: exclusion@gmail.com, michael@paquier.xyz, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table
In-Reply-To: <11c0916f-82de-decf-c27a-c01a4f8dfcef@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox