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
v6-0002-Collect-Function-Scan-storage-statistics-outside-.patch
Description: Binary data
v6-0001-Add-memory-disk-usage-for-Function-Scan-nodes-in-.patch
Description: Binary data
