From: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
---|---|
To: | Vitaly Burovoy <vitaly(dot)burovoy(at)gmail(dot)com> |
Cc: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, "Shulgin, Oleksandr" <oleksandr(dot)shulgin(at)zalando(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>, Guillaume Lelarge <guillaume(at)lelarge(dot)info>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: custom function for converting human readable sizes to bytes |
Date: | 2016-02-18 10:05:23 |
Message-ID: | CAEZATCWOLHftSKv=6TnSCQDTZZDY02k-r4E6cPQQqE9MYB0AEw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 18 February 2016 at 02:00, Vitaly Burovoy <vitaly(dot)burovoy(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Etsuro Fujita | 2016-02-18 10:18:59 | Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs) |
Previous Message | Vladimir Sitnikov | 2016-02-18 10:05:10 | Re: JDBC behaviour |