On Mon, Jan 4, 2016 at 6:03 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote: > > 2016-01-04 17:48 GMT+01:00 Shulgin, Oleksandr <oleksandr.shulgin@zalando.de>: >> >> On Mon, Jan 4, 2016 at 4:51 PM, Robert Haas <robertmhaas@gmail.com> wrote: >>> >>> On Mon, Jan 4, 2016 at 10:17 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote: >>> > [ new patch ] >>> >>> + case '-': >>> + ereport(ERROR, >>> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), >>> + errmsg("size cannot be negative"))); >>> >>> Why not? I bet if you copy any - sign to the buffer, this will Just Work. >> >> >> I'm also inclined on dropping that explicit check for empty string below and let numeric_in() error out on that. Does this look OK, or can it confuse someone:
>> >> postgres=# select pg_size_bytes(''); >> ERROR: invalid input syntax for type numeric: "" >> >> ? >> >>> + if ( conv->base_unit == GUC_UNIT_KB && >> >> >> Between "(" and "conv->..." I believe. > > > both fixed
Hm...
> + switch (*strptr) > + { > + /* ignore plus symbol */ > + case '+': > + case '-': > + *bufptr++ = *strptr++; > + break; > + }
Well, to that amount you don't need any special checks, I'm just not sure if reported error message is not misleading if we let numeric_in() handle all the errors. At least it can cope with the leading spaces, +/- and empty input quite well.
I don't would to catch a exception from numeric_in - so I try to solve some simple situations, where I can raise possible better error message.