On Mon, Mar 12, 2018 at 01:45:26PM -0400, Peter Eisentraut wrote:
> On 3/8/18 03:08, Michael Paquier wrote:
>> Here are some comments for patch 2.
>>
>> + if (endptr != token + length || errno == ERANGE ||
>> + /* check for overflow of int4 */
>> + val != (long) ((int32) val))
>> return T_Float;
>> It would be nice to have this check be consistent with the new
>> definition of ival and int32, One suggestion is to use directly int32 or
>> just have a static assertion that sizeof(int) == sizeof(int32)? Or
>> that's too much nannyism?
>
> Fixed in the attached next version.
Thanks, this looks good to me.
>> If you care about those, you
>> could also remove the ones in interval.c and datetime.c...
>
> Actually, we could just use the strtoint() defined there and apply it
> everywhere, so avoid repeating these patterns. Done so in an additional
> patch.
OK, that's a good idea.
+/*
+ * strtoint --- just like strtol, but returns int not long
+ */
It would be nice to document that caller should check for errno for
sanity checks, in which case caller should not rely on the returned
result.
Worth noting that pgbench has its own view of things with strtoint64.
Not worth bothering anyway.
--
Michael
