On Mon, 12 Jan 2026 at 04:23, Michael Paquier <michael@paquier.xyz> 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.cwhere 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
partsof the GIN code. Although it is widened to int32 here, for the sake of consistency, it should probably be using a
16-bitor 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
followthe same standard.
Thank you, I have included your findings in v3.
On Mon, 12 Jan 2026 at 11:53, zengman <zengman@halodbtech.com> 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
