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 couldn't read what Robert wrote upthread but I also would liketo have 2-base byte unit prifixes. (Sorry I couldn't
putitaside..)
2. Doesn't it allow units in lower case?
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
rejectsprifixing negative sign.
4. Useless includes
dbsize.c is modified to include guc.h but it is useless.
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.
6. Symmetry with pg_size_pretty
pg_size_pretty doesn't have units larger than TB. Just adding PBto it breaks backward compatibility, but leaving as it
isbreakssymmetry with this function. Some possible solution for thisthat I can guess for now are..
- 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
involving2-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 thatit forgets the unit 'B' or prifixing spaces or sings),
buttheyare necessarily have individual tests. The following SQLstatement will do them at once.
SELECT s, pg_size_bytes(s) FROM (VALUES ('1'), ('1B')...) as t(s);
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center