The patch seems fine to me, the following comments are mostly nitpicking:
1) Applies cleanly to the HEAD (although only by "patch" and not "git apply").
2) I think we should use "estimate" instead of "approximation" in the docs, it seems more correct / natural to me (but maybe I'm wrong on this one).
3) state_data_size does not make much sense to me - it should be state_size. This probably comes from the state_data_type, but that's ('state' + 'data type') and by replacing the second part with 'size' you'll get state_size.
This name is consistent with previous field state_data_type - I expected so this mean 'state data' + 'type'. I am not native speaker, so my position is not strong, but in this moment I am thinking so state_data_size has a sense. In this case both variant has sense - 'state data' + type or 'state' + 'data type'.
4) currently the state size may be specified for all data types, no matter if their size is fixed or variable. Wouldn't it be safer to allow this option only for variable-length data types? Right now you can specify stype=int and sspace=200 which does not make much sense to me. We can always relax the restrictions if needed, the opposite is much more difficult.
good idea
5) in numeric.c, there are no comments for - fields of the NumericAggState (some are obvious, some are not) - multiple functions are missing the initial comments (e.g. numeric_accum, do_numeric_accum)
ok
6) I think the "first" variable in do_numeric_accum is rather useless, it's trivial to remove it - just use the state->first instead and move the assignment to the proper branch (ok, this is nitpicking).
ok
7) I think the error message in makeNumericAggState should be something like "This must not be called in non-aggregate context!" or something like that.
I used a "a numeric aggregate function called in non-aggregate context" - it is similar to other related messages
8) The records in pg_aggregate.c are using either 0 (for fixed-length) or 128. This seems slightly excessive to me. What is the reasoning behind this? Is that because of the two NumericVar fields?
NumericAggState has 96 bytes - but you have to add a space for digits of included numeric values (inclued in NumericVar) -- so it is others 16 + 16 = 128. I am not able to specify how much digits will be used exactly - 16 bytes is just good enough estimation - it is not used for memory allocation, it is used for some planner magic.