On Thu, Jul 19, 2018 at 4:32 PM, Andres Freund <andres@xxxxxxxxxxx> 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
