Re: custom function for converting human readable sizes to bytes

2015-12-01 11:12 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

>
>
> 2015-12-01 11:02 GMT+01:00 Kyotaro HORIGUCHI <
> horiguchi.kyotaro@lab.ntt.co.jp>:
>
>> Hello, The meaning of "be orange" (I couldn't find it) interests
>> me but putting it aside..
>>
>> I have some random comments.
>>
>> At Mon, 30 Nov 2015 18:30:18 +0100, Pavel Stehule <
>> pavel.stehule@gmail.com> wrote in <
>> CAFj8pRCd6We3TQMR0VbCN98wF0P3O3H6NaU4BTaosWcj443Qjw@mail.gmail.com>
>> > Hi
>> >
>> >
>> > > 2. using independent implementation - there is some redundant code,
>> but we
>> > > can support duble insted int, and we can support some additional
>> units.
>> > >
>> >
>> >  new patch is based on own implementation - use numeric/bigint
>> calculations
>> >
>> > + regress tests and doc
>>
>> 1. What do you think about binary byte prefixes? (kiB, MiB...)
>>
>
> I don't know this mechanics - can you write it? It can be good idea/
>
>
>>
>>  I couldn't read what Robert wrote upthread but I also would like
>>  to have 2-base byte unit prifixes. (Sorry I couldn't put it
>>  aside..)
>>
>>
>> 2. Doesn't it allow units in lower case?
>>
>
> The parser is consistent with a behave used in configure file processing.
> We can change it, but then there will be divergence between this function
> and GUC parser.
>
>
>>
>>  I think '1gb' and so should be acceptable as an input.
>>
>>
>> 3. Why are you allow positive sign prefixing digits? I suppose
>>   that positive sign also shouldn't be allowed if it rejects
>>   prifixing negative sign.
>>
>
> I have not strong opinion about it. '-' is exactly wrong, but '+' can be
> acceptable. But it can be changed.
>
>
>>
>> 4. Useless includes
>>
>>  dbsize.c is modified to include guc.h but it is useless.
>>
>
> I'll fix it.
>
>
>>
>> 5. Error message
>>
>> +       if (ndigits == 0)
>> +               ereport(ERROR,
>> +                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
>> +                                        errmsg("string is empty")));
>>
>>  If pg_size_bytes allows prefixing positive sign or spaces,
>>  ndigits == 0 here doesn't mean that the whole string is empty.
>>
>>
> I'll fix it.
>
>
>>
>> 6. Symmetry with pg_size_pretty
>>
>>  pg_size_pretty doesn't have units larger than TB. Just adding PB
>>  to it breaks backward compatibility, but leaving as it is breaks
>>  symmetry with this function. Some possible solution for this
>>  that I can guess for now are..
>>
>
> I prefer a warning about possible compatibility issue in pg_size_pretty
> and support PB directly there.
>
>
>>
>>   - Leave it as is.
>>
>>   - New GUC to allow PB for pg_size_pretty().
>>
>>   - Expanded function such like pg_size_pretty2 (oops..)
>>
>>   - New polymorphic (sibling?) function with additional
>>     parameters to instruct how they work. (The following
>>     involving 2-base representations)
>>
>>     pg_size_pretty(n, <2base>, <allow_PB or such..>);
>>     pg_size_bytes(n, <2base>, <allow_PB or such..>);
>>
>> 7. Docs
>>
>> +          Converts a size in human-readable format with size units
>> +          to bytes
>>
>>  The descriptions for the functions around use 'into' instead of
>>  'to' for the preposition.
>>
>>
>> 8. Regression
>>
>>  The regression in the patch looks good enough as is (except that
>>  it forgets the unit 'B' or prifixing spaces or sings), but they
>>  are necessarily have individual tests. The following SQL
>>  statement will do them at once.
>>
>>   SELECT s, pg_size_bytes(s) FROM (VALUES ('1'), ('1B')...) as t(s);
>>
>>
> I'll fix it.
>

here is updated patch

Regards

Pavel


>
> Thank you for your ideas
>
> Regards
>
> Pavel
>
>
>> regards,
>>
>> --
>> Kyotaro Horiguchi
>> NTT Open Source Software Center
>>
>>
>>
>