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

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.

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]/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

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.

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

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.

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.

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/

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.

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/

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

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)