Re: [PATCH] Custom code int(32|64) => text conversions out of performance reasons

On Sun, Oct 31, 2010 at 5:41 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> While at it:

These words always make me a bit frightened when reviewing a patch,
since it's generally simpler if a single patch only does one thing.
However, in this case...

> * I remove the outdated
> -- NOTE: int[24] operators never check for over/underflow!
> -- Some of these answers are consequently numerically incorrect.
> warnings in the regressions tests.

...this part looks obviously OK, so I have committed it.

The rest is attached as a residual patch, except that I reverted this change:

> * I renamed pg_[il]toa to pg_s(16|32|64)toa - I found the names confusing. Not
> sure if its worth it.

I notice that int8out isn't terribly consistent with int2out and
int4out, in that it does an extra copy. Maybe that's justified given
the greater potential memory wastage, but I'm not certain. One
approach might be to pick some threshold value and allocate a buffer
in one of two sizes based on how large the value is relative to that
cutoff. But that might also be a stupid idea, not sure.

It would speed things up for me if you or someone else could take a
quick pass over what remains here and fix the formatting and
whitespace to be consistent with our general project style, and make
the comment headers more consistent among the functions being
added/modified.

I think the new regression tests look good.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company