MIME-Version: 1.0
References: <0106914a-9eb5-22be-40d8-652cc88c827d@enterprisedb.com>
 <20230220143452.GM1653@telsasoft.com> <e465dc9e-2d88-6ff7-9fa6-d41ab78dd56e@enterprisedb.com>
 <CAApHDvrn9aznj=h1srcjyPpQVBA_B+d7izYvtUa-5ni+GZ7z0A@mail.gmail.com>
 <c5fa9c81-f53d-8fd9-dfba-bc8c7091d5db@enterprisedb.com> <CAApHDvpzxuo5F8-zbDoK7ZFmVjMbvVK3XQjNcZZuGcMhBc0V5w@mail.gmail.com>
 <4b4613ec-d2c2-a615-9e3f-9f36cf5897ed@enterprisedb.com>
In-Reply-To: <4b4613ec-d2c2-a615-9e3f-9f36cf5897ed@enterprisedb.com>
From: David Rowley <dgrowleyml@gmail.com>
Date: Mon, 6 Mar 2023 21:27:36 +1300
Message-ID: <CAApHDvrCwMgSD_93LZr4CLMas8Hc61fXAQ-Cd4=+yoRfHnYbJA@mail.gmail.com>
Subject: Re: Add support for unit "B" to pg_size_pretty()
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>, pgsql-hackers@postgresql.org
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://www.postgresql.org/message-id/CAApHDvrCwMgSD_93LZr4CLMas8Hc61fXAQ-Cd4%3D%2ByoRfHnYbJA%40mail.gmail.com>
Precedence: bulk

On Mon, 6 Mar 2023 at 21:13, Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
>
> On 02.03.23 20:58, David Rowley wrote:
> > I think I'd prefer to see the size_bytes_unit_alias struct have an
> > index into size_pretty_units[] array. i.e:
>
> Ok, done that way.  (I had thought about that, but I was worried that
> that would be too error-prone to maintain.  But I suppose the tables
> don't change that often, and test cases would easily catch mistakes.)

Patch looks pretty good. I just see a small spelling mistake in:

+/* Additional unit aliases acceted by pg_size_bytes */

> I also updated the documentation a bit more.

I see I must have forgotten to add PB to the docs when pg_size_pretty
had that unit added.  I guess you added the "etc" to fix that?  I'm
wondering if that's the right choice. You modified the comment above
size_pretty_units[] to remind us to update the docs when adding units,
but the docs now say "etc", so do we need to?  I'd likely have gone
with just adding "PB" to the docs, that way it's pretty clear that new
units need to be mentioned in the docs.

David