Re: custom function for converting human readable sizes to bytes - Mailing list pgsql-hackers

On Tue, Dec 22, 2015 at 2:33 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
> 2015-12-22 6:22 GMT+01:00 Michael Paquier <michael.paquier@gmail.com>:
>>
>> On Tue, Dec 22, 2015 at 12:11 AM, Robert Haas <robertmhaas@gmail.com>
>> wrote:
>> > On Sun, Dec 20, 2015 at 4:54 AM, Pavel Stehule <pavel.stehule@gmail.com>
>> > wrote:
>> >> new update:
>> >>
>> >> 1. unit searching is case insensitive
>> >>
>> >> 2. initial support for binary byte prefixes - KiB, MiB, ..  (IEC
>> >> standard),
>> >> change behave for SI units
>> >>
>> >> Second point is much more complex then it is looking - if pg_size_bytes
>> >> should be consistent with pg_size_pretty.
>> >>
>> >> The current pg_size_pretty and transformations in guc.c are based on
>> >> JEDEC
>> >> standard. Using this standard for GUC has sense - using it for object
>> >> sizes
>> >> is probably unhappy.
>> >>
>> >> I tried to fix (and enhance) pg_size_pretty - now reports correct
>> >> units, and
>> >> via second parameter it allows to specify base: 2 (binary, IEC  -
>> >> default)
>> >> or 10 (SI).
>> >
>> > -1 from me.  I don't think we should muck with the way pg_size_pretty
>> > works.
>>
>> Yeah.
>>
>> + static const unit_multiplier unit_multiplier_table[] =
>> + {
>> +     {"B", 1L},
>> +     {"kiB", 1024L},
>> +     {"MiB", 1024L * 1024},
>> +     {"GiB", 1024L * 1024 * 1024},
>> +     {"TiB", 1024L * 1024 * 1024 * 1024},
>> +     {"PiB", 1024L * 1024 * 1024 * 1024 * 1024},
>> This is rather close to memory_unit_conversion_table in guc.c. Would
>> it be worth refactoring those unit tables into something more generic
>> instead of duplicating them?
>
>
> yes, it is possible with following impacts:
>
> 1. We need add PB to memory_unit_conversion_table in guc.c

No real objection to that. It would would make sense to have it, but
we could not use it directly for a GUC. This just reminded me that
even if we support TB in GUC params, it is not possible to set for
example a GUC_UNIT_KB param to more than 2TB because those are limited
to be int32.

> 2. This table holds multipliers in JEDEC standard - and introduce other
> standards IEC, SI there isn't good idea.
>
> Is it ok?

Do you think it would be interesting to have GUC parameters able to
use those units? If this is a standard, it may make sense to actually
have them, no? Just a random thought, that's not something this patch
should take care of, but it would be good to avoid code duplication
where we can avoid it.
Regards,

--
Michael