On Tue, Oct 14, 2014 at 3:22 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > > > On Tue, Oct 7, 2014 at 8:38 AM, Ali Akbar <the.ap...@gmail.com> wrote: >> >> 2014-10-06 22:51 GMT+07:00 Marti Raudsepp <ma...@juffo.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".
The patch looks good to me. Barring any objection I will commit the patch. Memo for me: CATALOG_VERSION_NO must be changed at the commit. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
