Re: custom function for converting human readable sizes to bytes

2016-02-16 11:25 GMT+01:00 Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>:

> On 16 February 2016 at 05:01, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> > 2016-02-15 10:16 GMT+01:00 Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>:
> >> Is there any reason not to make
> >> pg_size_bytes() return numeric?
> >>
> > This is a question. I have not a strong opinion about it. There are no
> any
> > technical objection - the result will be +/- same. But you will enforce
> > Numeric into outer expression evaluation.
> >
> > The result will not be used together with function pg_size_pretty, but
> much
> > more with functions pg_relation_size, pg_relation_size, .. and these
> > functions doesn't return Numeric. These functions returns bigint. Bigint
> is
> > much more natural type for this purpose.
> >
> > Is there any use case for Numeric?
> >
>
> [Shrug] I don't really have a strong opinion about it either, but it
> seemed that maybe the function might have some wider uses beyond just
> comparing object sizes, and since it's already computing the numeric
> size, it might as well just return it.
>

I see only one disadvantage - when we return numeric, then all following
expression will be in numeric, and for some functions, or some expression
the users will have to cast explicitly to bigint.

>
>
> Looking at the rest of the patch, I think there are other things that
> need tidying up -- the unit parsing code for one. This is going to
> some effort to reuse the memory_unit_conversion_table from guc.c, but
> the result is that it has added quite a bit of new code and now the
> responsibility for parsing different units is handled by different
> functions in different files, which IMO is quite messy. Worse, the
> error message is wrong and misleading:
>
> select pg_size_bytes('10 bytes'); -- OK
> select pg_size_bytes('10 gallons');
> ERROR: invalid size: "10 gallons"
> DETAIL: Invalid size unit "gallons"
> HINT: Valid units for this parameter are "kB", "MB", "GB", and "TB".
>
> which fails to mention that "bytes" is also a valid unit.
>

true

>
> Fixing that in parse_memory_unit() would be messy because it assumes a
> base unit of kB, so it would require a negative multiplier, and
> pg_size_bytes() would have to be taught to divide by the magnitude of
> negative multipliers in the same way that guc.c does.
>
> ISTM that it would be far less code, and much simpler and more
> readable to just parse the supported units directly in
> pg_size_bytes(), rather than trying to share code with guc.c, when the
> supported units are actually different and may well diverge further in
> the future.
>

yes, if we have different units, then independent control tables has more
sense.

>
> I'll try to hack something up, and see what it looks like.
>
> Regards,
> Dean
>