Re: Improving avg performance for numeric

On Mon, Sep 23, 2013 at 4:15 PM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
> Seems "ready for commiter" to me. I'll wait a few days for others to
> comment, and then I'll update the commitfest page.

Some thoughts:

The documentation doesn't reflect the restriction to type internal.
On a related note, why restrict this to type internal?

Formatting fixes are needed:

+ if (aggtransspace > 0)
+ {
+ costs->transitionSpace += aggtransspace;
+ }

Project style is not to use curly-braces for single statements. Also,
the changes to numeric.c add blank lines before and after function
header comments, which is not the usual style.

! if (state == NULL)
! PG_RETURN_NULL();
! else
! PG_RETURN_POINTER(state);

I think this should just say PG_RETURN_POINTER(state). PG_RETURN_NULL
is for returning an SQL NULL, not (void *) 0. Is there some reason
why we need an SQL NULL here, rather than a NULL pointer?

On the whole this looks fairly solid on a first read-through.

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