On Fri, 19 Dec 2025 at 04:29, Bertrand Drouvot <[email protected]> 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.
