Re: Fix memory leak in gist_page_items() of pageinspect

On Fri, 19 Dec 2025 at 04:29, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> Hi,
>
> On Fri, Dec 19, 2025 at 08:01:54AM +0900, Michael Paquier wrote:
>> On Wed, Dec 17, 2025 at 11:57:13AM +0000, Bertrand Drouvot wrote:
>> > Please note that for hash_bitmap_info() and pgstathashindex() the open calls are
>> > changed instead. For those we keep the IS_INDEX() checks to reject partitioned
>> > indexes (which index_open() accepts via validate_relation_kind()). So, that also
>> > changes the error messages in some tests. If we do prefer the previous error
>> > messages we could change the close calls instead (I prefer the way it's done
>> > in the attached though).
>>
>> I have noticed that the two surrounding relation_close() calls for the
>> parent tables did not get the notice of the change for brin.c of what
>> you are doing for the indexes, while we use table_open(). I have
>> fixed these.
>
> Nice catch, thanks!
>
>> It would be nicer if IS_INDEX() could be removed in the other code
>> paths you are suggesting to change, but the partitioned index argument
>> also means that we would have two code paths in charge of a relkind
>> check instead of one. Just using relation_*() may be cleaner.
>
> Yeah, and removing IS_INDEX() and adding a check for partitioned indexes would
> still mean 2 code paths. So, v2 changes the close calls (and that's consistent
> with what pgstatginindex_internal() is doing.
>

It would be reasonable to add a comment explaining the choice of
relation_open()/relation_close() instead of the index-specific
index_open()/index_close().

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