On Wed, 14 Jan 2026 at 17:53, Kirill Reshke <[email protected]> wrote: > Hi > > On Wed, 14 Jan 2026 at 17:40, Japin Li <[email protected]> wrote: >> >> On Wed, 14 Jan 2026 at 03:38, Andreas Karlsson <[email protected]> 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.
