Re: custom function for converting human readable sizes to bytes

2015-12-22 6:57 GMT+01:00 Michael Paquier <michael(dot)paquier(at)gmail(dot)com>:

> On Tue, Dec 22, 2015 at 2:33 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> wrote:
> >
> >
> > 2015-12-22 6:22 GMT+01:00 Michael Paquier <michael(dot)paquier(at)gmail(dot)com>:
> >>
> >> On Tue, Dec 22, 2015 at 12:11 AM, Robert Haas <robertmhaas(at)gmail(dot)com>
> >> wrote:
> >> > On Sun, Dec 20, 2015 at 4:54 AM, Pavel Stehule <
> pavel(dot)stehule(at)gmail(dot)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,
>

Nice to have it. But it can introduce a upgrade issues - there isn't
possible to specify base. If we do some change here, we have to do related
changes everywhere.

Our implementation is little bit obsolete, but nobody did some error
reports, so probably now isn't time to change it. It's not too important.

Better to have two conversion tables (simple and small), than opening new
topic, where I don't see any agreement to do changes - and these changes
can be done separately.

Pavel

> --
> Michael
>