David Rowley writes:
> On Mon, 5 Jul 2021 at 20:00, David Rowley <dgrowleyml@gmail.com> wrote:
>> I don't really like the fact that I had to add the doHalfRound field
>> to get the same rounding behaviour as the original functions. I'm
>> wondering if it would just be too clever just to track how many bits
>> we've shifted right by in pg_size_pretty* and compare that to the
>> value of multishift for the current unit and do appropriate rounding
>> to get us to the value of multishift. In theory, we could just keep
>> calling the half_rounded macro until we make it to the multishift
>> value. My current thoughts are that it's unlikely that anyone would
>> twiddle with the size_pretty_units array in such a way for the extra
>> code to be worth it. Maybe someone else feels differently.
>
> I made another pass over this and ended up removing the doHalfRound
> field in favour of just doing rounding based on the previous
> bitshifts.
>
> I did a few other tidy ups and I think it's a useful patch as it
> reduces the amount of code a bit and makes it dead simple to add new
> units in the future. Most importantly it'll help keep pg_size_pretty,
> pg_size_pretty_numeric and pg_size_bytes all in sync in regards to
> what units they support.
>
> Does anyone want to have a look over this? If not, I plan to push it
> in the next day or so.
>
> (I'm not sure why pgindent removed the space between != and NULL, but
> it did, so I left it.)
>
> David
I like the approach you took here; much cleaner to have one table for all of the individual
codepaths. Testing worked as expected; if we do decide to expand the units table there will be a
few additional changes (most significantly, the return value of `pg_size_bytes()` will need to switch
to `numeric`).
Thanks,
David
