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>
In-Reply-To: <c5fa9c81-f53d-8fd9-dfba-bc8c7091d5db@enterprisedb.com>
From: David Rowley <dgrowleyml@gmail.com>
Date: Fri, 3 Mar 2023 08:58:02 +1300
Message-ID: <CAApHDvpzxuo5F8-zbDoK7ZFmVjMbvVK3XQjNcZZuGcMhBc0V5w@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/CAApHDvpzxuo5F8-zbDoK7ZFmVjMbvVK3XQjNcZZuGcMhBc0V5w%40mail.gmail.com>
Precedence: bulk

On Mon, 27 Feb 2023 at 21:34, Peter Eisentraut
<peter.eisentraut@enterprisedb.com> wrote:
>
> On 22.02.23 03:39, David Rowley wrote:
> > 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.
>
> Ok, here is a new patch with a separate table of aliases.  (Might look
> like overkill, but I think the "PiB" etc. example you had could actually
> be a good use case for this as well.)

I think I'd prefer to see the size_bytes_unit_alias struct have an
index into size_pretty_units[] array. i.e:

struct size_bytes_unit_alias
{
    const char *alias; /* aliased unit name */
    const int unit_index; /* corresponding size_pretty_units element */
};

then the pg_size_bytes code can be simplified to:

/* If not found, look in the table of aliases */
if (unit->name == NULL)
{
    for (const struct size_bytes_unit_alias *a = size_bytes_aliases;
a->alias != NULL; a++)
    {
        if (pg_strcasecmp(strptr, a->alias) == 0)
        {
            unit = &size_pretty_units[a->unit_index];
            break;
        }
    }
}

which saves having to have the additional and slower nested loop code.

Apart from that, the patch looks fine.

David