Hi David, Thanks for the review!
> 1. I'm confused by the following in explain.sql. We don't support > parallel function scans, so the comments and setting of > max_parallel_workers_per_gather seem bogus: Function Scan isn't parallel-aware, but generate_series() is parallel safe, so with debug_parallel_query = regress the whole plan runs in a parallel worker. EXPLAIN only reads the storage information from the leader's tuplestore, so the Storage line doesn't appear in that case. Indeed, when I removed the setting and ran make check with debug_parallel_query = regress, the Storage line disappeared from all three of the new tests and the explain test failed. So I kept the setting and rewrote the comment to make the reason clearer. > 2. In the following, 10000 tuples seems a little excessive to spill 64 > ... > You could also move the existing "reset work_mem;" down after your > tests. Your 10-row test won't spill with work_mem set to 64. As you suggested, I changed it to 2500 rows, moved the existing "reset work_mem;" down after the new tests, and removed the "set work_mem to 64;" that I had added. In v3, only the tests in 0002 have changed; 0001 is unchanged. Regards, Tatsuya Kawata
v3-0001-Keep-Function-Scan-tuplestore-statistics-across-r.patch
Description: Binary data
v3-0002-Add-memory-disk-usage-for-Function-Scan-nodes-in-.patch
Description: Binary data
