pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table
5+ messages / 3 participants
[nested] [flat]

* BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table
@ 2023-12-13 09:00 PG Bug reporting form <noreply@postgresql.org>
  2023-12-13 14:18 ` Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 5+ messages in thread

From: PG Bug reporting form @ 2023-12-13 09:00 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: exclusion@gmail.com

The following bug has been logged on the website:

Bug reference:      18246
Logged by:          Alexander Lakhin
Email address:      exclusion@gmail.com
PostgreSQL version: 16.1
Operating system:   Ubuntu 22.04
Description:        

The following script:
CREATE EXTENSION pgstattuple;
CREATE TABLE t (a int, b int[], c int) PARTITION BY RANGE (a);

CREATE INDEX a_idx ON t USING btree(a);
CREATE INDEX b_idx ON t USING gin(b);
CREATE INDEX c_idx ON t USING hash(c);

SELECT pgstatindex('a_idx');
SELECT pgstatginindex('b_idx');
SELECT pgstathashindex('c_idx');

gives an unexpected error for the hash index:
ERROR:  could not open file "pg_tblspc/0/PG_16_202307071/0/0": No such file
or directory

while for btree and hash indexes errors are more adequate:
ERROR:  relation "a_idx" is not a btree index
ERROR:  relation "b_idx" is not a GIN index

Reproduced on REL_12_STABLE .. master.
(On master, thanks to commit 049ef3398, which added an Assert in smgr.c,
that assertion fails.)

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))



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table
  2023-12-13 09:00 BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table PG Bug reporting form <noreply@postgresql.org>
@ 2023-12-13 14:18 ` Michael Paquier <michael@paquier.xyz>
  2023-12-13 18:00   ` Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table Alexander Lakhin <exclusion@gmail.com>
  0 siblings, 1 reply; 5+ messages in thread

From: Michael Paquier @ 2023-12-13 14:18 UTC (permalink / raw)
  To: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org

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?
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../ZXm9Js8D_W_cFWzW@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table
  2023-12-13 09:00 BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table PG Bug reporting form <noreply@postgresql.org>
  2023-12-13 14:18 ` Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table Michael Paquier <michael@paquier.xyz>
@ 2023-12-13 18:00   ` Alexander Lakhin <exclusion@gmail.com>
  2023-12-15 13:00     ` Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table Alexander Lakhin <exclusion@gmail.com>
  0 siblings, 1 reply; 5+ messages in thread

From: Alexander Lakhin @ 2023-12-13 18:00 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; pgsql-bugs@lists.postgresql.org

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,
I've decided to change also index_open() -> relation_open() because
index_open() only adds a check, that is not suitable for this concrete
case, so there is no much sense to use the different function.
(In fact, that check was good enough at the time of e759854a0, but
8b08f7d48 changed it.)
As a result, error messages for hash indexes are becoming consistent with
other indexes. Also I've added a test case for the issue, but maybe it's
superfluous...

Best regards,
Alexander

Attachments:

  [text/x-patch] v1-fix-pgstathashindex.patch (4.2K, ../../876b6e52-f068-648a-3f0e-7eaabd57cf77@gmail.com/2-v1-fix-pgstathashindex.patch)
  download | inline diff:
diff --git a/contrib/pgstattuple/expected/pgstattuple.out b/contrib/pgstattuple/expected/pgstattuple.out
index 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.c
index 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.sql
index 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


^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table
  2023-12-13 09:00 BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table PG Bug reporting form <noreply@postgresql.org>
  2023-12-13 14:18 ` Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table Michael Paquier <michael@paquier.xyz>
  2023-12-13 18:00   ` Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table Alexander Lakhin <exclusion@gmail.com>
@ 2023-12-15 13:00     ` Alexander Lakhin <exclusion@gmail.com>
  2023-12-19 09:20       ` Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table Michael Paquier <michael@paquier.xyz>
  0 siblings, 1 reply; 5+ messages in thread

From: Alexander Lakhin @ 2023-12-15 13:00 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; pgsql-bugs@lists.postgresql.org

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.c
index 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.out
index 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.c
index 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.sql
index 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


^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table
  2023-12-13 09:00 BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table PG Bug reporting form <noreply@postgresql.org>
  2023-12-13 14:18 ` Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table Michael Paquier <michael@paquier.xyz>
  2023-12-13 18:00   ` Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table Alexander Lakhin <exclusion@gmail.com>
  2023-12-15 13:00     ` Re: BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table Alexander Lakhin <exclusion@gmail.com>
@ 2023-12-19 09:20       ` Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 5+ messages in thread

From: Michael Paquier @ 2023-12-19 09:20 UTC (permalink / raw)
  To: Alexander Lakhin <exclusion@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org

On Fri, Dec 15, 2023 at 04:00:01PM +0300, Alexander Lakhin wrote:
> 13.12.2023 21:00, Alexander Lakhin wrote:
>> 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,

Thanks.  The one for pgstattuple has been applied down to 12 as of
a8dd62ef4959.

> 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

Right.

> 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.

Not sure to agree on that.  It seems a bit strange to bump on a
different error when using a partitioned index or a relkind not
allowed by index_open().  And this is after looking at the pgstattuple
code.

At the end, I've switched to relation_open() for this one, added a
test and applied the second patch as of 208470136421.  I've checked
the rest and yes, it looks like there are no other holes like the
ones you have reported for the contrib modules when it comes to index
manipulations.
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../ZYFgbOGYpsaen5LF@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 5+ messages in thread


end of thread, other threads:[~2023-12-19 09:20 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-12-13 09:00 BUG #18246: pgstathashindex() attempts to read invalid file for hash index attached to partitioned table PG Bug reporting form <noreply@postgresql.org>
2023-12-13 14:18 ` Michael Paquier <michael@paquier.xyz>
2023-12-13 18:00   ` Alexander Lakhin <exclusion@gmail.com>
2023-12-15 13:00     ` Alexander Lakhin <exclusion@gmail.com>
2023-12-19 09:20       ` Michael Paquier <michael@paquier.xyz>

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