On Mon, 12 Jan 2026 at 04:23, Michael Paquier <[email protected]> wrote: > > On Sun, Jan 11, 2026 at 04:58:39PM +0500, Kirill Reshke wrote: > > Maybe, I have stopped some more cases, in v2-0001 > > Right. It's true that we could be more consistent for all these based > on their base type, some of them, particularly in the GIN code now, > caring about using the correct macro. It may be a good occasion to > double-check the whole tree for similar holes based on unsigned types. > -- > Michael
Ok > Hi Kirill, Roman, and Michael, > While double-checking the tree for similar holes as Michael suggested, I > noticed a couple more inconsistencies in contrib/pageinspect/ginfuncs.c where > we are using signed macros for unsigned types. > Specifically, in gin_page_opaque_info, we use Int32GetDatum for maxoff: > > values[1] = Int32GetDatum(opaq->maxoff); > > Since maxoff is of type OffsetNumber (uint16), this seems to be the exact > same pattern Kirill is addressing in other parts of the GIN code. Although it > is widened to int32 here, for the sake of consistency, it should probably be > using a 16-bit or unsigned macro. > > Similarly, in gin_metapage_info, tailFreeSize (which is defined as uint32 in > GinMetaPageData) is also converted using Int32GetDatum: > > values[2] = Int32GetDatum(metadata->tailFreeSize); > > It might be better to include these cleanups in the next version of the patch > to ensure all pageinspect GIN functions follow the same standard. Thank you, I have included your findings in v3. On Mon, 12 Jan 2026 at 11:53, zengman <[email protected]> wrote: > > Hi, > > I’ve also seen such cases in the kernel code, and I’m wondering if this > should be added to the patch here? > ``` > postgres@zxm-VMware-Virtual-Platform:~/code/postgres$ git diff > diff --git a/src/backend/utils/adt/lockfuncs.c > b/src/backend/utils/adt/lockfuncs.c > index bcbc226125c..9dadd6da672 100644 > --- a/src/backend/utils/adt/lockfuncs.c > +++ b/src/backend/utils/adt/lockfuncs.c > @@ -329,7 +329,7 @@ pg_lock_status(PG_FUNCTION_ARGS) > values[1] = > ObjectIdGetDatum(instance->locktag.locktag_field1); > values[8] = > ObjectIdGetDatum(instance->locktag.locktag_field2); > values[6] = > ObjectIdGetDatum(instance->locktag.locktag_field3); > - values[9] = > Int16GetDatum(instance->locktag.locktag_field4); > + values[9] = > UInt16GetDatum(instance->locktag.locktag_field4); > nulls[2] = true; > nulls[3] = true; > nulls[4] = true; > @@ -343,7 +343,7 @@ pg_lock_status(PG_FUNCTION_ARGS) > values[1] = > ObjectIdGetDatum(instance->locktag.locktag_field1); > values[7] = > ObjectIdGetDatum(instance->locktag.locktag_field2); > values[8] = > ObjectIdGetDatum(instance->locktag.locktag_field3); > - values[9] = > Int16GetDatum(instance->locktag.locktag_field4); > + values[9] = > UInt16GetDatum(instance->locktag.locktag_field4); > nulls[2] = true; > nulls[3] = true; > nulls[4] = true; Thank you, I have included your findings in v3. PFA v3 with fixes for signed usage across the tree, with my new findings and suggestions from thread -- Best regards, Kirill Reshke
v3-0001-Use-correct-macro-for-accessing-unsigned-numbers.patch
Description: Binary data
