Re: [PATCH] Add memory/disk usage for Function Scan nodes in EXPLAIN

Hi David,

> It's the FunctionNext() changes that I'm not keen on. I feel that's
> adding too much code to a function that executes on every row.

Thanks for the review and the patch! I'll keep that in mind when
touching the executor's hot paths.

Attached is v6, based on your patch, which leaves FunctionNext() as is.
0001 is v5 as is, and 0002 is your patch with my changes on top. I kept
them separate so that the changes from your patch are easy to see, but
0002 should be squashed into 0001.

The main change is in the total taken at rescan. The statistics were
only collected when bms_overlap() returned true, so the tuplestores of
the functions whose parameters didn't change weren't included. I've
changed that and moved the code into update_storage_stats(). For example,
the following query showed 111kB, whereas the correct value is 189kB:

explain (analyze, costs off, timing off, summary off) select * from
(values(2000),(1)) v(i), lateral (select * from rows from
(generate_series(1, v.i), generate_series(1, 2000)));

Other than that, I've put the number of rows in the ROWS FROM test back
to 2500, and fixed compiler warnings and a typo in a comment. With 1250
rows, neither tuplestore spilled to disk on a build without
--enable-cassert, so the test failed with "Memory".

Regards,
Tatsuya Kawata