Re: Add IS_INDEX macro to brin and gist index

From: Japin Li <japinli(at)hotmail(dot)com>
To: Kirill Reshke <reshkekirill(at)gmail(dot)com>
Cc: Andreas Karlsson <andreas(at)proxel(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add IS_INDEX macro to brin and gist index
Date: 2026-01-14 14:55:20
Message-ID: SYAPR01MB3038F06B6896820E059DC7EBB68FA@SYAPR01MB3038.ausprd01.prod.outlook.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 14 Jan 2026 at 17:53, Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:
> Hi
>
> On Wed, 14 Jan 2026 at 17:40, Japin Li <japinli(at)hotmail(dot)com> wrote:
>>
>> On Wed, 14 Jan 2026 at 03:38, Andreas Karlsson <andreas(at)proxel(dot)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] https://www.postgresql.org/message-id/CALdSSPj-ADRgBk1_gspb2Q0eY2wxQHLfiWfFOmAwSxMF_AboRQ%40mail.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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kirill Reshke 2026-01-14 15:03:10 Re: GIN pageinspect support for entry tree and posting tree
Previous Message Japin Li 2026-01-14 14:51:06 Re: GIN pageinspect support for entry tree and posting tree