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.
> By the way, why do you remove HAVE_LONG_INT_64? On platforms where long
> is 4 bytes this would still be a no-op.
Right, but the compiler can optimize it away then. No need to have #ifdefs.
> The comment block on top of the definition of Value in value.h still
> mentions "long", while it should mention "int" per your patch.
done
> 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.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
