Re: Add generate_series(numeric, numeric)

On Tue, Oct 7, 2014 at 8:38 AM, Ali Akbar <the(dot)apaan(at)gmail(dot)com> wrote:

> 2014-10-06 22:51 GMT+07:00 Marti Raudsepp <marti(at)juffo(dot)org>:
>
>
>
>> > the one that tests values just before numeric overflow
>>
>> Actually I don't know if that's too useful. I think you should add a
>> test case that causes an error to be thrown.
>>
>
> Actually i added the test case because in the code, when adding step into
> current for the last value, i expected it to overflow:
>
> /* increment current in preparation for next iteration */
> add_var(&fctx->current, &fctx->step, &fctx->current);
>
> where in the last calculation, current is 9 * 10^131071. Plus 10^131071,
> it will be 10^131072, which i expected to overflow numeric type (in the
> doc, numeric's range is "up to 131072 digits before the decimal point").
>
> In attached patch, i narrowed the test case to produce smaller result.
>

Well, as things stand now, the logic relies on cmp_var and the sign of the
stop and current values. it is right that it would be better to check for
overflow before going through the next iteration, and the cleanest and
cheapest way to do so would be to move the call to make_result after
add_var and save the result variable in the function context, or something
similar, but honestly I am not sure it is worth the complication as it
would mean some refactoring on what make_result actually already does.

I looked at this patch again a bit and finished with the attached, adding
an example in the docs, refining the error messages and improving a bit the
regression tests. I have nothing more to comment, so I am marking this
patch as "ready for committer".
Regards,
--
Michael