Add IS_INDEX macro to brin and gist index

Started by Japin Li10 days ago13 messages

japinli@hotmail.com

10 days ago

1 attachment(s)

Hi, hackers,

While working on pageinspect [0]/messages/by-id/CALdSSPiN13n7feQcY0WCmq8jzxjwqhNrt1E=g=g6aZANyE_OoQ@mail.gmail.com, I noticed that brin_page_items() and
gist_page_items() only checked the access method (IS_BRIN/IS_GIST) but did
not verify that the passed relation is actually an index relation.

To make the check more robust and consistent with other pageinspect index
functions (like btreefuncs.c, hashfuncs.c, etc.), the attached patch:

1. Defines a local helper macro IS_INDEX(r) in both brinfuncs.c and gistfuncs.c.

2. Updates the error check to require both: the relation must be an index and
use the expected access method.

The change is very small, low-risk, and only affects two functions in
contrib/pageinspect.

[0]: /messages/by-id/CALdSSPiN13n7feQcY0WCmq8jzxjwqhNrt1E=g=g6aZANyE_OoQ@mail.gmail.com

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Attachments:

v1-0001-Add-IS_INDEX-macro-to-brin-and-gist-index.patchtext/x-patchDownload

From a63f79687c392f0d339350bc85e0eeb213b29cdb Mon Sep 17 00:00:00 2001
From: Japin Li <japinli@hotmail.com>
Date: Wed, 14 Jan 2026 09:55:46 +0800
Subject: [PATCH v1] Add IS_INDEX macro to brin and gist index

---
 contrib/pageinspect/brinfuncs.c | 3 ++-
 contrib/pageinspect/gistfuncs.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index 26cf78252ed..4be2225e429 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -28,6 +28,7 @@ PG_FUNCTION_INFO_V1(brin_page_items);
 PG_FUNCTION_INFO_V1(brin_metapage_info);
 PG_FUNCTION_INFO_V1(brin_revmap_data);
 
+#define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
 #define IS_BRIN(r) ((r)->rd_rel->relam == BRIN_AM_OID)
 
 typedef struct brin_column_state
@@ -164,7 +165,7 @@ brin_page_items(PG_FUNCTION_ARGS)
 
 	indexRel = index_open(indexRelid, AccessShareLock);
 
-	if (!IS_BRIN(indexRel))
+	if (!IS_INDEX(indexRel) || !IS_BRIN(indexRel))
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("\"%s\" is not a %s index",
diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c
index e7759488c36..7062b7b423f 100644
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -30,6 +30,7 @@ PG_FUNCTION_INFO_V1(gist_page_opaque_info);
 PG_FUNCTION_INFO_V1(gist_page_items);
 PG_FUNCTION_INFO_V1(gist_page_items_bytea);
 
+#define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
 #define IS_GIST(r) ((r)->rd_rel->relam == GIST_AM_OID)
 
 
@@ -217,7 +218,7 @@ gist_page_items(PG_FUNCTION_ARGS)
 	/* Open the relation */
 	indexRel = index_open(indexRelid, AccessShareLock);
 
-	if (!IS_GIST(indexRel))
+	if (!IS_INDEX(indexRel) || !IS_GIST(indexRel))
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("\"%s\" is not a %s index",
-- 
2.43.0

Andreas Karlsson

andreas.karlsson@percona.com

10 days ago

In reply to: Japin Li (#1)

Re: Add IS_INDEX macro to brin and gist index

On 1/14/26 2:56 AM, Japin Li wrote:

Hi, hackers,

While working on pageinspect [0], I noticed that brin_page_items() and
gist_page_items() only checked the access method (IS_BRIN/IS_GIST) but did
not verify that the passed relation is actually an index relation.

To make the check more robust and consistent with other pageinspect index
functions (like btreefuncs.c, hashfuncs.c, etc.), the attached patch:

1. Defines a local helper macro IS_INDEX(r) in both brinfuncs.c and gistfuncs.c.

2. Updates the error check to require both: the relation must be an index and
use the expected access method.

The change is very small, low-risk, and only affects two functions in
contrib/pageinspect.

Since the two functions you touch are gist_page_items() and
brin_page_items() is there actually any harm from being able to use the
index definition from a partitioned when parsing the page? Seems
unnecessary to prevent people from doing so unless I am missing
something. It is not like we have any way to prevent the wrong index
from being used when parsing page the page so why prevent partitioned
indexes specifically?

Andreas

Japin Li

japinli@hotmail.com

10 days ago

In reply to: Andreas Karlsson (#2)

Re: Add IS_INDEX macro to brin and gist index

On Wed, 14 Jan 2026 at 03:38, Andreas Karlsson <andreas@proxel.se> wrote:

On 1/14/26 2:56 AM, Japin Li wrote:

Hi, hackers,
While working on pageinspect [0], I noticed that brin_page_items()
and
gist_page_items() only checked the access method (IS_BRIN/IS_GIST) but did
not verify that the passed relation is actually an index relation.
To make the check more robust and consistent with other pageinspect
index
functions (like btreefuncs.c, hashfuncs.c, etc.), the attached patch:
1. Defines a local helper macro IS_INDEX(r) in both brinfuncs.c and
gistfuncs.c.
2. Updates the error check to require both: the relation must be an
index and
use the expected access method.
The change is very small, low-risk, and only affects two functions
in
contrib/pageinspect.

Since the two functions you touch are gist_page_items() and
brin_page_items() is there actually any harm from being able to use
the index definition from a partitioned when parsing the page? Seems
unnecessary to prevent people from doing so unless I am missing
something. It is not like we have any way to prevent the wrong index
from being used when parsing page the page so why prevent partitioned
indexes specifically?

Thanks for the thoughtful question!

The reason I added the IS_INDEX check (and reject partitioned indexes) is that
a partitioned index in PostgreSQL doesn't actually store any data pages itself
it only exists as a logical parent for the partition-level indexes. So passing
a partitioned index OID to brin_page_items() or gist_page_items() would
fundamentally not make sense: there are no real pages to inspect, and trying
to read a page from it would fail.

postgres=# SELECT * FROM brin_page_items(get_raw_page('brin_test_create_at_idx', 0), 'brin_test_create_at_idx');
ERROR: cannot get raw page from relation "brin_test_create_at_idx"
DETAIL: This operation is not supported for partitioned indexes.

OTOH, this is also fully consistent with how other pageinspect functions (like
btree_page_items, hash_page_items, etc.) behave — they only accept concrete,
non-partitioned index relations that actually have physical pages.

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Kirill Reshke

reshkekirill@gmail.com

10 days ago

In reply to: Japin Li (#3)

Re: Add IS_INDEX macro to brin and gist index

On Wed, 14 Jan 2026 at 17:40, Japin Li <japinli@hotmail.com> wrote:

On Wed, 14 Jan 2026 at 03:38, Andreas Karlsson <andreas@proxel.se> wrote:

On 1/14/26 2:56 AM, Japin Li wrote:

Hi, hackers,
While working on pageinspect [0], I noticed that brin_page_items()
and
gist_page_items() only checked the access method (IS_BRIN/IS_GIST) but did
not verify that the passed relation is actually an index relation.
To make the check more robust and consistent with other pageinspect
index
functions (like btreefuncs.c, hashfuncs.c, etc.), the attached patch:
1. Defines a local helper macro IS_INDEX(r) in both brinfuncs.c and
gistfuncs.c.
2. Updates the error check to require both: the relation must be an
index and
use the expected access method.
The change is very small, low-risk, and only affects two functions
in
contrib/pageinspect.

Since the two functions you touch are gist_page_items() and
brin_page_items() is there actually any harm from being able to use
the index definition from a partitioned when parsing the page? Seems
unnecessary to prevent people from doing so unless I am missing
something. It is not like we have any way to prevent the wrong index
from being used when parsing page the page so why prevent partitioned
indexes specifically?

Thanks for the thoughtful question!

The reason I added the IS_INDEX check (and reject partitioned indexes) is that
a partitioned index in PostgreSQL doesn't actually store any data pages itself
it only exists as a logical parent for the partition-level indexes. So passing
a partitioned index OID to brin_page_items() or gist_page_items() would
fundamentally not make sense: there are no real pages to inspect, and trying
to read a page from it would fail.

Correct, we do not need to run page inspect functions against
partitioned indexes.

Also, maybe we define this as pageinspect.h, like in [0]/messages/by-id/CALdSSPj-ADRgBk1_gspb2Q0eY2wxQHLfiWfFOmAwSxMF_AboRQ@mail.gmail.com ?

[0]: /messages/by-id/CALdSSPj-ADRgBk1_gspb2Q0eY2wxQHLfiWfFOmAwSxMF_AboRQ@mail.gmail.com

--
Best regards,
Kirill Reshke

Japin Li

japinli@hotmail.com

10 days ago

In reply to: Kirill Reshke (#4)

Re: Add IS_INDEX macro to brin and gist index

On Wed, 14 Jan 2026 at 17:53, Kirill Reshke <reshkekirill@gmail.com> wrote:

Hi

On Wed, 14 Jan 2026 at 17:40, Japin Li <japinli@hotmail.com> wrote:

On Wed, 14 Jan 2026 at 03:38, Andreas Karlsson <andreas@proxel.se> wrote:

On 1/14/26 2:56 AM, Japin Li wrote:

Hi, hackers,
While working on pageinspect [0], I noticed that brin_page_items()
and
gist_page_items() only checked the access method (IS_BRIN/IS_GIST) but did
not verify that the passed relation is actually an index relation.
To make the check more robust and consistent with other pageinspect
index
functions (like btreefuncs.c, hashfuncs.c, etc.), the attached patch:
1. Defines a local helper macro IS_INDEX(r) in both brinfuncs.c and
gistfuncs.c.
2. Updates the error check to require both: the relation must be an
index and
use the expected access method.
The change is very small, low-risk, and only affects two functions
in
contrib/pageinspect.

Since the two functions you touch are gist_page_items() and
brin_page_items() is there actually any harm from being able to use
the index definition from a partitioned when parsing the page? Seems
unnecessary to prevent people from doing so unless I am missing
something. It is not like we have any way to prevent the wrong index
from being used when parsing page the page so why prevent partitioned
indexes specifically?

Thanks for the thoughtful question!

The reason I added the IS_INDEX check (and reject partitioned indexes) is that
a partitioned index in PostgreSQL doesn't actually store any data pages itself
it only exists as a logical parent for the partition-level indexes. So passing
a partitioned index OID to brin_page_items() or gist_page_items() would
fundamentally not make sense: there are no real pages to inspect, and trying
to read a page from it would fail.

Correct, we do not need to run page inspect functions against
partitioned indexes.

Also, maybe we define this as pageinspect.h, like in [0] ?

[0] /messages/by-id/CALdSSPj-ADRgBk1_gspb2Q0eY2wxQHLfiWfFOmAwSxMF_AboRQ@mail.gmail.com

Yeah, I actually prepared a patch for this in [0].

Or we could address it in this thread if you prefer. What do you think?

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Kirill Reshke

reshkekirill@gmail.com

10 days ago

In reply to: Japin Li (#5)

Re: Add IS_INDEX macro to brin and gist index

On Wed, 14 Jan 2026 at 19:55, Japin Li <japinli@hotmail.com> wrote:

Or we could address it in this thread if you prefer. What do you think?

As you wish - we can continue here, because this is a separate change
which has its independent value.

--
Best regards,
Kirill Reshke

Japin Li

japinli@hotmail.com

10 days ago

In reply to: Kirill Reshke (#6)

1 attachment(s)

Re: Add IS_INDEX macro to brin and gist index

On Wed, 14 Jan 2026 at 20:07, Kirill Reshke <reshkekirill@gmail.com> wrote:

On Wed, 14 Jan 2026 at 19:55, Japin Li <japinli@hotmail.com> wrote:

Or we could address it in this thread if you prefer. What do you think?

As you wish - we can continue here, because this is a separate change
which has its independent value.

Attach the v2 patch.

1. Move the IS_INDEX macro to pageinspect.h
2. Check relation kind for both brin and gist index

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Attachments:

v2-0001-Add-IS_INDEX-macro-to-brin-and-gist-index.patchtext/x-patchDownload

From e0f7ce413bd4d725da893efdc1f4bdbbf732ad27 Mon Sep 17 00:00:00 2001
From: Japin Li <japinli@hotmail.com>
Date: Wed, 14 Jan 2026 09:55:46 +0800
Subject: [PATCH v2] Add IS_INDEX macro to brin and gist index

This commit also moves IS_INDEX macro to pageinspect.h

Suggested-by: Japin Li <japinli@hotmail.com>
Suggested-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>

Discussion: https://postgr.es/m/MEAPR01MB3031A889D4B3F610E9D2A3AFB68FA@MEAPR01MB3031.ausprd01.prod.outlook.com
Discussion: https://postgr.es/m/BFCA6110-EC97-4569-917A-747A72F62CE8@yandex-team.ru
---
 contrib/pageinspect/brinfuncs.c | 3 ++-
 contrib/pageinspect/gistfuncs.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index 26cf78252ed..4be2225e429 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -28,6 +28,7 @@ PG_FUNCTION_INFO_V1(brin_page_items);
 PG_FUNCTION_INFO_V1(brin_metapage_info);
 PG_FUNCTION_INFO_V1(brin_revmap_data);
 
+#define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
 #define IS_BRIN(r) ((r)->rd_rel->relam == BRIN_AM_OID)
 
 typedef struct brin_column_state
@@ -164,7 +165,7 @@ brin_page_items(PG_FUNCTION_ARGS)
 
 	indexRel = index_open(indexRelid, AccessShareLock);
 
-	if (!IS_BRIN(indexRel))
+	if (!IS_INDEX(indexRel) || !IS_BRIN(indexRel))
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("\"%s\" is not a %s index",
diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c
index a205cb8a334..49d6d4940fc 100644
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -30,6 +30,7 @@ PG_FUNCTION_INFO_V1(gist_page_opaque_info);
 PG_FUNCTION_INFO_V1(gist_page_items);
 PG_FUNCTION_INFO_V1(gist_page_items_bytea);
 
+#define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
 #define IS_GIST(r) ((r)->rd_rel->relam == GIST_AM_OID)
 
 
@@ -217,7 +218,7 @@ gist_page_items(PG_FUNCTION_ARGS)
 	/* Open the relation */
 	indexRel = index_open(indexRelid, AccessShareLock);
 
-	if (!IS_GIST(indexRel))
+	if (!IS_INDEX(indexRel) || !IS_GIST(indexRel))
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("\"%s\" is not a %s index",
-- 
2.43.0

Japin Li

japinli@hotmail.com

10 days ago

In reply to: Japin Li (#7)

1 attachment(s)

Re: Add IS_INDEX macro to brin and gist index

On Wed, 14 Jan 2026 at 23:31, Japin Li <japinli@hotmail.com> wrote:

On Wed, 14 Jan 2026 at 20:07, Kirill Reshke <reshkekirill@gmail.com> wrote:

On Wed, 14 Jan 2026 at 19:55, Japin Li <japinli@hotmail.com> wrote:

Or we could address it in this thread if you prefer. What do you think?

As you wish - we can continue here, because this is a separate change
which has its independent value.

Attach the v2 patch.

1. Move the IS_INDEX macro to pageinspect.h
2. Check relation kind for both brin and gist index

Sorry about that — I uploaded the wrong patch by mistake.
Here's v3 attached. Thanks for your patience!

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Attachments:

v3-0001-Add-IS_INDEX-macro-to-brin-and-gist-index.patchtext/x-patchDownload

From 94bc42b008a5417fd70e7f6823f1baeb43f00aa1 Mon Sep 17 00:00:00 2001
From: Japin Li <japinli@hotmail.com>
Date: Wed, 14 Jan 2026 09:55:46 +0800
Subject: [PATCH v3] Add IS_INDEX macro to brin and gist index

This commit also moves IS_INDEX macro to pageinspect.h

Suggested-by: Japin Li <japinli@hotmail.com>
Suggested-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>

Discussion: https://postgr.es/m/MEAPR01MB3031A889D4B3F610E9D2A3AFB68FA@MEAPR01MB3031.ausprd01.prod.outlook.com
Discussion: https://postgr.es/m/BFCA6110-EC97-4569-917A-747A72F62CE8@yandex-team.ru
---
 contrib/pageinspect/brinfuncs.c   | 2 +-
 contrib/pageinspect/btreefuncs.c  | 7 +++----
 contrib/pageinspect/gistfuncs.c   | 2 +-
 contrib/pageinspect/hashfuncs.c   | 5 ++---
 contrib/pageinspect/pageinspect.h | 2 ++
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index 26cf78252ed..a1cbd9fa040 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -28,7 +28,7 @@ PG_FUNCTION_INFO_V1(brin_page_items);
 PG_FUNCTION_INFO_V1(brin_metapage_info);
 PG_FUNCTION_INFO_V1(brin_revmap_data);
 
-#define IS_BRIN(r) ((r)->rd_rel->relam == BRIN_AM_OID)
+#define IS_BRIN(r) (IS_INDEX(r) && (r)->rd_rel->relam == BRIN_AM_OID)
 
 typedef struct brin_column_state
 {
diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index 0585b7cee40..a49a9beee68 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -49,8 +49,7 @@ PG_FUNCTION_INFO_V1(bt_page_stats_1_9);
 PG_FUNCTION_INFO_V1(bt_page_stats);
 PG_FUNCTION_INFO_V1(bt_multi_page_stats);
 
-#define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
-#define IS_BTREE(r) ((r)->rd_rel->relam == BTREE_AM_OID)
+#define IS_BTREE(r) (IS_INDEX(r) && (r)->rd_rel->relam == BTREE_AM_OID)
 
 /* ------------------------------------------------
  * structure for single btree page statistics
@@ -225,7 +224,7 @@ check_relation_block_range(Relation rel, int64 blkno)
 static void
 bt_index_block_validate(Relation rel, int64 blkno)
 {
-	if (!IS_INDEX(rel) || !IS_BTREE(rel))
+	if (!IS_BTREE(rel))
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("\"%s\" is not a %s index",
@@ -858,7 +857,7 @@ bt_metap(PG_FUNCTION_ARGS)
 	relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
 	rel = relation_openrv(relrv, AccessShareLock);
 
-	if (!IS_INDEX(rel) || !IS_BTREE(rel))
+	if (!IS_BTREE(rel))
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("\"%s\" is not a %s index",
diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c
index a205cb8a334..88776b04d5d 100644
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -30,7 +30,7 @@ PG_FUNCTION_INFO_V1(gist_page_opaque_info);
 PG_FUNCTION_INFO_V1(gist_page_items);
 PG_FUNCTION_INFO_V1(gist_page_items_bytea);
 
-#define IS_GIST(r) ((r)->rd_rel->relam == GIST_AM_OID)
+#define IS_GIST(r) (IS_INDEX(r) && (r)->rd_rel->relam == GIST_AM_OID)
 
 
 static Page verify_gist_page(bytea *raw_page);
diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c
index 7fc97d043ce..86e73b36b41 100644
--- a/contrib/pageinspect/hashfuncs.c
+++ b/contrib/pageinspect/hashfuncs.c
@@ -28,8 +28,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)
+#define IS_HASH(r) (IS_INDEX(r) && (r)->rd_rel->relam == HASH_AM_OID)
 
 /* ------------------------------------------------
  * structure for single hash page statistics
@@ -421,7 +420,7 @@ hash_bitmap_info(PG_FUNCTION_ARGS)
 	 */
 	indexRel = relation_open(indexRelid, AccessShareLock);
 
-	if (!IS_INDEX(indexRel) || !IS_HASH(indexRel))
+	if (!IS_HASH(indexRel))
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("\"%s\" is not a %s index",
diff --git a/contrib/pageinspect/pageinspect.h b/contrib/pageinspect/pageinspect.h
index b241fdc97b2..7d472dc784f 100644
--- a/contrib/pageinspect/pageinspect.h
+++ b/contrib/pageinspect/pageinspect.h
@@ -15,6 +15,8 @@
 
 #include "storage/bufpage.h"
 
+#define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
+
 /*
  * Extension version number, for supporting older extension versions' objects
  */
-- 
2.43.0

Alvaro Herrera

alvherre@2ndquadrant.com

10 days ago

In reply to: Japin Li (#8)

Re: Add IS_INDEX macro to brin and gist index

On 2026-Jan-14, Japin Li wrote:

-#define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
-#define IS_BTREE(r) ((r)->rd_rel->relam == BTREE_AM_OID)
+#define IS_BTREE(r) (IS_INDEX(r) && (r)->rd_rel->relam == BTREE_AM_OID)

I find this coding rather pointless. You can more easily do something
like

#define IS_BTREE(r) ((r)->rd_rel->relkind == RELKIND_INDEX && (r)->rd_rel->relam == BTREE_AM_OID)

and get rid of the IS_INDEX macro completely, if it's not used anywhere
else. Same for all the other index AMs.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/

#10

Japin Li

japinli@hotmail.com

10 days ago

In reply to: Alvaro Herrera (#9)

1 attachment(s)

Re: Add IS_INDEX macro to brin and gist index

On Wed, 14 Jan 2026 at 16:40, Álvaro Herrera <alvherre@kurilemu.de> wrote:

On 2026-Jan-14, Japin Li wrote:
-#define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
-#define IS_BTREE(r) ((r)->rd_rel->relam == BTREE_AM_OID)
+#define IS_BTREE(r) (IS_INDEX(r) && (r)->rd_rel->relam == BTREE_AM_OID)
I find this coding rather pointless. You can more easily do something
like

#define IS_BTREE(r) ((r)->rd_rel->relkind == RELKIND_INDEX && (r)->rd_rel->relam == BTREE_AM_OID)

and get rid of the IS_INDEX macro completely, if it's not used anywhere
else. Same for all the other index AMs.

Thanks for the review.

I've fixed it according to your suggestion.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

Attachments:

v3-0001-Check-relkind-for-both-BRIN-and-GIST-indexes.patchtext/x-patch; charset=utf-8Download

From 95bc5110a0dd80511397131cf83399bbf7e24a45 Mon Sep 17 00:00:00 2001
From: Japin Li <japinli@hotmail.com>
Date: Wed, 14 Jan 2026 09:55:46 +0800
Subject: [PATCH v3] Check relkind for both BRIN and GIST indexes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, only the access method was checked for BRIN and GIST indexes,
allowing non-index relations to be passed.

This commit also removes the pointless IS_INDEX macro.

Suggested-by: Japin Li <japinli@hotmail.com>
Suggested-by: Andrey Borodin <x4mmm@yandex-team.ru>
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>

Discussion: https://postgr.es/m/MEAPR01MB3031A889D4B3F610E9D2A3AFB68FA@MEAPR01MB3031.ausprd01.prod.outlook.com
Discussion: https://postgr.es/m/BFCA6110-EC97-4569-917A-747A72F62CE8@yandex-team.ru
---
 contrib/pageinspect/brinfuncs.c  | 2 +-
 contrib/pageinspect/btreefuncs.c | 7 +++----
 contrib/pageinspect/gistfuncs.c  | 2 +-
 contrib/pageinspect/hashfuncs.c  | 5 ++---
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index 26cf78252ed..6761a109745 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -28,7 +28,7 @@ PG_FUNCTION_INFO_V1(brin_page_items);
 PG_FUNCTION_INFO_V1(brin_metapage_info);
 PG_FUNCTION_INFO_V1(brin_revmap_data);
 
-#define IS_BRIN(r) ((r)->rd_rel->relam == BRIN_AM_OID)
+#define IS_BRIN(r) ((r)->rd_rel->relkind == RELKIND_INDEX && (r)->rd_rel->relam == BRIN_AM_OID)
 
 typedef struct brin_column_state
 {
diff --git a/contrib/pageinspect/btreefuncs.c b/contrib/pageinspect/btreefuncs.c
index 0585b7cee40..9a3f08daceb 100644
--- a/contrib/pageinspect/btreefuncs.c
+++ b/contrib/pageinspect/btreefuncs.c
@@ -49,8 +49,7 @@ PG_FUNCTION_INFO_V1(bt_page_stats_1_9);
 PG_FUNCTION_INFO_V1(bt_page_stats);
 PG_FUNCTION_INFO_V1(bt_multi_page_stats);
 
-#define IS_INDEX(r) ((r)->rd_rel->relkind == RELKIND_INDEX)
-#define IS_BTREE(r) ((r)->rd_rel->relam == BTREE_AM_OID)
+#define IS_BTREE(r) ((r)->rd_rel->relkind == RELKIND_INDEX && (r)->rd_rel->relam == BTREE_AM_OID)
 
 /* ------------------------------------------------
  * structure for single btree page statistics
@@ -225,7 +224,7 @@ check_relation_block_range(Relation rel, int64 blkno)
 static void
 bt_index_block_validate(Relation rel, int64 blkno)
 {
-	if (!IS_INDEX(rel) || !IS_BTREE(rel))
+	if (!IS_BTREE(rel))
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("\"%s\" is not a %s index",
@@ -858,7 +857,7 @@ bt_metap(PG_FUNCTION_ARGS)
 	relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
 	rel = relation_openrv(relrv, AccessShareLock);
 
-	if (!IS_INDEX(rel) || !IS_BTREE(rel))
+	if (!IS_BTREE(rel))
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("\"%s\" is not a %s index",
diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c
index a205cb8a334..e472a46596f 100644
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -30,7 +30,7 @@ PG_FUNCTION_INFO_V1(gist_page_opaque_info);
 PG_FUNCTION_INFO_V1(gist_page_items);
 PG_FUNCTION_INFO_V1(gist_page_items_bytea);
 
-#define IS_GIST(r) ((r)->rd_rel->relam == GIST_AM_OID)
+#define IS_GIST(r) ((r)->rd_rel->relkind == RELKIND_INDEX && (r)->rd_rel->relam == GIST_AM_OID)
 
 
 static Page verify_gist_page(bytea *raw_page);
diff --git a/contrib/pageinspect/hashfuncs.c b/contrib/pageinspect/hashfuncs.c
index 7fc97d043ce..d3242a386f1 100644
--- a/contrib/pageinspect/hashfuncs.c
+++ b/contrib/pageinspect/hashfuncs.c
@@ -28,8 +28,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)
+#define IS_HASH(r) ((r)->rd_rel->relkind == RELKIND_INDEX && (r)->rd_rel->relam == HASH_AM_OID)
 
 /* ------------------------------------------------
  * structure for single hash page statistics
@@ -421,7 +420,7 @@ hash_bitmap_info(PG_FUNCTION_ARGS)
 	 */
 	indexRel = relation_open(indexRelid, AccessShareLock);
 
-	if (!IS_INDEX(indexRel) || !IS_HASH(indexRel))
+	if (!IS_HASH(indexRel))
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("\"%s\" is not a %s index",
-- 
2.43.0

#11

Alvaro Herrera

alvherre@2ndquadrant.com

10 days ago

In reply to: Japin Li (#10)

Re: Add IS_INDEX macro to brin and gist index

On 2026-Jan-15, Japin Li wrote:

Thanks for the review.

I've fixed it according to your suggestion.

Thanks! Unless somebody hates this, I'll get it pushed quickly.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/

#12

Michael Paquier

michael@paquier.xyz

9 days ago

In reply to: Alvaro Herrera (#11)

Re: Add IS_INDEX macro to brin and gist index

On Wed, Jan 14, 2026 at 06:54:17PM +0100, Alvaro Herrera wrote:

Thanks! Unless somebody hates this, I'll get it pushed quickly.

That looks fine by me, at quick glance, removing some code churn.
--
Michael

#13

Alvaro Herrera

alvherre@2ndquadrant.com

9 days ago

In reply to: Japin Li (#10)

Re: Add IS_INDEX macro to brin and gist index

After looking at this more closely, I wonder if this is really doing
what we want. For BRIN and GiST, you can only pass an index to the
brin_page_items and gist_page_items functions respectively, and that
only as to let the function know what relation the page comes from. The
actual read from the index comes from get_raw_page().

So in the regression database, I created
create index on regress_constr_partitioned using brin (a);

and then tried this

select * from brin_page_items(get_raw_page('regress_constr_partitioned_a_idx1', 1), 'regress_constr_partition1_a_idx'::regclass);

this gives me the following existing error:
ERROR: cannot get raw page from relation "regress_constr_partitioned_a_idx1"
DETALLE: This operation is not supported for partitioned indexes.

but if I instead do it the other way around,

select * from brin_page_items(get_raw_page('regress_constr_partition1_a_idx', 1), 'regress_constr_partitioned_a_idx1'::regclass);

the error is now
ERROR: "regress_constr_partitioned_a_idx1" is not a BRIN index

I wonder ... shouldn't these reports be more similar? Or, there's also
the alternative view that we don't _need_ to throw an error here. If I
remove the new check, I get this

select * from brin_page_items(get_raw_page('regress_constr_partition1_a_idx', 2), 'regress_constr_partitioned_a_idx1'::regclass);
itemoffset │ blknum │ attnum │ allnulls │ hasnulls │ placeholder │ empty │ value
────────────┼────────┼────────┼──────────┼──────────┼─────────────┼───────┼───────
1 │ 0 │ 1 │ t │ f │ f │ t │
(1 fila)

which seems ... perfectly okay? I mean, why are you worried about this?

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Computing is too important to be left to men." (Karen Spärck Jones)