Re: Add support for unit "B" to pg_size_pretty()

On Wed, 22 Feb 2023 at 12:47, Peter Eisentraut
<peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
> >> diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
> >> index dbd404101f..9ecd5428c3 100644
> >> --- a/src/backend/utils/adt/dbsize.c
> >> +++ b/src/backend/utils/adt/dbsize.c
> >> @@ -49,6 +49,7 @@ struct size_pretty_unit
> >> /* When adding units here also update the error message in pg_size_bytes */
> >> static const struct size_pretty_unit size_pretty_units[] = {
> >> {"bytes", 10 * 1024, false, 0},
> >> + {"B", 10 * 1024, false, 0},
> >
> > This adds a duplicate line (unitbits=0) where no other existing line
> > uses duplicates. If that's intentional, I think it deserves a comment
> > highlighting that it's an /*alias*/, and about why that does the right
> > thing, either here about or in the commit message.
>
> I have added a comment about that.

hmm. I didn't really code pg_size_pretty with aliases in mind. I don't
think you can do this. There's code in pg_size_pretty() and
pg_size_pretty_numeric() that'll not work correctly. We look ahead to
the next unit to check if there is one so we know we must use this
unit if there are no other units to convert to.

Let's assume someone in the future reads your comment about aliases
and thinks we can just go and add an alias for any unit. Here we'll
add PiB for PB.

diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index dbd404101f..8e22969a76 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -54,6 +54,7 @@ static const struct size_pretty_unit size_pretty_units[] = {
{"GB", 20 * 1024 - 1, true, 30},
{"TB", 20 * 1024 - 1, true, 40},
{"PB", 20 * 1024 - 1, true, 50},
+ {"PiB", 20 * 1024 - 1, true, 50},
{NULL, 0, false, 0}
};

testing it, I see:

postgres=# select pg_size_pretty(10000::numeric * 1024*1024*1024*1024*1024);
pg_size_pretty
----------------
10000 PB
(1 row)

postgres=# select pg_size_pretty(20000::numeric * 1024*1024*1024*1024*1024);
pg_size_pretty
----------------
20000 PiB
(1 row)

I think we'll likely get complaints about PB being used sometimes and
PiB being used at other times.

I think you'll need to find another way to make the aliases work.
Maybe another array with the name and an int to reference the
corresponding index in size_pretty_units.

David