Hi Peter,
Thanks for your feedback.
> I'm tempted to think the correct direction here would be to use uint8
> and its associated macros and functions more rigorously, not less.
OK, if my understanding is correct this leaves us char_increment() and
char_decrement() which currently use DatumGetUInt8 inconsistently with
CharGetDatum for the argument and return value correspondingly.
> I don't think it's worth making an isolated fix here for the one use of
> UInt8GetDatum()
I think you meant not this particular change so I included it to the
patch. We can keep nbtcompare.c as if you believe this change is not
that important (it arguably isn't).
> The change in pageinspect looks correct, but then when you look
> nearby and further around, you will find that there are also a bunch of
> (if not most) UInt16GetDatum and UInt32GetDatum that are wrong
> [...]
Agree. I did my best to fix all the inconsistencies in pageinsect.
> There are a lot of opportunities to clean this up, and I suspect that
> while many of these will work either way in practice because the actual
> values don't go far enough to hit the signed/unsigned boundary, I think
> there could a number of little bugs here to be fixed.
I believe you were referring to places other than pageinspect. I will
investigate and create separate threads with corresponding patches
later.
--
Best regards,
Aleksander Alekseev
