On Sun, 28 Jul 2024 at 16:30, Joseph Koshakow <koshy44@gmail.com> wrote:
> Attached is an updated patch with your approach. I removed the 0 from
> the negative case because I think it was unnecessary, but happy to add
> it back in if I missed something.
I made a few adjustments and pushed this. I did keep the 0 - part as
some compilers don't seem to like not having the 0. e.g MSVC gives:
../src/backend/utils/adt/dbsize.c(578): warning C4146: unary minus
operator applied to unsigned type, result still unsigned
I thought a bit about if it was really worth keeping the regression
test or not and in the end decided it was likely worthwhile keeping
it, so I expanded it slightly to cover both PG_INT64_MIN and
PG_INT64_MAX values. It looks slightly less like we're earmarking the
fact that there was a bug that way, and also seems to be of some
additional value.
PG15 did see quite a significant rewrite of the pg_size_pretty code.
The bug does still exist in PG14 and earlier, but on looking at what
it would take to fix it there I got a bit unexcited at the risk to
reward ratio of adjusting that code and just left it alone. I've
backpatched only as far as PG15. I'm sure someone else will feel I
should have done something else there, but that's the judgement call I
made.
Thanks for the patch.
David
