Re: Added prosupport function for estimating numeric generate_series rows

On Fri, 29 Nov 2024 at 06:25, 孤傲小二~阿沐 <tsinghualucky912(at)foxmail(dot)com> wrote:
> Hello, thank you very much for your attention and guidance. I have modified and improved the problem you mentioned. The patch of version v2 is attached below.

I've only had a quick look at the patch.

* The following needs a DatumGetFloat8():

+ req->rows = numericvar_to_double_no_overflow(&q) + 1;

* It would also be good to see a comment explaining the following line:

+ if(nstep.sign != var_diff.sign)

Something like: /* When the sign of the step size and the series range
don't match, there are no rows in the series. */

* You should add one test for the generate_series(numeric, numeric).
You've only got tests for the 3 arg version.

Also a few minor things:

* Missing space after "if"

+ if(arg3)

* We always have an empty line after variable declarations, that's missing in:

+ NumericVar q;
+ init_var(&q);

* No need for the braces in:

+ if (NUMERIC_IS_SPECIAL(step))
+ {
+ goto cleanup;
+ }

David