On Sun, Oct 5, 2014 at 7:39 PM, Ali Akbar <the.ap...@gmail.com> wrote:
> > 2014-10-05 15:21 GMT+07:00 Ali Akbar <the.ap...@gmail.com>: > >> - i think you can use the fctx->current variable without temporary >> variable (there's comment in the add_var function: Full version of add >> functionality on variable level (handling signs). result might point to one >> of the operands too without danger.). But you _must_ switch the context >> first because add_var will allocate new array for the data and freeing the >> old one. >> > Yep. > - numeric can be NaN. We must reject it as first, finish and last >> parameter. >> > Makes sense. > - numeric datatype is large, but there are limitations. According to doc, >> the limit is: up to 131072 digits before the decimal point; up to 16383 >> digits after the decimal point. How can we check if the next step >> overflows? As a comparison, in int.c, generate_series_step_int4 checks if >> its overflows, and stop the next call by setting step to 0. Should we do >> that? >> > Yes we should. > - while testing regression test, opr_sanity checks that the > generate_series_numeric function is used twice (once for 2 parameter and > once for the 3 parameter function), so i changed the name to > generate_series_step_numeric and created new function > generate_series_numeric that calls generate_series_step_numeric. > Yep. It seems to me that this patch is heading in a good direction (haven't tested or tried to break it, just looked at the code). However please be careful of code format, particularly brackets for "if" blocks. For example this thing: if (foo) { blah; } Should be that: if (foo) blah; Then in the case of multiple lines, this thing: if (foo) { blah; blah2; } Should be that: if (foo) { blah; blah2; } Code convention is detailed in the docs: http://www.postgresql.org/docs/devel/static/source.html Regards, -- Michael