On Thu, Jul 19, 2018 at 4:32 PM, Andres Freund <andres@anarazel.de> wrote:
>> 1. Why the error message changes? If there's a good reason, it should
>> be done as a separate commit, or at least well-documented in the
>> commit message.
>
> Because there's a lot of "invalid input syntax for type %s: \"%s\"",
> error messages, and we shouldn't force translators to have separate
> version that inlines the first %s. But you're right, it'd be worthwhile
> to point that out in the commit message.
It just seems weird that they're bundled together in one commit like this.
>> 2. Does the likely/unlikely stuff make a noticeable difference?
>
> Yes. It's also largely a copy from existing code (scanint8), so I don't
> really want to differ here.
OK.
>> 3. If this is a drop-in replacement for pg_atoi, why not just recode
>> pg_atoi this way -- or have it call this -- and leave the callers
>> unchanged?
>
> Because pg_atoi supports a variable 'terminator'.
OK.
>> 4. Are we sure this is faster on all platforms, or could it work out
>> the other way on, say, BSD?
>
> I'd be *VERY* surprised if any would be faster. It's not easy to write a
> faster implmentation, than what I've proposed, and especially not so if
> you use strtol() as the API (variable bases, a bit of locale support).
OK.
Nothing else from me...
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
