Re: custom function for converting human readable sizes to bytes

2015-12-01 11:12 GMT+01:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:

>
>
> 2015-12-01 11:02 GMT+01:00 Kyotaro HORIGUCHI <
> horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)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(dot)stehule(at)gmail(dot)com> wrote in <
>> CAFj8pRCd6We3TQMR0VbCN98wF0P3O3H6NaU4BTaosWcj443Qjw(at)mail(dot)gmail(dot)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
>>
>>
>>
>