On 18 February 2016 at 02:00, Vitaly Burovoy <vitaly.buro...@gmail.com> wrote: >> + else >> + have_digits = false; > Does it worth to move it to the declaration and remove that "else" block? > + bool have_digits = false;
OK, that's probably a bit neater. > Is it important to use: >> + ObjectIdGetDatum(InvalidOid), >> + Int32GetDatum(-1))); > instead of "0, -1"? Previously I left immediate constants because I > found the same thing in jsonb.c (rows 363, 774)... I think it's preferable to use the macros because they make it clearer what datatypes are being passed around. I think that's the more common style, but the JSONB code is not technically wrong either. >> + if (pg_strcasecmp(strptr, "bytes") == 0) >> + else if >> + else if >> + else if > It seems little ugly for me. In that case (not using values from GUC) > I'd create a static array for it and do a small cycle via it. Would it > better? That's a matter of personal preference. I prefer the values inline because then the values are closer to where they're being used, which for me makes it easier to follow (see e.g., convert_priv_string()). In guc.c they're in lookup tables because they're referred to from multiple places, and because it needs to switch between tables based on context, neither of which is true here. If there is a need to re-use these values elsewhere in the future it can be refactored, but right now that feels like an over-engineered solution for this problem. >> + NumericGetDatum(mul_num), > Two more space for indentation. pgindent pulled that back. > Also I've found that with allowing exponent there is a weird result > (we tried to avoid "type numeric" in the error message): > SELECT > pg_size_bytes('123e100000000000000000000000000000000000000000000000000000000000000000000000 > kB'); > ERROR: invalid input syntax for type numeric: > "123e100000000000000000000000000000000000000000000000000000000000000000000000" OK, I'll add a check for that. Thanks for testing. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers