On Mon, Sep 23, 2013 at 4:15 PM, Tomas Vondra <t...@fuzzy.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
